From 644b0fa55423581bf4092192d35fd22436400948 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Thu, 22 Aug 2024 16:44:04 -0400 Subject: [PATCH 01/45] adds sdk changes to cw logs components --- exporter/awscloudwatchlogsexporter/config.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/exporter/awscloudwatchlogsexporter/config.go b/exporter/awscloudwatchlogsexporter/config.go index 915003e993d0..1888e45c5839 100644 --- a/exporter/awscloudwatchlogsexporter/config.go +++ b/exporter/awscloudwatchlogsexporter/config.go @@ -12,6 +12,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/awsutil" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" ) // Config represent a configuration for the CloudWatch logs exporter. @@ -26,6 +27,9 @@ type Config struct { // that share the same source. LogStreamName string `mapstructure:"log_stream_name"` + //Entity is the name of the service identifier of the CloudWatch log stream + Entity cloudwatchlogs.Entity + // Endpoint is the CloudWatch Logs service endpoint which the requests // are forwarded to. https://docs.aws.amazon.com/general/latest/gr/cwl_region.html // e.g. logs.us-east-1.amazonaws.com From ce92f5ef7f7c218515aeff4293ea9d67327767e3 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Wed, 28 Aug 2024 18:40:44 -0400 Subject: [PATCH 02/45] adds entity on export --- exporter/awscloudwatchlogsexporter/config.go | 4 -- exporter/awsemfexporter/emf_exporter.go | 10 ++-- exporter/awsemfexporter/emf_exporter_test.go | 63 ++++++++++++++++---- exporter/awsemfexporter/metric_translator.go | 55 ++++++++++++++++- internal/aws/cwlogs/cwlog_client_test.go | 26 ++++++++ internal/aws/cwlogs/pusher.go | 32 ++++++++-- internal/aws/cwlogs/pusher_test.go | 1 + internal/aws/cwlogs/utils.go | 15 +++++ 8 files changed, 177 insertions(+), 29 deletions(-) diff --git a/exporter/awscloudwatchlogsexporter/config.go b/exporter/awscloudwatchlogsexporter/config.go index 1888e45c5839..915003e993d0 100644 --- a/exporter/awscloudwatchlogsexporter/config.go +++ b/exporter/awscloudwatchlogsexporter/config.go @@ -12,7 +12,6 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/awsutil" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" ) // Config represent a configuration for the CloudWatch logs exporter. @@ -27,9 +26,6 @@ type Config struct { // that share the same source. LogStreamName string `mapstructure:"log_stream_name"` - //Entity is the name of the service identifier of the CloudWatch log stream - Entity cloudwatchlogs.Entity - // Endpoint is the CloudWatch Logs service endpoint which the requests // are forwarded to. https://docs.aws.amazon.com/general/latest/gr/cwl_region.html // e.g. logs.us-east-1.amazonaws.com diff --git a/exporter/awsemfexporter/emf_exporter.go b/exporter/awsemfexporter/emf_exporter.go index 32a62dc33c36..c05eb3e19075 100644 --- a/exporter/awsemfexporter/emf_exporter.go +++ b/exporter/awsemfexporter/emf_exporter.go @@ -36,7 +36,7 @@ const ( ) type emfExporter struct { - pusherMap map[cwlogs.StreamKey]cwlogs.Pusher + pusherMap map[string]cwlogs.Pusher svcStructuredLog *cwlogs.Client config *Config @@ -86,7 +86,7 @@ func newEmfExporter(config *Config, set exporter.Settings) (*emfExporter, error) metricTranslator: newMetricTranslator(*config), retryCnt: *awsConfig.MaxRetries, collectorID: collectorIdentifier.String(), - pusherMap: map[cwlogs.StreamKey]cwlogs.Pusher{}, + pusherMap: map[string]cwlogs.Pusher{}, processResourceLabels: func(map[string]string) {}, } @@ -179,10 +179,10 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e func (emf *emfExporter) getPusher(key cwlogs.StreamKey) cwlogs.Pusher { var ok bool - if _, ok = emf.pusherMap[key]; !ok { - emf.pusherMap[key] = cwlogs.NewPusher(key, emf.retryCnt, *emf.svcStructuredLog, emf.config.logger) + if _, ok = emf.pusherMap[key.Hash()]; !ok { + emf.pusherMap[key.Hash()] = cwlogs.NewPusher(key, emf.retryCnt, *emf.svcStructuredLog, emf.config.logger) } - return emf.pusherMap[key] + return emf.pusherMap[key.Hash()] } func (emf *emfExporter) listPushers() []cwlogs.Pusher { diff --git a/exporter/awsemfexporter/emf_exporter_test.go b/exporter/awsemfexporter/emf_exporter_test.go index ecb2d54036df..7e64de02c4bc 100644 --- a/exporter/awsemfexporter/emf_exporter_test.go +++ b/exporter/awsemfexporter/emf_exporter_test.go @@ -6,6 +6,8 @@ package awsemfexporter import ( "context" "errors" + "github.com/aws/aws-sdk-go/aws" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" "os" "testing" @@ -32,6 +34,18 @@ func init() { os.Setenv("AWS_SECRET_ACCESS_KEY", "test") } +var entity = cloudwatchlogs.Entity{ + Attributes: map[string]*string{ + "PlatformType": aws.String("AWS::EC2"), + "InstanceId": aws.String("i-123456789"), + "AutoScalingGroup": aws.String("test-group"), + }, + KeyAttributes: map[string]*string{ + "Name": aws.String("myService"), + "Environment": aws.String("myEnvironment"), + }, +} + type mockPusher struct { mock.Mock } @@ -192,10 +206,12 @@ func TestConsumeMetricsWithLogGroupStreamConfig(t *testing.T) { }) require.Error(t, exp.pushMetricsData(ctx, md)) require.NoError(t, exp.shutdown(ctx)) - pusherMap, ok := exp.pusherMap[cwlogs.StreamKey{ + streamKey := &cwlogs.StreamKey{ LogGroupName: expCfg.LogGroupName, LogStreamName: expCfg.LogStreamName, - }] + Entity: &entity, + } + pusherMap, ok := exp.pusherMap[streamKey.Hash()] assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -223,10 +239,11 @@ func TestConsumeMetricsWithLogGroupStreamValidPlaceholder(t *testing.T) { }) require.Error(t, exp.pushMetricsData(ctx, md)) require.NoError(t, exp.shutdown(ctx)) - pusherMap, ok := exp.pusherMap[cwlogs.StreamKey{ + streamKey := &cwlogs.StreamKey{ LogGroupName: "/aws/ecs/containerinsights/test-cluster-name/performance", LogStreamName: "test-task-id", - }] + } + pusherMap, ok := exp.pusherMap[streamKey.Hash()] assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -243,21 +260,39 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { exp, err := newEmfExporter(expCfg, exportertest.NewNopSettings()) assert.NoError(t, err) assert.NotNil(t, exp) + var entity = &cloudwatchlogs.Entity{ + Attributes: map[string]*string{ + "PlatformType": aws.String("AWS::EC2"), + "AutoScalingGroup": aws.String("test-group"), + "InstanceId": aws.String("i-123456789"), + }, + KeyAttributes: map[string]*string{ + "Name": aws.String("myService"), + "Environment": aws.String("myEnvironment"), + }, + } md := generateTestMetrics(testMetric{ metricNames: []string{"metric_1", "metric_2"}, metricValues: [][]float64{{100}, {4}}, resourceAttributeMap: map[string]any{ - "aws.ecs.cluster.name": "test-cluster-name", - "aws.ecs.task.id": "test-task-id", + "aws.ecs.cluster.name": "test-cluster-name", + "aws.ecs.task.id": "test-task-id", + keyAttributeEntityServiceName: "myService", + keyAttributeEntityDeploymentEnvironment: "myEnvironment", + attributeEntityPlatformType: "AWS::EC2", + attributeEntityASG: "test-group", + attributeEntityInstanceID: "i-123456789", }, }) require.Error(t, exp.pushMetricsData(ctx, md)) require.NoError(t, exp.shutdown(ctx)) - pusherMap, ok := exp.pusherMap[cwlogs.StreamKey{ + streamKey := cwlogs.StreamKey{ LogGroupName: expCfg.LogGroupName, LogStreamName: "test-task-id", - }] + Entity: entity, + } + pusherMap, ok := exp.pusherMap[streamKey.Hash()] assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -285,10 +320,11 @@ func TestConsumeMetricsWithWrongPlaceholder(t *testing.T) { }) require.Error(t, exp.pushMetricsData(ctx, md)) require.NoError(t, exp.shutdown(ctx)) - pusherMap, ok := exp.pusherMap[cwlogs.StreamKey{ + streamKey := cwlogs.StreamKey{ LogGroupName: expCfg.LogGroupName, LogStreamName: expCfg.LogStreamName, - }] + } + pusherMap, ok := exp.pusherMap[streamKey.Hash()] assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -312,11 +348,12 @@ func TestPushMetricsDataWithErr(t *testing.T) { logPusher.On("ForceFlush", nil).Return("some error").Once() logPusher.On("ForceFlush", nil).Return("").Once() logPusher.On("ForceFlush", nil).Return("some error").Once() - exp.pusherMap = map[cwlogs.StreamKey]cwlogs.Pusher{} - exp.pusherMap[cwlogs.StreamKey{ + exp.pusherMap = map[string]cwlogs.Pusher{} + streamKey := cwlogs.StreamKey{ LogGroupName: "test-logGroupName", LogStreamName: "test-logStreamName", - }] = logPusher + } + exp.pusherMap[streamKey.Hash()] = logPusher md := generateTestMetrics(testMetric{ metricNames: []string{"metric_1", "metric_2"}, diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 19415b46113b..236b6ed123d8 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -7,6 +7,8 @@ import ( "encoding/json" "errors" "fmt" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" + "go.opentelemetry.io/collector/pdata/pcommon" "reflect" "strings" "time" @@ -32,6 +34,18 @@ const ( containerInsightsReceiver = "awscontainerinsight" attributeReceiver = "receiver" fieldPrometheusMetricType = "prom_metric_type" + + //Entity fields + keyAttributeEntityServiceName = "aws.entity.service.name" + serviceName = "Name" + keyAttributeEntityDeploymentEnvironment = "aws.entity.deployment.environment" + deploymentEnvironment = "Environment" + attributeEntityPlatformType = "aws.entity.platform.type" + platformType = "PlatformType" + attributeEntityASG = "aws.entity.autoscalegroup" + autoScalingGroup = "AutoScalingGroup" + attributeEntityInstanceID = "aws.entity.instance.id" + instanceIDTag = "InstanceId" ) var errMissingMetricsForEnhancedContainerInsights = errors.New("nil event detected with EnhancedContainerInsights enabled") @@ -79,6 +93,7 @@ type groupedMetricMetadata struct { timestampMs int64 logGroup string logStream string + entity *cloudwatchlogs.Entity metricDataType pmetric.MetricType retainInitialValueForDelta bool } @@ -123,14 +138,15 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri cWNamespace := getNamespace(rm, config.Namespace) logGroup, logStream, patternReplaceSucceeded := getLogInfo(rm, cWNamespace, config) deltaInitialValue := config.RetainInitialValueOfDeltaMetric + resourceAttributes := rm.Resource().Attributes() ilms := rm.ScopeMetrics() var metricReceiver string - if receiver, ok := rm.Resource().Attributes().Get(attributeReceiver); ok { + if receiver, ok := resourceAttributes.Get(attributeReceiver); ok { metricReceiver = receiver.Str() } - if serviceName, ok := rm.Resource().Attributes().Get("service.name"); ok { + if serviceName, ok := resourceAttributes.Get("service.name"); ok { if strings.HasPrefix(serviceName.Str(), "containerInsightsKubeAPIServerScraper") || strings.HasPrefix(serviceName.Str(), "containerInsightsDCGMExporterScraper") || strings.HasPrefix(serviceName.Str(), "containerInsightsNeuronMonitorScraper") { @@ -139,6 +155,8 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri } } + entity := fetchEntityFields(resourceAttributes) + for j := 0; j < ilms.Len(); j++ { ilm := ilms.At(j) if ilm.Scope().Name() != "" { @@ -154,6 +172,7 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri timestampMs: timestamp, logGroup: logGroup, logStream: logStream, + entity: &entity, metricDataType: metric.Type(), retainInitialValueForDelta: deltaInitialValue, }, @@ -169,6 +188,36 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri return nil } +func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { + entityFields := map[string]*string{ + keyAttributeEntityServiceName: nil, + keyAttributeEntityDeploymentEnvironment: nil, + attributeEntityASG: nil, + attributeEntityInstanceID: nil, + attributeEntityPlatformType: nil, + } + + for key := range entityFields { + if val, ok := resourceAttributes.Get(key); ok { + strVal := val.Str() + entityFields[key] = &strVal + resourceAttributes.Remove(key) + } + } + + return cloudwatchlogs.Entity{ + Attributes: map[string]*string{ + platformType: entityFields[attributeEntityPlatformType], + autoScalingGroup: entityFields[attributeEntityASG], + instanceIDTag: entityFields[attributeEntityInstanceID], + }, + KeyAttributes: map[string]*string{ + serviceName: entityFields[keyAttributeEntityServiceName], + deploymentEnvironment: entityFields[keyAttributeEntityDeploymentEnvironment], + }, + } +} + // translateGroupedMetricToCWMetric converts Grouped Metric format to CloudWatch Metric format. func translateGroupedMetricToCWMetric(groupedMetric *groupedMetric, config *Config) *cWMetrics { labels := groupedMetric.labels @@ -505,6 +554,7 @@ func translateGroupedMetricToEmf(groupedMetric *groupedMetric, config *Config, d logGroup := groupedMetric.metadata.logGroup logStream := groupedMetric.metadata.logStream + entity := groupedMetric.metadata.entity if logStream == "" { logStream = defaultLogStream @@ -512,6 +562,7 @@ func translateGroupedMetricToEmf(groupedMetric *groupedMetric, config *Config, d event.LogGroupName = logGroup event.LogStreamName = logStream + event.Entity = entity return event, nil } diff --git a/internal/aws/cwlogs/cwlog_client_test.go b/internal/aws/cwlogs/cwlog_client_test.go index 5e9fee1a42e1..2f63d96a8296 100644 --- a/internal/aws/cwlogs/cwlog_client_test.go +++ b/internal/aws/cwlogs/cwlog_client_test.go @@ -82,6 +82,18 @@ var previousSequenceToken = "0000" var expectedNextSequenceToken = "1111" var logGroup = "logGroup" var logStreamName = "logStream" + +var entity = cloudwatchlogs.Entity{ + Attributes: map[string]*string{ + "PlatformType": aws.String("AWS::EC2"), + "EC2.InstanceId": aws.String("i-123456789"), + "EC2.AutoScalingGroup": aws.String("test-group"), + }, + KeyAttributes: map[string]*string{ + "Name": aws.String("myService"), + "Environment": aws.String("myEnvironment"), + }, +} var emptySequenceToken = "" func TestPutLogEvents_HappyCase(t *testing.T) { @@ -91,6 +103,7 @@ func TestPutLogEvents_HappyCase(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &previousSequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ NextSequenceToken: &expectedNextSequenceToken} @@ -111,6 +124,7 @@ func TestPutLogEvents_HappyCase_SomeRejectedInfo(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &previousSequenceToken, + Entity: &entity, } rejectedLogEventsInfo := &cloudwatchlogs.RejectedLogEventsInfo{ ExpiredLogEventEndIndex: aws.Int64(1), @@ -137,6 +151,7 @@ func TestPutLogEvents_NonAWSError(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &previousSequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ NextSequenceToken: &expectedNextSequenceToken} @@ -157,6 +172,7 @@ func TestPutLogEvents_InvalidParameterException(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &previousSequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ NextSequenceToken: &expectedNextSequenceToken} @@ -178,6 +194,7 @@ func TestPutLogEvents_OperationAbortedException(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &previousSequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ NextSequenceToken: &expectedNextSequenceToken} @@ -199,6 +216,7 @@ func TestPutLogEvents_ServiceUnavailableException(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &previousSequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ NextSequenceToken: &expectedNextSequenceToken} @@ -220,6 +238,7 @@ func TestPutLogEvents_UnknownException(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &previousSequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ NextSequenceToken: &expectedNextSequenceToken} @@ -241,6 +260,7 @@ func TestPutLogEvents_ThrottlingException(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &previousSequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ NextSequenceToken: &expectedNextSequenceToken} @@ -262,6 +282,7 @@ func TestPutLogEvents_ResourceNotFoundException(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &emptySequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ @@ -289,6 +310,7 @@ func TestLogRetention_NeverExpire(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &emptySequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ @@ -324,6 +346,7 @@ func TestLogRetention_RetentionDaysInputted(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &emptySequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ @@ -360,6 +383,7 @@ func TestSetTags_NotCalled(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &emptySequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ @@ -395,6 +419,7 @@ func TestSetTags_Called(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &emptySequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ @@ -431,6 +456,7 @@ func TestPutLogEvents_AllRetriesFail(t *testing.T) { LogGroupName: &logGroup, LogStreamName: &logStreamName, SequenceToken: &emptySequenceToken, + Entity: &entity, } putLogEventsOutput := &cloudwatchlogs.PutLogEventsOutput{ diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index 929adcee00b6..4176d0b560d4 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -4,7 +4,10 @@ package cwlogs // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" import ( + "crypto/sha256" + "encoding/hex" "errors" + "fmt" "sort" "sync" "time" @@ -58,6 +61,20 @@ func NewEvent(timestampMs int64, message string) *Event { type StreamKey struct { LogGroupName string LogStreamName string + Entity *cloudwatchlogs.Entity +} + +// Custom hash function for StreamKey. Necessary to uniquely identify with Entity. +func (sk *StreamKey) Hash() string { + data := fmt.Sprintf( + "%s|%s|%s|%s", + sk.LogGroupName, + sk.LogStreamName, + mapToString(sk.Entity.Attributes), + mapToString(sk.Entity.KeyAttributes), + ) + hash := sha256.Sum256([]byte(data)) + return hex.EncodeToString(hash[:]) } func (logEvent *Event) Validate(logger *zap.Logger) error { @@ -186,6 +203,8 @@ type logPusher struct { logGroupName *string // log stream name of the current logPusher logStreamName *string + // entity name of the current logPusher + entity *cloudwatchlogs.Entity logEventBatch *eventBatch @@ -271,7 +290,8 @@ func (p *logPusher) pushEventBatch(req any) error { p.logger.Debug("logpusher: publish log events successfully.", zap.Int("NumOfLogEvents", len(putLogEventsInput.LogEvents)), zap.Float64("LogEventsSize", float64(logEventBatch.byteTotal)/float64(1024)), - zap.Int64("Time", time.Since(startTime).Nanoseconds()/int64(time.Millisecond))) + zap.Int64("Time", time.Since(startTime).Nanoseconds()/int64(time.Millisecond)), + zap.String("Entity", logEventBatch.putLogEventsInput.Entity.GoString())) return nil } @@ -288,6 +308,7 @@ func (p *logPusher) addLogEvent(logEvent *Event) *eventBatch { currentBatch = newEventBatch(StreamKey{ LogGroupName: *p.logGroupName, LogStreamName: *p.logStreamName, + Entity: p.entity, }) } currentBatch.append(logEvent) @@ -304,6 +325,7 @@ func (p *logPusher) renewEventBatch() *eventBatch { p.logEventBatch = newEventBatch(StreamKey{ LogGroupName: *p.logGroupName, LogStreamName: *p.logStreamName, + Entity: p.entity, }) } @@ -314,7 +336,7 @@ func (p *logPusher) renewEventBatch() *eventBatch { type multiStreamPusher struct { logStreamManager LogStreamManager client Client - pusherMap map[StreamKey]Pusher + pusherMap map[string]Pusher logger *zap.Logger } @@ -323,7 +345,7 @@ func newMultiStreamPusher(logStreamManager LogStreamManager, client Client, logg logStreamManager: logStreamManager, client: client, logger: logger, - pusherMap: make(map[StreamKey]Pusher), + pusherMap: make(map[string]Pusher), } } @@ -335,9 +357,9 @@ func (m *multiStreamPusher) AddLogEntry(event *Event) error { var pusher Pusher var ok bool - if pusher, ok = m.pusherMap[event.StreamKey]; !ok { + if pusher, ok = m.pusherMap[event.StreamKey.Hash()]; !ok { pusher = NewPusher(event.StreamKey, 1, m.client, m.logger) - m.pusherMap[event.StreamKey] = pusher + m.pusherMap[event.StreamKey.Hash()] = pusher } return pusher.AddLogEntry(event) diff --git a/internal/aws/cwlogs/pusher_test.go b/internal/aws/cwlogs/pusher_test.go index 2f1b7b97924e..e055346dfdf8 100644 --- a/internal/aws/cwlogs/pusher_test.go +++ b/internal/aws/cwlogs/pusher_test.go @@ -114,6 +114,7 @@ func newMockPusher() *logPusher { return newLogPusher(StreamKey{ LogGroupName: logGroup, LogStreamName: logStreamName, + Entity: &entity, }, *svc, zap.NewNop()) } diff --git a/internal/aws/cwlogs/utils.go b/internal/aws/cwlogs/utils.go index 1f1adc53960b..d91c607b1c12 100644 --- a/internal/aws/cwlogs/utils.go +++ b/internal/aws/cwlogs/utils.go @@ -7,6 +7,8 @@ import ( "errors" "fmt" "regexp" + "sort" + "strings" ) // Added function to check if value is an accepted number of log retention days @@ -68,3 +70,16 @@ func ValidateTagsInput(input map[string]*string) error { return nil } + +// Helper function to convert a map to a consistent string representation +func mapToString(m map[string]*string) string { + if m == nil { + return "" + } + var pairs []string + for k, v := range m { + pairs = append(pairs, fmt.Sprintf("%s:%s", k, *v)) + } + sort.Strings(pairs) // Ensure a consistent order + return strings.Join(pairs, ";") +} From 83df5a3bfb2f272f01b6d9bc4bd88d916520edb0 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 30 Aug 2024 12:39:50 -0400 Subject: [PATCH 03/45] adds debug statements --- ec2-user@ip-172-31-75-235.ec2.internal | 151 ++++++++++++++++++ exporter/awsemfexporter/emf_exporter.go | 19 ++- exporter/awsemfexporter/emf_exporter_test.go | 28 +++- exporter/awsemfexporter/grouped_metric.go | 9 ++ exporter/awsemfexporter/metric_translator.go | 8 +- .../awsemfexporter/metric_translator_test.go | 1 + internal/aws/cwlogs/pusher.go | 40 +++-- internal/aws/cwlogs/utils.go | 6 +- 8 files changed, 241 insertions(+), 21 deletions(-) create mode 100644 ec2-user@ip-172-31-75-235.ec2.internal diff --git a/ec2-user@ip-172-31-75-235.ec2.internal b/ec2-user@ip-172-31-75-235.ec2.internal new file mode 100644 index 000000000000..a9e24d89f314 --- /dev/null +++ b/ec2-user@ip-172-31-75-235.ec2.internal @@ -0,0 +1,151 @@ +From ba53a4a8662d9636563487ef476652d30bb9a1c3 Mon Sep 17 00:00:00 2001 +From: Dinakar Chappa +Date: Thu, 22 Aug 2024 16:44:04 -0400 +Subject: [PATCH] adds sdk changes to cw logs components + +--- + exporter/awscloudwatchlogsexporter/config.go | 4 ++++ + exporter/awscloudwatchlogsexporter/exporter.go | 2 +- + exporter/awscloudwatchlogsexporter/exporter_test.go | 2 +- + internal/aws/cwlogs/cwlog_client.go | 4 ++-- + internal/aws/cwlogs/cwlog_client_test.go | 5 +++-- + internal/aws/cwlogs/pusher.go | 3 ++- + internal/aws/cwlogs/pusher_test.go | 3 ++- + 7 files changed, 15 insertions(+), 8 deletions(-) + +diff --git a/exporter/awscloudwatchlogsexporter/config.go b/exporter/awscloudwatchlogsexporter/config.go +index 915003e993..1888e45c58 100644 +--- a/exporter/awscloudwatchlogsexporter/config.go ++++ b/exporter/awscloudwatchlogsexporter/config.go +@@ -12,6 +12,7 @@ import ( + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/awsutil" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" ++ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" + ) + + // Config represent a configuration for the CloudWatch logs exporter. +@@ -26,6 +27,9 @@ type Config struct { + // that share the same source. + LogStreamName string `mapstructure:"log_stream_name"` + ++ //Entity is the name of the service identifier of the CloudWatch log stream ++ Entity cloudwatchlogs.Entity ++ + // Endpoint is the CloudWatch Logs service endpoint which the requests + // are forwarded to. https://docs.aws.amazon.com/general/latest/gr/cwl_region.html + // e.g. logs.us-east-1.amazonaws.com +diff --git a/exporter/awscloudwatchlogsexporter/exporter.go b/exporter/awscloudwatchlogsexporter/exporter.go +index f58f1ca932..0837fad539 100644 +--- a/exporter/awscloudwatchlogsexporter/exporter.go ++++ b/exporter/awscloudwatchlogsexporter/exporter.go +@@ -13,7 +13,6 @@ import ( + + "github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware" + "github.com/aws/aws-sdk-go/aws" +- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" + "github.com/google/uuid" + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/consumer" +@@ -25,6 +24,7 @@ import ( + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/awsutil" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" ++ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" + ) + + type cwlExporter struct { +diff --git a/exporter/awscloudwatchlogsexporter/exporter_test.go b/exporter/awscloudwatchlogsexporter/exporter_test.go +index 950ee1060b..3cf3fa599f 100644 +--- a/exporter/awscloudwatchlogsexporter/exporter_test.go ++++ b/exporter/awscloudwatchlogsexporter/exporter_test.go +@@ -12,7 +12,6 @@ import ( + + "github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware" + "github.com/aws/aws-sdk-go/aws" +- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +@@ -22,6 +21,7 @@ import ( + "go.opentelemetry.io/collector/pdata/plog" + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" ++ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" + ) + + func init() { +diff --git a/internal/aws/cwlogs/cwlog_client.go b/internal/aws/cwlogs/cwlog_client.go +index e5df33723c..c423f060d9 100644 +--- a/internal/aws/cwlogs/cwlog_client.go ++++ b/internal/aws/cwlogs/cwlog_client.go +@@ -12,12 +12,12 @@ import ( + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/aws/request" + "github.com/aws/aws-sdk-go/aws/session" +- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" +- "github.com/aws/aws-sdk-go/service/cloudwatchlogs/cloudwatchlogsiface" + "go.opentelemetry.io/collector/component" + "go.uber.org/zap" + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/handler" ++ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" ++ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs/cloudwatchlogsiface" + ) + + const ( +diff --git a/internal/aws/cwlogs/cwlog_client_test.go b/internal/aws/cwlogs/cwlog_client_test.go +index 1f8c318378..5e9fee1a42 100644 +--- a/internal/aws/cwlogs/cwlog_client_test.go ++++ b/internal/aws/cwlogs/cwlog_client_test.go +@@ -13,12 +13,13 @@ import ( + "github.com/aws/aws-sdk-go/aws/client/metadata" + "github.com/aws/aws-sdk-go/aws/request" + "github.com/aws/aws-sdk-go/aws/session" +- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" +- "github.com/aws/aws-sdk-go/service/cloudwatchlogs/cloudwatchlogsiface" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "go.opentelemetry.io/collector/component" + "go.uber.org/zap" ++ ++ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" ++ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs/cloudwatchlogsiface" + ) + + func newAlwaysPassMockLogClient(putLogEventsFunc func(args mock.Arguments)) *Client { +diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go +index cee16a6941..929adcee00 100644 +--- a/internal/aws/cwlogs/pusher.go ++++ b/internal/aws/cwlogs/pusher.go +@@ -10,8 +10,9 @@ import ( + "time" + + "github.com/aws/aws-sdk-go/aws" +- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" + "go.uber.org/zap" ++ ++ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" + ) + + const ( +diff --git a/internal/aws/cwlogs/pusher_test.go b/internal/aws/cwlogs/pusher_test.go +index 9b43697985..2f1b7b9792 100644 +--- a/internal/aws/cwlogs/pusher_test.go ++++ b/internal/aws/cwlogs/pusher_test.go +@@ -11,10 +11,11 @@ import ( + "time" + + "github.com/aws/aws-sdk-go/aws" +- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "go.uber.org/zap" ++ ++ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" + ) + + // logEvent Tests +-- +2.39.3 (Apple Git-145) + diff --git a/exporter/awsemfexporter/emf_exporter.go b/exporter/awsemfexporter/emf_exporter.go index c05eb3e19075..b40aca6b8f0e 100644 --- a/exporter/awsemfexporter/emf_exporter.go +++ b/exporter/awsemfexporter/emf_exporter.go @@ -132,7 +132,9 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e for _, groupedMetric := range groupedMetrics { putLogEvent, err := translateGroupedMetricToEmf(groupedMetric, emf.config, defaultLogStream) + emf.config.logger.Info("putLogEvent", zap.Any("putLogEvent", putLogEvent)) if err != nil { + emf.config.logger.Error("error with translating grouped metric to emf") if errors.Is(err, errMissingMetricsForEnhancedContainerInsights) { emf.config.logger.Debug("Dropping empty putLogEvents for enhanced container insights", zap.Error(err)) continue @@ -152,16 +154,20 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e if emfPusher != nil { returnError := emfPusher.AddLogEntry(putLogEvent) if returnError != nil { + emf.config.logger.Error("could not get the emf pusher", zap.Error(returnError)) return wrapErrorIfBadRequest(returnError) } + emf.config.logger.Info("did not crash when adding the log entry!") } } } if strings.EqualFold(outputDestination, outputDestinationCloudWatch) { - for _, emfPusher := range emf.listPushers() { + for num, emfPusher := range emf.listPushers() { + emf.config.logger.Info("force flushing emfpusher #" + string(rune(num))) returnError := emfPusher.ForceFlush() if returnError != nil { + emf.config.logger.Info("error with force flushing") // TODO now we only have one logPusher, so it's ok to return after first error occurred err := wrapErrorIfBadRequest(returnError) if err != nil { @@ -169,20 +175,23 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e } return err } + emf.config.logger.Info("succeeded in force flushing") } } - emf.config.logger.Debug("Finish processing resource metrics", zap.Any("labels", labels)) + emf.config.logger.Info("Finish processing resource metrics", zap.Any("labels", labels)) return nil } func (emf *emfExporter) getPusher(key cwlogs.StreamKey) cwlogs.Pusher { var ok bool - if _, ok = emf.pusherMap[key.Hash()]; !ok { - emf.pusherMap[key.Hash()] = cwlogs.NewPusher(key, emf.retryCnt, *emf.svcStructuredLog, emf.config.logger) + hash := key.Hash() + if _, ok = emf.pusherMap[hash]; !ok { + emf.pusherMap[hash] = cwlogs.NewPusher(key, emf.retryCnt, *emf.svcStructuredLog, emf.config.logger) } - return emf.pusherMap[key.Hash()] + emf.config.logger.Info("The hash is " + hash) + return emf.pusherMap[hash] } func (emf *emfExporter) listPushers() []cwlogs.Pusher { diff --git a/exporter/awsemfexporter/emf_exporter_test.go b/exporter/awsemfexporter/emf_exporter_test.go index 7e64de02c4bc..d491b1ce799c 100644 --- a/exporter/awsemfexporter/emf_exporter_test.go +++ b/exporter/awsemfexporter/emf_exporter_test.go @@ -209,9 +209,17 @@ func TestConsumeMetricsWithLogGroupStreamConfig(t *testing.T) { streamKey := &cwlogs.StreamKey{ LogGroupName: expCfg.LogGroupName, LogStreamName: expCfg.LogStreamName, - Entity: &entity, + Entity: &cloudwatchlogs.Entity{Attributes: map[string]*string{ + "PlatformType": nil, + "InstanceId": nil, + "AutoScalingGroup": nil, + }, KeyAttributes: map[string]*string{ + "Name": nil, + "Environment": nil, + }}, } - pusherMap, ok := exp.pusherMap[streamKey.Hash()] + streamKeyHash := streamKey.Hash() + pusherMap, ok := exp.pusherMap[streamKeyHash] assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -242,6 +250,14 @@ func TestConsumeMetricsWithLogGroupStreamValidPlaceholder(t *testing.T) { streamKey := &cwlogs.StreamKey{ LogGroupName: "/aws/ecs/containerinsights/test-cluster-name/performance", LogStreamName: "test-task-id", + Entity: &cloudwatchlogs.Entity{Attributes: map[string]*string{ + "PlatformType": nil, + "InstanceId": nil, + "AutoScalingGroup": nil, + }, KeyAttributes: map[string]*string{ + "Name": nil, + "Environment": nil, + }}, } pusherMap, ok := exp.pusherMap[streamKey.Hash()] assert.True(t, ok) @@ -323,6 +339,14 @@ func TestConsumeMetricsWithWrongPlaceholder(t *testing.T) { streamKey := cwlogs.StreamKey{ LogGroupName: expCfg.LogGroupName, LogStreamName: expCfg.LogStreamName, + Entity: &cloudwatchlogs.Entity{Attributes: map[string]*string{ + "PlatformType": nil, + "InstanceId": nil, + "AutoScalingGroup": nil, + }, KeyAttributes: map[string]*string{ + "Name": nil, + "Environment": nil, + }}, } pusherMap, ok := exp.pusherMap[streamKey.Hash()] assert.True(t, ok) diff --git a/exporter/awsemfexporter/grouped_metric.go b/exporter/awsemfexporter/grouped_metric.go index dd3426cdc2e9..37cfa94a311e 100644 --- a/exporter/awsemfexporter/grouped_metric.go +++ b/exporter/awsemfexporter/grouped_metric.go @@ -38,6 +38,7 @@ func addToGroupedMetric( ) error { dps := getDataPoints(pmd, metadata, config.logger) + config.logger.Info("got datapoints") if dps == nil || dps.Len() == 0 { return nil } @@ -53,11 +54,14 @@ func addToGroupedMetric( continue } dps, retained := dps.CalculateDeltaDatapoints(i, metadata.instrumentationScopeName, config.DetailedMetrics, calculators) + config.logger.Info("Calculated Delta Datapoints") if !retained { continue } + config.logger.Info("Retained") for _, dp := range dps { + config.logger.Info("Traversing through datapoints") labels := dp.labels if metricType, ok := labels["Type"]; ok { @@ -69,6 +73,7 @@ func addToGroupedMetric( // if patterns were found in config file and weren't replaced by resource attributes, replace those patterns with metric labels. // if patterns are provided for a valid key and that key doesn't exist in the resource attributes, it is replaced with `undefined`. if !patternReplaceSucceeded { + config.logger.Info("patternReplaceSucceeded is false") if strings.Contains(metadata.logGroup, "undefined") { metadata.logGroup, _ = replacePatterns(config.LogGroupName, labels, config.logger) } @@ -76,18 +81,22 @@ func addToGroupedMetric( metadata.logStream, _ = replacePatterns(config.LogStreamName, labels, config.logger) } } + config.logger.Info("replaced the patterns for lg and ls") metric := &metricInfo{ value: dp.value, unit: translateUnit(pmd, descriptor), } + config.logger.Info("created the metric") if dp.timestampMs > 0 { metadata.timestampMs = dp.timestampMs } + config.logger.Info("set the timestamp") // Extra params to use when grouping metrics groupKey := aws.NewKey(metadata.groupedMetricMetadata, labels) + //config.logger.Info("entity in metadata key:" + groupKey.MetricMetadata.(cWMetricMetadata).entity.GoString()) if _, ok := groupedMetrics[groupKey]; ok { // if MetricName already exists in metrics map, print warning log if _, ok := groupedMetrics[groupKey].metrics[dp.name]; ok { diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 236b6ed123d8..037530c99680 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -156,6 +156,7 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri } entity := fetchEntityFields(resourceAttributes) + config.logger.Info("fetched entity:" + entity.GoString()) for j := 0; j < ilms.Len(); j++ { ilm := ilms.At(j) @@ -179,8 +180,10 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri instrumentationScopeName: instrumentationScopeName, receiver: metricReceiver, } + config.logger.Info("entity after metadata creation:" + metadata.groupedMetricMetadata.entity.GoString()) err := addToGroupedMetric(metric, groupedMetrics, metadata, patternReplaceSucceeded, mt.metricDescriptor, config, mt.calculators) if err != nil { + config.logger.Info("error with adding to grouped metric:" + err.Error()) return err } } @@ -432,7 +435,7 @@ func translateCWMetricToEMF(cWMetric *cWMetrics, config *Config) (*cwlogs.Event, var f any err := json.Unmarshal([]byte(val), &f) if err != nil { - config.logger.Debug( + config.logger.Info( "Failed to parse json-encoded string", zap.String("label key", key), zap.String("label value", val), @@ -517,6 +520,7 @@ func translateCWMetricToEMF(cWMetric *cWMetrics, config *Config) (*cwlogs.Event, pleMsg, err := json.Marshal(fieldMap) if err != nil { + config.logger.Error("error with marshalling the fieldMap: ", zap.Error(err)) return nil, err } @@ -524,6 +528,7 @@ func translateCWMetricToEMF(cWMetric *cWMetrics, config *Config) (*cwlogs.Event, if len(metricsMap) > 0 { metricsMsg, err := json.Marshal(metricsMap) if err != nil { + config.logger.Error("error with marshalling the metricsMap: ", zap.Error(err)) return nil, err } metricsMsg[0] = ',' @@ -545,6 +550,7 @@ func translateGroupedMetricToEmf(groupedMetric *groupedMetric, config *Config, d cWMetric := translateGroupedMetricToCWMetric(groupedMetric, config) event, err := translateCWMetricToEMF(cWMetric, config) if err != nil { + config.logger.Info("error with translating CW Metric to EMF", zap.Error(err)) return nil, err } // Drop a nil putLogEvent for EnhancedContainerInsights diff --git a/exporter/awsemfexporter/metric_translator_test.go b/exporter/awsemfexporter/metric_translator_test.go index 076bbf48ad72..60479c88a769 100644 --- a/exporter/awsemfexporter/metric_translator_test.go +++ b/exporter/awsemfexporter/metric_translator_test.go @@ -39,6 +39,7 @@ func createTestResourceMetricsHelper(numMetrics int) pmetric.ResourceMetrics { rm.Resource().Attributes().PutStr("ClusterName", "myCluster") rm.Resource().Attributes().PutStr("PodName", "myPod") rm.Resource().Attributes().PutStr(attributeReceiver, prometheusReceiver) + rm.Resource().Attributes().PutStr(keyAttributeEntityServiceName, "myServiceName") sm := rm.ScopeMetrics().AppendEmpty() m := sm.Metrics().AppendEmpty() diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index 4176d0b560d4..0794e5b732c3 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -4,8 +4,6 @@ package cwlogs // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" import ( - "crypto/sha256" - "encoding/hex" "errors" "fmt" "sort" @@ -66,15 +64,25 @@ type StreamKey struct { // Custom hash function for StreamKey. Necessary to uniquely identify with Entity. func (sk *StreamKey) Hash() string { + var attributes, keyAttributes string + if sk.Entity != nil { + if sk.Entity.Attributes != nil { + attributes = mapToString(sk.Entity.Attributes) + } + if sk.Entity.KeyAttributes != nil { + keyAttributes = mapToString(sk.Entity.KeyAttributes) + } + } + data := fmt.Sprintf( "%s|%s|%s|%s", sk.LogGroupName, sk.LogStreamName, - mapToString(sk.Entity.Attributes), - mapToString(sk.Entity.KeyAttributes), + attributes, + keyAttributes, ) - hash := sha256.Sum256([]byte(data)) - return hex.EncodeToString(hash[:]) + //hash := sha256.Sum256([]byte(data)) + return data } func (logEvent *Event) Validate(logger *zap.Logger) error { @@ -133,7 +141,9 @@ func newEventBatch(key StreamKey) *eventBatch { putLogEventsInput: &cloudwatchlogs.PutLogEventsInput{ LogGroupName: aws.String(key.LogGroupName), LogStreamName: aws.String(key.LogStreamName), - LogEvents: make([]*cloudwatchlogs.InputLogEvent, 0, maxRequestEventCount)}, + LogEvents: make([]*cloudwatchlogs.InputLogEvent, 0, maxRequestEventCount), + Entity: key.Entity, + }, } } @@ -262,7 +272,9 @@ func (p *logPusher) AddLogEntry(logEvent *Event) error { } func (p *logPusher) ForceFlush() error { + p.logger.Info("in force flush method for the log pusher") prevBatch := p.renewEventBatch() + p.logger.Info("renewedEventBatch. The entity here is " + prevBatch.putLogEventsInput.Entity.GoString()) if prevBatch != nil { return p.pushEventBatch(prevBatch) } @@ -320,13 +332,16 @@ func (p *logPusher) addLogEvent(logEvent *Event) *eventBatch { func (p *logPusher) renewEventBatch() *eventBatch { var prevBatch *eventBatch + p.logger.Info("renewing EventBatch, just before the if statement") if len(p.logEventBatch.putLogEventsInput.LogEvents) > 0 { + p.logger.Info("renewing EventBatch. The entity here is " + p.logEventBatch.putLogEventsInput.Entity.GoString()) prevBatch = p.logEventBatch p.logEventBatch = newEventBatch(StreamKey{ LogGroupName: *p.logGroupName, LogStreamName: *p.logStreamName, Entity: p.entity, }) + p.logger.Info("renewed EventBatch. The entity here is " + p.logEventBatch.putLogEventsInput.Entity.GoString()) } return prevBatch @@ -417,25 +432,26 @@ type LogStreamManager interface { type logStreamManager struct { logStreamMutex sync.Mutex - streams map[StreamKey]bool + streams map[string]bool client Client } func NewLogStreamManager(svcStructuredLog Client) LogStreamManager { return &logStreamManager{ client: svcStructuredLog, - streams: make(map[StreamKey]bool), + streams: make(map[string]bool), } } func (lsm *logStreamManager) InitStream(streamKey StreamKey) error { - if _, ok := lsm.streams[streamKey]; !ok { + hash := streamKey.Hash() + if _, ok := lsm.streams[hash]; !ok { lsm.logStreamMutex.Lock() defer lsm.logStreamMutex.Unlock() - if _, ok := lsm.streams[streamKey]; !ok { + if _, ok := lsm.streams[hash]; !ok { err := lsm.client.CreateStream(&streamKey.LogGroupName, &streamKey.LogStreamName) - lsm.streams[streamKey] = true + lsm.streams[hash] = true return err } } diff --git a/internal/aws/cwlogs/utils.go b/internal/aws/cwlogs/utils.go index d91c607b1c12..5b6cfb41a052 100644 --- a/internal/aws/cwlogs/utils.go +++ b/internal/aws/cwlogs/utils.go @@ -78,7 +78,11 @@ func mapToString(m map[string]*string) string { } var pairs []string for k, v := range m { - pairs = append(pairs, fmt.Sprintf("%s:%s", k, *v)) + if v == nil { + pairs = append(pairs, fmt.Sprintf("%s:", k)) + } else { + pairs = append(pairs, fmt.Sprintf("%s:%s", k, *v)) + } } sort.Strings(pairs) // Ensure a consistent order return strings.Join(pairs, ";") From d184f65068fce1f9d486567bbfb737991353e6e1 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 3 Sep 2024 17:33:28 -0400 Subject: [PATCH 04/45] adds more log lines in newEventBatch function --- internal/aws/cwlogs/pusher.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index 0794e5b732c3..233daa5734fd 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -136,7 +136,12 @@ type eventBatch struct { } // Create a new log event batch if needed. -func newEventBatch(key StreamKey) *eventBatch { +func newEventBatch(key StreamKey, logger *zap.Logger) *eventBatch { + logger.Info("in newEventBatch function") + logger.Info("logGroupName: " + key.LogGroupName) + logger.Info("logStreamName: " + key.LogStreamName) + logger.Info("entity: " + key.Entity.GoString()) + return &eventBatch{ putLogEventsInput: &cloudwatchlogs.PutLogEventsInput{ LogGroupName: aws.String(key.LogGroupName), @@ -340,7 +345,7 @@ func (p *logPusher) renewEventBatch() *eventBatch { LogGroupName: *p.logGroupName, LogStreamName: *p.logStreamName, Entity: p.entity, - }) + }, p.logger) p.logger.Info("renewed EventBatch. The entity here is " + p.logEventBatch.putLogEventsInput.Entity.GoString()) } From 9c159618f340f74146eca5dc9233ca59ba5479cc Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 3 Sep 2024 17:35:50 -0400 Subject: [PATCH 05/45] fixes other newEventBatch calls --- internal/aws/cwlogs/pusher.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index 233daa5734fd..4708b9a74f4c 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -250,7 +250,7 @@ func newLogPusher(streamKey StreamKey, svcStructuredLog: svcStructuredLog, logger: logger, } - pusher.logEventBatch = newEventBatch(streamKey) + pusher.logEventBatch = newEventBatch(streamKey, logger) return pusher } @@ -326,7 +326,7 @@ func (p *logPusher) addLogEvent(logEvent *Event) *eventBatch { LogGroupName: *p.logGroupName, LogStreamName: *p.logStreamName, Entity: p.entity, - }) + }, p.logger) } currentBatch.append(logEvent) p.logEventBatch = currentBatch From 525100f50b4420a0a05f6891eb46c3dbae186631 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 3 Sep 2024 17:56:09 -0400 Subject: [PATCH 06/45] adds type assertion logging to ensure type is the same when being sent --- internal/aws/cwlogs/pusher.go | 2 ++ internal/aws/cwlogs/pusher_test.go | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index 4708b9a74f4c..4543a857b584 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -140,6 +140,7 @@ func newEventBatch(key StreamKey, logger *zap.Logger) *eventBatch { logger.Info("in newEventBatch function") logger.Info("logGroupName: " + key.LogGroupName) logger.Info("logStreamName: " + key.LogStreamName) + logger.Info("p.entity type: " + fmt.Sprintf("%T", key.Entity)) logger.Info("entity: " + key.Entity.GoString()) return &eventBatch{ @@ -340,6 +341,7 @@ func (p *logPusher) renewEventBatch() *eventBatch { p.logger.Info("renewing EventBatch, just before the if statement") if len(p.logEventBatch.putLogEventsInput.LogEvents) > 0 { p.logger.Info("renewing EventBatch. The entity here is " + p.logEventBatch.putLogEventsInput.Entity.GoString()) + p.logger.Info("p.entity type: " + fmt.Sprintf("%T", p.entity)) prevBatch = p.logEventBatch p.logEventBatch = newEventBatch(StreamKey{ LogGroupName: *p.logGroupName, diff --git a/internal/aws/cwlogs/pusher_test.go b/internal/aws/cwlogs/pusher_test.go index e055346dfdf8..df32a762142b 100644 --- a/internal/aws/cwlogs/pusher_test.go +++ b/internal/aws/cwlogs/pusher_test.go @@ -131,7 +131,8 @@ func TestPusher_newLogEventBatch(t *testing.T) { logEventBatch := newEventBatch(StreamKey{ LogGroupName: logGroup, LogStreamName: logStreamName, - }) + Entity: &entity, + }, zap.NewExample()) assert.Equal(t, int64(0), logEventBatch.maxTimestampMs) assert.Equal(t, int64(0), logEventBatch.minTimestampMs) assert.Equal(t, 0, logEventBatch.byteTotal) From ec8e74e945c235d3ffd32082b45d534efcfc4029 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 3 Sep 2024 18:04:16 -0400 Subject: [PATCH 07/45] verifies current entity instead of previous --- internal/aws/cwlogs/pusher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index 4543a857b584..389aa22455f5 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -340,7 +340,7 @@ func (p *logPusher) renewEventBatch() *eventBatch { var prevBatch *eventBatch p.logger.Info("renewing EventBatch, just before the if statement") if len(p.logEventBatch.putLogEventsInput.LogEvents) > 0 { - p.logger.Info("renewing EventBatch. The entity here is " + p.logEventBatch.putLogEventsInput.Entity.GoString()) + p.logger.Info("renewing EventBatch. The entity here is " + p.entity.GoString()) p.logger.Info("p.entity type: " + fmt.Sprintf("%T", p.entity)) prevBatch = p.logEventBatch p.logEventBatch = newEventBatch(StreamKey{ From 36dbeacd62ddbd18863ddda2247ce42f9d80eeab Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 3 Sep 2024 18:12:19 -0400 Subject: [PATCH 08/45] adds entity when creating new log pusher --- exporter/awsemfexporter/emf_exporter.go | 1 + internal/aws/cwlogs/pusher.go | 1 + 2 files changed, 2 insertions(+) diff --git a/exporter/awsemfexporter/emf_exporter.go b/exporter/awsemfexporter/emf_exporter.go index b40aca6b8f0e..964704164877 100644 --- a/exporter/awsemfexporter/emf_exporter.go +++ b/exporter/awsemfexporter/emf_exporter.go @@ -165,6 +165,7 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e if strings.EqualFold(outputDestination, outputDestinationCloudWatch) { for num, emfPusher := range emf.listPushers() { emf.config.logger.Info("force flushing emfpusher #" + string(rune(num))) + emf.config.logger.Info("entity is ") returnError := emfPusher.ForceFlush() if returnError != nil { emf.config.logger.Info("error with force flushing") diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index 389aa22455f5..ea2da8f02f31 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -248,6 +248,7 @@ func newLogPusher(streamKey StreamKey, pusher := &logPusher{ logGroupName: aws.String(streamKey.LogGroupName), logStreamName: aws.String(streamKey.LogStreamName), + entity: streamKey.Entity, svcStructuredLog: svcStructuredLog, logger: logger, } From c4ca2a57694ba71ac57cd0654704da1424a4a23f Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 00:45:58 -0400 Subject: [PATCH 09/45] fills entity fields with certain known otel attributes --- exporter/awsemfexporter/metric_translator.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 037530c99680..a1c2b0bb5c91 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -210,13 +210,13 @@ func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { return cloudwatchlogs.Entity{ Attributes: map[string]*string{ - platformType: entityFields[attributeEntityPlatformType], + platformType: entityFields["host.type"], autoScalingGroup: entityFields[attributeEntityASG], - instanceIDTag: entityFields[attributeEntityInstanceID], + instanceIDTag: entityFields["host.name"], }, KeyAttributes: map[string]*string{ - serviceName: entityFields[keyAttributeEntityServiceName], - deploymentEnvironment: entityFields[keyAttributeEntityDeploymentEnvironment], + serviceName: entityFields["aws.entity.k8s.namespace.name"], + deploymentEnvironment: entityFields["aws.deployment.name"], }, } } From 9e883bd3cedbde04f1733b7f860c36d0df7ad876 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 01:06:15 -0400 Subject: [PATCH 10/45] log lines for resourceAttributes --- exporter/awsemfexporter/metric_translator.go | 1 + 1 file changed, 1 insertion(+) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index a1c2b0bb5c91..44cd4bfe58ed 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -155,6 +155,7 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri } } + config.logger.Info("resourceAttributes", zap.Any("resourceAttributes", resourceAttributes.AsRaw())) entity := fetchEntityFields(resourceAttributes) config.logger.Info("fetched entity:" + entity.GoString()) From 882eb116929c4162e20c554eb95b436421b61789 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 01:21:51 -0400 Subject: [PATCH 11/45] hardcodes otel attributes to ensure entity population works --- exporter/awsemfexporter/metric_translator.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 44cd4bfe58ed..6b7b919dcbe5 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -194,11 +194,11 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { entityFields := map[string]*string{ - keyAttributeEntityServiceName: nil, - keyAttributeEntityDeploymentEnvironment: nil, - attributeEntityASG: nil, - attributeEntityInstanceID: nil, - attributeEntityPlatformType: nil, + "aws.entity.k8s.namespace.name": nil, // Correct key for k8s namespace + "aws.deployment.name": nil, // Correct key for deployment name + "host.name": nil, // Correct key for instance ID (host.name) + "host.type": nil, // Correct key for platform type (host.type) + "attributeEntityASG": nil, // Adjust this key based on your actual key for ASG, if any } for key := range entityFields { From bbcf3bf2691e05d6be4301fd617d25f999e98890 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 01:28:15 -0400 Subject: [PATCH 12/45] temporarily comments out attribute removal --- exporter/awsemfexporter/metric_translator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 6b7b919dcbe5..399481c77b3d 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -205,7 +205,7 @@ func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { if val, ok := resourceAttributes.Get(key); ok { strVal := val.Str() entityFields[key] = &strVal - resourceAttributes.Remove(key) + //resourceAttributes.Remove(key) } } From fec89d23fe5093c935bc4e70ffc2ed6241bd88fa Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 01:34:14 -0400 Subject: [PATCH 13/45] removes asg temporarily --- exporter/awsemfexporter/metric_translator.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 399481c77b3d..fc21c6d3133a 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -211,9 +211,8 @@ func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { return cloudwatchlogs.Entity{ Attributes: map[string]*string{ - platformType: entityFields["host.type"], - autoScalingGroup: entityFields[attributeEntityASG], - instanceIDTag: entityFields["host.name"], + platformType: entityFields["host.type"], + instanceIDTag: entityFields["host.name"], }, KeyAttributes: map[string]*string{ serviceName: entityFields["aws.entity.k8s.namespace.name"], From 96bc334d9d42dbde0e65bc537d1a1e775fc67153 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 14:08:58 -0400 Subject: [PATCH 14/45] adds keyAttributes after changing test app_signals config --- ec2-user@ip-172-31-75-235.ec2.internal | 151 ------------------- exporter/awsemfexporter/metric_translator.go | 20 ++- 2 files changed, 9 insertions(+), 162 deletions(-) delete mode 100644 ec2-user@ip-172-31-75-235.ec2.internal diff --git a/ec2-user@ip-172-31-75-235.ec2.internal b/ec2-user@ip-172-31-75-235.ec2.internal deleted file mode 100644 index a9e24d89f314..000000000000 --- a/ec2-user@ip-172-31-75-235.ec2.internal +++ /dev/null @@ -1,151 +0,0 @@ -From ba53a4a8662d9636563487ef476652d30bb9a1c3 Mon Sep 17 00:00:00 2001 -From: Dinakar Chappa -Date: Thu, 22 Aug 2024 16:44:04 -0400 -Subject: [PATCH] adds sdk changes to cw logs components - ---- - exporter/awscloudwatchlogsexporter/config.go | 4 ++++ - exporter/awscloudwatchlogsexporter/exporter.go | 2 +- - exporter/awscloudwatchlogsexporter/exporter_test.go | 2 +- - internal/aws/cwlogs/cwlog_client.go | 4 ++-- - internal/aws/cwlogs/cwlog_client_test.go | 5 +++-- - internal/aws/cwlogs/pusher.go | 3 ++- - internal/aws/cwlogs/pusher_test.go | 3 ++- - 7 files changed, 15 insertions(+), 8 deletions(-) - -diff --git a/exporter/awscloudwatchlogsexporter/config.go b/exporter/awscloudwatchlogsexporter/config.go -index 915003e993..1888e45c58 100644 ---- a/exporter/awscloudwatchlogsexporter/config.go -+++ b/exporter/awscloudwatchlogsexporter/config.go -@@ -12,6 +12,7 @@ import ( - - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/awsutil" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" -+ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" - ) - - // Config represent a configuration for the CloudWatch logs exporter. -@@ -26,6 +27,9 @@ type Config struct { - // that share the same source. - LogStreamName string `mapstructure:"log_stream_name"` - -+ //Entity is the name of the service identifier of the CloudWatch log stream -+ Entity cloudwatchlogs.Entity -+ - // Endpoint is the CloudWatch Logs service endpoint which the requests - // are forwarded to. https://docs.aws.amazon.com/general/latest/gr/cwl_region.html - // e.g. logs.us-east-1.amazonaws.com -diff --git a/exporter/awscloudwatchlogsexporter/exporter.go b/exporter/awscloudwatchlogsexporter/exporter.go -index f58f1ca932..0837fad539 100644 ---- a/exporter/awscloudwatchlogsexporter/exporter.go -+++ b/exporter/awscloudwatchlogsexporter/exporter.go -@@ -13,7 +13,6 @@ import ( - - "github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware" - "github.com/aws/aws-sdk-go/aws" -- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" - "github.com/google/uuid" - "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/consumer" -@@ -25,6 +24,7 @@ import ( - - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/awsutil" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" -+ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" - ) - - type cwlExporter struct { -diff --git a/exporter/awscloudwatchlogsexporter/exporter_test.go b/exporter/awscloudwatchlogsexporter/exporter_test.go -index 950ee1060b..3cf3fa599f 100644 ---- a/exporter/awscloudwatchlogsexporter/exporter_test.go -+++ b/exporter/awscloudwatchlogsexporter/exporter_test.go -@@ -12,7 +12,6 @@ import ( - - "github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware" - "github.com/aws/aws-sdk-go/aws" -- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" -@@ -22,6 +21,7 @@ import ( - "go.opentelemetry.io/collector/pdata/plog" - - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" -+ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" - ) - - func init() { -diff --git a/internal/aws/cwlogs/cwlog_client.go b/internal/aws/cwlogs/cwlog_client.go -index e5df33723c..c423f060d9 100644 ---- a/internal/aws/cwlogs/cwlog_client.go -+++ b/internal/aws/cwlogs/cwlog_client.go -@@ -12,12 +12,12 @@ import ( - "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/aws/aws-sdk-go/aws/request" - "github.com/aws/aws-sdk-go/aws/session" -- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" -- "github.com/aws/aws-sdk-go/service/cloudwatchlogs/cloudwatchlogsiface" - "go.opentelemetry.io/collector/component" - "go.uber.org/zap" - - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/handler" -+ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" -+ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs/cloudwatchlogsiface" - ) - - const ( -diff --git a/internal/aws/cwlogs/cwlog_client_test.go b/internal/aws/cwlogs/cwlog_client_test.go -index 1f8c318378..5e9fee1a42 100644 ---- a/internal/aws/cwlogs/cwlog_client_test.go -+++ b/internal/aws/cwlogs/cwlog_client_test.go -@@ -13,12 +13,13 @@ import ( - "github.com/aws/aws-sdk-go/aws/client/metadata" - "github.com/aws/aws-sdk-go/aws/request" - "github.com/aws/aws-sdk-go/aws/session" -- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" -- "github.com/aws/aws-sdk-go/service/cloudwatchlogs/cloudwatchlogsiface" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "go.opentelemetry.io/collector/component" - "go.uber.org/zap" -+ -+ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" -+ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs/cloudwatchlogsiface" - ) - - func newAlwaysPassMockLogClient(putLogEventsFunc func(args mock.Arguments)) *Client { -diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go -index cee16a6941..929adcee00 100644 ---- a/internal/aws/cwlogs/pusher.go -+++ b/internal/aws/cwlogs/pusher.go -@@ -10,8 +10,9 @@ import ( - "time" - - "github.com/aws/aws-sdk-go/aws" -- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" - "go.uber.org/zap" -+ -+ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" - ) - - const ( -diff --git a/internal/aws/cwlogs/pusher_test.go b/internal/aws/cwlogs/pusher_test.go -index 9b43697985..2f1b7b9792 100644 ---- a/internal/aws/cwlogs/pusher_test.go -+++ b/internal/aws/cwlogs/pusher_test.go -@@ -11,10 +11,11 @@ import ( - "time" - - "github.com/aws/aws-sdk-go/aws" -- "github.com/aws/aws-sdk-go/service/cloudwatchlogs" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "go.uber.org/zap" -+ -+ "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" - ) - - // logEvent Tests --- -2.39.3 (Apple Git-145) - diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index fc21c6d3133a..78b68e8607c6 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -194,11 +194,13 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { entityFields := map[string]*string{ - "aws.entity.k8s.namespace.name": nil, // Correct key for k8s namespace - "aws.deployment.name": nil, // Correct key for deployment name - "host.name": nil, // Correct key for instance ID (host.name) - "host.type": nil, // Correct key for platform type (host.type) - "attributeEntityASG": nil, // Adjust this key based on your actual key for ASG, if any + "aws.entity.k8s.namespace.name": nil, // Correct key for k8s namespace + "aws.deployment.name": nil, // Correct key for deployment name + "host.name": nil, // Correct key for instance ID (host.name) + "host.type": nil, // Correct key for platform type (host.type) + "attributeEntityASG": nil, // Adjust this key based on your actual key for ASG, if any + keyAttributeEntityServiceName: nil, + keyAttributeEntityDeploymentEnvironment: nil, } for key := range entityFields { @@ -210,13 +212,9 @@ func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { } return cloudwatchlogs.Entity{ - Attributes: map[string]*string{ - platformType: entityFields["host.type"], - instanceIDTag: entityFields["host.name"], - }, KeyAttributes: map[string]*string{ - serviceName: entityFields["aws.entity.k8s.namespace.name"], - deploymentEnvironment: entityFields["aws.deployment.name"], + serviceName: entityFields[keyAttributeEntityServiceName], + deploymentEnvironment: entityFields[keyAttributeEntityDeploymentEnvironment], }, } } From b4f0e3d0261df8c251ad333e4097bf75fe6fb7e0 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 14:43:53 -0400 Subject: [PATCH 15/45] reimplement removal of the resource attributes --- exporter/awsemfexporter/metric_translator.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 78b68e8607c6..a1a114dc9af8 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -194,11 +194,6 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { entityFields := map[string]*string{ - "aws.entity.k8s.namespace.name": nil, // Correct key for k8s namespace - "aws.deployment.name": nil, // Correct key for deployment name - "host.name": nil, // Correct key for instance ID (host.name) - "host.type": nil, // Correct key for platform type (host.type) - "attributeEntityASG": nil, // Adjust this key based on your actual key for ASG, if any keyAttributeEntityServiceName: nil, keyAttributeEntityDeploymentEnvironment: nil, } @@ -207,7 +202,7 @@ func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { if val, ok := resourceAttributes.Get(key); ok { strVal := val.Str() entityFields[key] = &strVal - //resourceAttributes.Remove(key) + resourceAttributes.Remove(key) } } From 345ea32f748971e5870db12b3fee33b8dabed307 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 15:13:15 -0400 Subject: [PATCH 16/45] refactors fetching of entity fields --- exporter/awsemfexporter/metric_translator.go | 34 ++++++++++++-------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index a1a114dc9af8..b60b4546755d 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/aws/aws-sdk-go/aws" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" "go.opentelemetry.io/collector/pdata/pcommon" "reflect" @@ -18,7 +19,7 @@ import ( "go.uber.org/zap" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" - aws "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/metrics" + awsmetrics "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/metrics" ) const ( @@ -41,6 +42,8 @@ const ( keyAttributeEntityDeploymentEnvironment = "aws.entity.deployment.environment" deploymentEnvironment = "Environment" attributeEntityPlatformType = "aws.entity.platform.type" + entityType = "Type" + service = "Service" platformType = "PlatformType" attributeEntityASG = "aws.entity.autoscalegroup" autoScalingGroup = "AutoScalingGroup" @@ -48,6 +51,10 @@ const ( instanceIDTag = "InstanceId" ) +var keyAttributeEntityFields = []string{ + keyAttributeEntityServiceName, + keyAttributeEntityDeploymentEnvironment, +} var errMissingMetricsForEnhancedContainerInsights = errors.New("nil event detected with EnhancedContainerInsights enabled") var fieldPrometheusTypes = map[pmetric.MetricType]string{ @@ -118,8 +125,8 @@ func newMetricTranslator(config Config) metricTranslator { return metricTranslator{ metricDescriptor: mt, calculators: &emfCalculators{ - delta: aws.NewFloat64DeltaCalculator(), - summary: aws.NewMetricCalculator(calculateSummaryDelta), + delta: awsmetrics.NewFloat64DeltaCalculator(), + summary: awsmetrics.NewMetricCalculator(calculateSummaryDelta), }, } } @@ -142,11 +149,11 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri ilms := rm.ScopeMetrics() var metricReceiver string - if receiver, ok := resourceAttributes.Get(attributeReceiver); ok { + if receiver, ok := rm.Resource().Attributes().Get(attributeReceiver); ok { metricReceiver = receiver.Str() } - if serviceName, ok := resourceAttributes.Get("service.name"); ok { + if serviceName, ok := rm.Resource().Attributes().Get("service.name"); ok { if strings.HasPrefix(serviceName.Str(), "containerInsightsKubeAPIServerScraper") || strings.HasPrefix(serviceName.Str(), "containerInsightsDCGMExporterScraper") || strings.HasPrefix(serviceName.Str(), "containerInsightsNeuronMonitorScraper") { @@ -156,7 +163,7 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri } config.logger.Info("resourceAttributes", zap.Any("resourceAttributes", resourceAttributes.AsRaw())) - entity := fetchEntityFields(resourceAttributes) + entity := fetchEntityFields(rm.Resource().Attributes()) config.logger.Info("fetched entity:" + entity.GoString()) for j := 0; j < ilms.Len(); j++ { @@ -193,23 +200,24 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri } func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { - entityFields := map[string]*string{ - keyAttributeEntityServiceName: nil, - keyAttributeEntityDeploymentEnvironment: nil, + serviceKeyAttr := map[string]*string{ + entityType: aws.String(service), } - for key := range entityFields { + for _, key := range keyAttributeEntityFields { if val, ok := resourceAttributes.Get(key); ok { strVal := val.Str() - entityFields[key] = &strVal + if strVal != "" { + serviceKeyAttr[key] = aws.String(strVal) + } resourceAttributes.Remove(key) } } return cloudwatchlogs.Entity{ KeyAttributes: map[string]*string{ - serviceName: entityFields[keyAttributeEntityServiceName], - deploymentEnvironment: entityFields[keyAttributeEntityDeploymentEnvironment], + serviceName: serviceKeyAttr[keyAttributeEntityServiceName], + deploymentEnvironment: serviceKeyAttr[keyAttributeEntityDeploymentEnvironment], }, } } From 57a595353174e1655abf82d5fdfadb08b2f06e9d Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 17:15:45 -0400 Subject: [PATCH 17/45] adds resourceMutex for concurrency issue to the resource Attributes --- exporter/awsemfexporter/metric_translator.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index b60b4546755d..a001dfc90a66 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -12,6 +12,7 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "reflect" "strings" + "sync" "time" "go.opentelemetry.io/collector/pdata/pmetric" @@ -51,6 +52,8 @@ const ( instanceIDTag = "InstanceId" ) +var resourceMutex sync.Mutex + var keyAttributeEntityFields = []string{ keyAttributeEntityServiceName, keyAttributeEntityDeploymentEnvironment, @@ -203,6 +206,8 @@ func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { serviceKeyAttr := map[string]*string{ entityType: aws.String(service), } + resourceMutex.Lock() + defer resourceMutex.Unlock() for _, key := range keyAttributeEntityFields { if val, ok := resourceAttributes.Get(key); ok { @@ -215,10 +220,7 @@ func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { } return cloudwatchlogs.Entity{ - KeyAttributes: map[string]*string{ - serviceName: serviceKeyAttr[keyAttributeEntityServiceName], - deploymentEnvironment: serviceKeyAttr[keyAttributeEntityDeploymentEnvironment], - }, + KeyAttributes: serviceKeyAttr, } } From 7272016cbc76adf43d087302208a3effdada3e5b Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 18:18:53 -0400 Subject: [PATCH 18/45] copies atrribute map over to a new map to enable mutations --- exporter/awsemfexporter/metric_translator.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index a001dfc90a66..14bec0ca6792 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -166,7 +166,7 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri } config.logger.Info("resourceAttributes", zap.Any("resourceAttributes", resourceAttributes.AsRaw())) - entity := fetchEntityFields(rm.Resource().Attributes()) + resourceAttributes, entity := fetchEntityFields(resourceAttributes) config.logger.Info("fetched entity:" + entity.GoString()) for j := 0; j < ilms.Len(); j++ { @@ -202,26 +202,26 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri return nil } -func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { +func fetchEntityFields(resourceAttributes pcommon.Map) (cloudwatchlogs.Entity, pcommon.Map) { + mutableResourceAttributes := pcommon.NewMap() + resourceAttributes.CopyTo(mutableResourceAttributes) serviceKeyAttr := map[string]*string{ entityType: aws.String(service), } - resourceMutex.Lock() - defer resourceMutex.Unlock() for _, key := range keyAttributeEntityFields { - if val, ok := resourceAttributes.Get(key); ok { + if val, ok := mutableResourceAttributes.Get(key); ok { strVal := val.Str() if strVal != "" { serviceKeyAttr[key] = aws.String(strVal) } - resourceAttributes.Remove(key) + mutableResourceAttributes.Remove(key) } } return cloudwatchlogs.Entity{ KeyAttributes: serviceKeyAttr, - } + }, mutableResourceAttributes } // translateGroupedMetricToCWMetric converts Grouped Metric format to CloudWatch Metric format. From f9e02bbf2a774401d6756e2e039f1382cd6f4be4 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 18:20:30 -0400 Subject: [PATCH 19/45] fix for assigning variables --- exporter/awsemfexporter/metric_translator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 14bec0ca6792..f404a3a839df 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -166,7 +166,7 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri } config.logger.Info("resourceAttributes", zap.Any("resourceAttributes", resourceAttributes.AsRaw())) - resourceAttributes, entity := fetchEntityFields(resourceAttributes) + entity, resourceAttributes := fetchEntityFields(resourceAttributes) config.logger.Info("fetched entity:" + entity.GoString()) for j := 0; j < ilms.Len(); j++ { From 5e2a2952ecf76368fae0b2c1ca0f9bc14a4c902a Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 18:57:16 -0400 Subject: [PATCH 20/45] addresses request size error with PLE --- exporter/awsemfexporter/metric_translator.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index f404a3a839df..4b412e289edb 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -58,6 +58,12 @@ var keyAttributeEntityFields = []string{ keyAttributeEntityServiceName, keyAttributeEntityDeploymentEnvironment, } + +var keyAttributeEntityToShortNameMap = map[string]string{ + keyAttributeEntityServiceName: serviceName, + keyAttributeEntityDeploymentEnvironment: deploymentEnvironment, +} + var errMissingMetricsForEnhancedContainerInsights = errors.New("nil event detected with EnhancedContainerInsights enabled") var fieldPrometheusTypes = map[pmetric.MetricType]string{ @@ -209,13 +215,13 @@ func fetchEntityFields(resourceAttributes pcommon.Map) (cloudwatchlogs.Entity, p entityType: aws.String(service), } - for _, key := range keyAttributeEntityFields { - if val, ok := mutableResourceAttributes.Get(key); ok { + for entityField, shortName := range keyAttributeEntityToShortNameMap { + if val, ok := mutableResourceAttributes.Get(entityField); ok { strVal := val.Str() if strVal != "" { - serviceKeyAttr[key] = aws.String(strVal) + serviceKeyAttr[shortName] = aws.String(strVal) } - mutableResourceAttributes.Remove(key) + mutableResourceAttributes.Remove(entityField) } } From 3beae3cc0aa53e734b1a59149bb68a0086689ac9 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 19:20:53 -0400 Subject: [PATCH 21/45] adds normal attributes, refactors --- exporter/awsemfexporter/emf_exporter_test.go | 3 -- exporter/awsemfexporter/metric_translator.go | 53 +++++++++++--------- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/exporter/awsemfexporter/emf_exporter_test.go b/exporter/awsemfexporter/emf_exporter_test.go index d491b1ce799c..d3584b169c9b 100644 --- a/exporter/awsemfexporter/emf_exporter_test.go +++ b/exporter/awsemfexporter/emf_exporter_test.go @@ -296,9 +296,6 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { "aws.ecs.task.id": "test-task-id", keyAttributeEntityServiceName: "myService", keyAttributeEntityDeploymentEnvironment: "myEnvironment", - attributeEntityPlatformType: "AWS::EC2", - attributeEntityASG: "test-group", - attributeEntityInstanceID: "i-123456789", }, }) require.Error(t, exp.pushMetricsData(ctx, md)) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 4b412e289edb..b95d3c82b91a 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -12,7 +12,6 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "reflect" "strings" - "sync" "time" "go.opentelemetry.io/collector/pdata/pmetric" @@ -42,28 +41,30 @@ const ( serviceName = "Name" keyAttributeEntityDeploymentEnvironment = "aws.entity.deployment.environment" deploymentEnvironment = "Environment" - attributeEntityPlatformType = "aws.entity.platform.type" entityType = "Type" service = "Service" - platformType = "PlatformType" - attributeEntityASG = "aws.entity.autoscalegroup" - autoScalingGroup = "AutoScalingGroup" - attributeEntityInstanceID = "aws.entity.instance.id" - instanceIDTag = "InstanceId" + AttributeEntityCluster = "aws.entity.k8s.cluster.name" + cluster = "Cluster" + AttributeEntityNamespace = "aws.entity.k8s.namespace.name" + namespace = "Namespace" + AttributeEntityWorkload = "aws.entity.k8s.workload.name" + workload = "Workload" + AttributeEntityNode = "aws.entity.k8s.node.name" + node = "Node" ) -var resourceMutex sync.Mutex - -var keyAttributeEntityFields = []string{ - keyAttributeEntityServiceName, - keyAttributeEntityDeploymentEnvironment, -} - var keyAttributeEntityToShortNameMap = map[string]string{ keyAttributeEntityServiceName: serviceName, keyAttributeEntityDeploymentEnvironment: deploymentEnvironment, } +var attributeEntityToShortNameMap = map[string]string{ + AttributeEntityCluster: cluster, + AttributeEntityNamespace: deploymentEnvironment, + AttributeEntityWorkload: workload, + AttributeEntityNode: node, +} + var errMissingMetricsForEnhancedContainerInsights = errors.New("nil event detected with EnhancedContainerInsights enabled") var fieldPrometheusTypes = map[pmetric.MetricType]string{ @@ -214,19 +215,14 @@ func fetchEntityFields(resourceAttributes pcommon.Map) (cloudwatchlogs.Entity, p serviceKeyAttr := map[string]*string{ entityType: aws.String(service), } + attributeMap := map[string]*string{} - for entityField, shortName := range keyAttributeEntityToShortNameMap { - if val, ok := mutableResourceAttributes.Get(entityField); ok { - strVal := val.Str() - if strVal != "" { - serviceKeyAttr[shortName] = aws.String(strVal) - } - mutableResourceAttributes.Remove(entityField) - } - } + processAttributes(keyAttributeEntityToShortNameMap, serviceKeyAttr) + processAttributes(attributeEntityToShortNameMap, attributeMap) return cloudwatchlogs.Entity{ KeyAttributes: serviceKeyAttr, + Attributes: attributeMap, }, mutableResourceAttributes } @@ -581,3 +577,14 @@ func translateGroupedMetricToEmf(groupedMetric *groupedMetric, config *Config, d return event, nil } + +func processAttributes(entityMap map[string]string, targetMap map[string]*string) { + for entityField, shortName := range entityMap { + if val, ok := mutableResourceAttributes.Get(entityField); ok { + if strVal := val.Str(); strVal != "" { + targetMap[shortName] = aws.String(strVal) + } + mutableResourceAttributes.Remove(entityField) + } + } +} From c0b2081d65a95cb71f533f52a4d272403868ac85 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 19:22:23 -0400 Subject: [PATCH 22/45] fixes helper function --- exporter/awsemfexporter/metric_translator.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index b95d3c82b91a..647613f221f1 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -217,8 +217,8 @@ func fetchEntityFields(resourceAttributes pcommon.Map) (cloudwatchlogs.Entity, p } attributeMap := map[string]*string{} - processAttributes(keyAttributeEntityToShortNameMap, serviceKeyAttr) - processAttributes(attributeEntityToShortNameMap, attributeMap) + processAttributes(keyAttributeEntityToShortNameMap, serviceKeyAttr, mutableResourceAttributes) + processAttributes(attributeEntityToShortNameMap, attributeMap, mutableResourceAttributes) return cloudwatchlogs.Entity{ KeyAttributes: serviceKeyAttr, @@ -578,7 +578,7 @@ func translateGroupedMetricToEmf(groupedMetric *groupedMetric, config *Config, d return event, nil } -func processAttributes(entityMap map[string]string, targetMap map[string]*string) { +func processAttributes(entityMap map[string]string, targetMap map[string]*string, mutableResourceAttributes pcommon.Map) { for entityField, shortName := range entityMap { if val, ok := mutableResourceAttributes.Get(entityField); ok { if strVal := val.Str(); strVal != "" { From c87246beefd61b6ba031b1f337d2d1cbe0ae42c3 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Fri, 6 Sep 2024 19:33:42 -0400 Subject: [PATCH 23/45] fixes attribute map --- exporter/awsemfexporter/metric_translator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 647613f221f1..d21bcb0b74cf 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -60,7 +60,7 @@ var keyAttributeEntityToShortNameMap = map[string]string{ var attributeEntityToShortNameMap = map[string]string{ AttributeEntityCluster: cluster, - AttributeEntityNamespace: deploymentEnvironment, + AttributeEntityNamespace: namespace, AttributeEntityWorkload: workload, AttributeEntityNode: node, } From dc696445512691eb54f18a8da460d84ef31049f8 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Sun, 8 Sep 2024 21:11:17 -0400 Subject: [PATCH 24/45] removes log lines, fixes unit tests --- exporter/awsemfexporter/emf_exporter.go | 13 +-- exporter/awsemfexporter/emf_exporter_test.go | 94 +++++++++---------- exporter/awsemfexporter/grouped_metric.go | 6 -- exporter/awsemfexporter/metric_translator.go | 26 ++--- .../awsemfexporter/metric_translator_test.go | 3 +- exporter/awsemfexporter/util.go | 12 +++ exporter/awsemfexporter/util_test.go | 36 +++++++ internal/aws/cwlogs/pusher.go | 19 +--- internal/aws/cwlogs/pusher_test.go | 2 +- 9 files changed, 105 insertions(+), 106 deletions(-) diff --git a/exporter/awsemfexporter/emf_exporter.go b/exporter/awsemfexporter/emf_exporter.go index 964704164877..68d55e2ad62a 100644 --- a/exporter/awsemfexporter/emf_exporter.go +++ b/exporter/awsemfexporter/emf_exporter.go @@ -132,9 +132,7 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e for _, groupedMetric := range groupedMetrics { putLogEvent, err := translateGroupedMetricToEmf(groupedMetric, emf.config, defaultLogStream) - emf.config.logger.Info("putLogEvent", zap.Any("putLogEvent", putLogEvent)) if err != nil { - emf.config.logger.Error("error with translating grouped metric to emf") if errors.Is(err, errMissingMetricsForEnhancedContainerInsights) { emf.config.logger.Debug("Dropping empty putLogEvents for enhanced container insights", zap.Error(err)) continue @@ -154,21 +152,16 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e if emfPusher != nil { returnError := emfPusher.AddLogEntry(putLogEvent) if returnError != nil { - emf.config.logger.Error("could not get the emf pusher", zap.Error(returnError)) return wrapErrorIfBadRequest(returnError) } - emf.config.logger.Info("did not crash when adding the log entry!") } } } if strings.EqualFold(outputDestination, outputDestinationCloudWatch) { - for num, emfPusher := range emf.listPushers() { - emf.config.logger.Info("force flushing emfpusher #" + string(rune(num))) - emf.config.logger.Info("entity is ") + for _, emfPusher := range emf.listPushers() { returnError := emfPusher.ForceFlush() if returnError != nil { - emf.config.logger.Info("error with force flushing") // TODO now we only have one logPusher, so it's ok to return after first error occurred err := wrapErrorIfBadRequest(returnError) if err != nil { @@ -176,11 +169,10 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e } return err } - emf.config.logger.Info("succeeded in force flushing") } } - emf.config.logger.Info("Finish processing resource metrics", zap.Any("labels", labels)) + emf.config.logger.Debug("Finish processing resource metrics", zap.Any("labels", labels)) return nil } @@ -191,7 +183,6 @@ func (emf *emfExporter) getPusher(key cwlogs.StreamKey) cwlogs.Pusher { if _, ok = emf.pusherMap[hash]; !ok { emf.pusherMap[hash] = cwlogs.NewPusher(key, emf.retryCnt, *emf.svcStructuredLog, emf.config.logger) } - emf.config.logger.Info("The hash is " + hash) return emf.pusherMap[hash] } diff --git a/exporter/awsemfexporter/emf_exporter_test.go b/exporter/awsemfexporter/emf_exporter_test.go index d3584b169c9b..02984e817f3d 100644 --- a/exporter/awsemfexporter/emf_exporter_test.go +++ b/exporter/awsemfexporter/emf_exporter_test.go @@ -6,13 +6,14 @@ package awsemfexporter import ( "context" "errors" - "github.com/aws/aws-sdk-go/aws" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/entity" "os" "testing" "github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -34,18 +35,6 @@ func init() { os.Setenv("AWS_SECRET_ACCESS_KEY", "test") } -var entity = cloudwatchlogs.Entity{ - Attributes: map[string]*string{ - "PlatformType": aws.String("AWS::EC2"), - "InstanceId": aws.String("i-123456789"), - "AutoScalingGroup": aws.String("test-group"), - }, - KeyAttributes: map[string]*string{ - "Name": aws.String("myService"), - "Environment": aws.String("myEnvironment"), - }, -} - type mockPusher struct { mock.Mock } @@ -209,17 +198,12 @@ func TestConsumeMetricsWithLogGroupStreamConfig(t *testing.T) { streamKey := &cwlogs.StreamKey{ LogGroupName: expCfg.LogGroupName, LogStreamName: expCfg.LogStreamName, - Entity: &cloudwatchlogs.Entity{Attributes: map[string]*string{ - "PlatformType": nil, - "InstanceId": nil, - "AutoScalingGroup": nil, - }, KeyAttributes: map[string]*string{ - "Name": nil, - "Environment": nil, + Entity: &cloudwatchlogs.Entity{KeyAttributes: map[string]*string{ + entity.EntityType: aws.String(entity.Service), }}, } - streamKeyHash := streamKey.Hash() - pusherMap, ok := exp.pusherMap[streamKeyHash] + expectedStreamKeyHash := streamKey.Hash() + pusherMap, ok := exp.pusherMap[expectedStreamKeyHash] assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -241,8 +225,11 @@ func TestConsumeMetricsWithLogGroupStreamValidPlaceholder(t *testing.T) { metricNames: []string{"metric_1", "metric_2"}, metricValues: [][]float64{{100}, {4}}, resourceAttributeMap: map[string]any{ - "aws.ecs.cluster.name": "test-cluster-name", - "aws.ecs.task.id": "test-task-id", + "aws.ecs.cluster.name": "test-cluster-name", + "aws.ecs.task.id": "test-task-id", + entity.KeyAttributeEntityServiceName: "myService", + entity.KeyAttributeEntityDeploymentEnvironment: "myEnvironment", + entity.AttributeEntityCluster: "test-cluster-name", }, }) require.Error(t, exp.pushMetricsData(ctx, md)) @@ -250,14 +237,15 @@ func TestConsumeMetricsWithLogGroupStreamValidPlaceholder(t *testing.T) { streamKey := &cwlogs.StreamKey{ LogGroupName: "/aws/ecs/containerinsights/test-cluster-name/performance", LogStreamName: "test-task-id", - Entity: &cloudwatchlogs.Entity{Attributes: map[string]*string{ - "PlatformType": nil, - "InstanceId": nil, - "AutoScalingGroup": nil, - }, KeyAttributes: map[string]*string{ - "Name": nil, - "Environment": nil, - }}, + Entity: &cloudwatchlogs.Entity{ + Attributes: map[string]*string{ + "Cluster": aws.String("test-cluster-name"), + }, + KeyAttributes: map[string]*string{ + "Type": aws.String(entity.Service), + "Name": aws.String("myService"), + "Environment": aws.String("myEnvironment"), + }}, } pusherMap, ok := exp.pusherMap[streamKey.Hash()] assert.True(t, ok) @@ -277,12 +265,8 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, exp) var entity = &cloudwatchlogs.Entity{ - Attributes: map[string]*string{ - "PlatformType": aws.String("AWS::EC2"), - "AutoScalingGroup": aws.String("test-group"), - "InstanceId": aws.String("i-123456789"), - }, KeyAttributes: map[string]*string{ + "Type": aws.String(entity.Service), "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), }, @@ -292,10 +276,10 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { metricNames: []string{"metric_1", "metric_2"}, metricValues: [][]float64{{100}, {4}}, resourceAttributeMap: map[string]any{ - "aws.ecs.cluster.name": "test-cluster-name", - "aws.ecs.task.id": "test-task-id", - keyAttributeEntityServiceName: "myService", - keyAttributeEntityDeploymentEnvironment: "myEnvironment", + "aws.ecs.cluster.name": "test-cluster-name", + "aws.ecs.task.id": "test-task-id", + entity.KeyAttributeEntityServiceName: "myService", + entity.KeyAttributeEntityDeploymentEnvironment: "myEnvironment", }, }) require.Error(t, exp.pushMetricsData(ctx, md)) @@ -327,8 +311,10 @@ func TestConsumeMetricsWithWrongPlaceholder(t *testing.T) { metricNames: []string{"metric_1", "metric_2"}, metricValues: [][]float64{{100}, {4}}, resourceAttributeMap: map[string]any{ - "aws.ecs.cluster.name": "test-cluster-name", - "aws.ecs.task.id": "test-task-id", + "aws.ecs.cluster.name": "test-cluster-name", + "aws.ecs.task.id": "test-task-id", + entity.KeyAttributeEntityServiceName: "myService", + entity.KeyAttributeEntityDeploymentEnvironment: "myEnvironment", }, }) require.Error(t, exp.pushMetricsData(ctx, md)) @@ -336,14 +322,13 @@ func TestConsumeMetricsWithWrongPlaceholder(t *testing.T) { streamKey := cwlogs.StreamKey{ LogGroupName: expCfg.LogGroupName, LogStreamName: expCfg.LogStreamName, - Entity: &cloudwatchlogs.Entity{Attributes: map[string]*string{ - "PlatformType": nil, - "InstanceId": nil, - "AutoScalingGroup": nil, - }, KeyAttributes: map[string]*string{ - "Name": nil, - "Environment": nil, - }}, + Entity: &cloudwatchlogs.Entity{ + KeyAttributes: map[string]*string{ + "Type": aws.String(entity.Service), + "Name": aws.String("myService"), + "Environment": aws.String("myEnvironment"), + }, + }, } pusherMap, ok := exp.pusherMap[streamKey.Hash()] assert.True(t, ok) @@ -373,6 +358,11 @@ func TestPushMetricsDataWithErr(t *testing.T) { streamKey := cwlogs.StreamKey{ LogGroupName: "test-logGroupName", LogStreamName: "test-logStreamName", + Entity: &cloudwatchlogs.Entity{ + KeyAttributes: map[string]*string{ + "Type": aws.String(entity.Service), + }, + }, } exp.pusherMap[streamKey.Hash()] = logPusher diff --git a/exporter/awsemfexporter/grouped_metric.go b/exporter/awsemfexporter/grouped_metric.go index 37cfa94a311e..c95186f88129 100644 --- a/exporter/awsemfexporter/grouped_metric.go +++ b/exporter/awsemfexporter/grouped_metric.go @@ -38,7 +38,6 @@ func addToGroupedMetric( ) error { dps := getDataPoints(pmd, metadata, config.logger) - config.logger.Info("got datapoints") if dps == nil || dps.Len() == 0 { return nil } @@ -54,14 +53,12 @@ func addToGroupedMetric( continue } dps, retained := dps.CalculateDeltaDatapoints(i, metadata.instrumentationScopeName, config.DetailedMetrics, calculators) - config.logger.Info("Calculated Delta Datapoints") if !retained { continue } config.logger.Info("Retained") for _, dp := range dps { - config.logger.Info("Traversing through datapoints") labels := dp.labels if metricType, ok := labels["Type"]; ok { @@ -73,7 +70,6 @@ func addToGroupedMetric( // if patterns were found in config file and weren't replaced by resource attributes, replace those patterns with metric labels. // if patterns are provided for a valid key and that key doesn't exist in the resource attributes, it is replaced with `undefined`. if !patternReplaceSucceeded { - config.logger.Info("patternReplaceSucceeded is false") if strings.Contains(metadata.logGroup, "undefined") { metadata.logGroup, _ = replacePatterns(config.LogGroupName, labels, config.logger) } @@ -81,7 +77,6 @@ func addToGroupedMetric( metadata.logStream, _ = replacePatterns(config.LogStreamName, labels, config.logger) } } - config.logger.Info("replaced the patterns for lg and ls") metric := &metricInfo{ value: dp.value, @@ -96,7 +91,6 @@ func addToGroupedMetric( // Extra params to use when grouping metrics groupKey := aws.NewKey(metadata.groupedMetricMetadata, labels) - //config.logger.Info("entity in metadata key:" + groupKey.MetricMetadata.(cWMetricMetadata).entity.GoString()) if _, ok := groupedMetrics[groupKey]; ok { // if MetricName already exists in metrics map, print warning log if _, ok := groupedMetrics[groupKey].metrics[dp.name]; ok { diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index d21bcb0b74cf..e54abb078258 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -7,18 +7,18 @@ import ( "encoding/json" "errors" "fmt" - "github.com/aws/aws-sdk-go/aws" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" - "go.opentelemetry.io/collector/pdata/pcommon" "reflect" "strings" "time" + "github.com/aws/aws-sdk-go/aws" + "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pmetric" "go.uber.org/multierr" "go.uber.org/zap" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" awsmetrics "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/metrics" ) @@ -172,9 +172,7 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri } } - config.logger.Info("resourceAttributes", zap.Any("resourceAttributes", resourceAttributes.AsRaw())) entity, resourceAttributes := fetchEntityFields(resourceAttributes) - config.logger.Info("fetched entity:" + entity.GoString()) for j := 0; j < ilms.Len(); j++ { ilm := ilms.At(j) @@ -198,10 +196,8 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri instrumentationScopeName: instrumentationScopeName, receiver: metricReceiver, } - config.logger.Info("entity after metadata creation:" + metadata.groupedMetricMetadata.entity.GoString()) err := addToGroupedMetric(metric, groupedMetrics, metadata, patternReplaceSucceeded, mt.metricDescriptor, config, mt.calculators) if err != nil { - config.logger.Info("error with adding to grouped metric:" + err.Error()) return err } } @@ -210,6 +206,8 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri } func fetchEntityFields(resourceAttributes pcommon.Map) (cloudwatchlogs.Entity, pcommon.Map) { + //the original resourceAttributes map is immutable, so we need to create a mutable copy + //to remove the entity fields from the attributes mutableResourceAttributes := pcommon.NewMap() resourceAttributes.CopyTo(mutableResourceAttributes) serviceKeyAttr := map[string]*string{ @@ -440,7 +438,7 @@ func translateCWMetricToEMF(cWMetric *cWMetrics, config *Config) (*cwlogs.Event, var f any err := json.Unmarshal([]byte(val), &f) if err != nil { - config.logger.Info( + config.logger.Debug( "Failed to parse json-encoded string", zap.String("label key", key), zap.String("label value", val), @@ -555,7 +553,6 @@ func translateGroupedMetricToEmf(groupedMetric *groupedMetric, config *Config, d cWMetric := translateGroupedMetricToCWMetric(groupedMetric, config) event, err := translateCWMetricToEMF(cWMetric, config) if err != nil { - config.logger.Info("error with translating CW Metric to EMF", zap.Error(err)) return nil, err } // Drop a nil putLogEvent for EnhancedContainerInsights @@ -577,14 +574,3 @@ func translateGroupedMetricToEmf(groupedMetric *groupedMetric, config *Config, d return event, nil } - -func processAttributes(entityMap map[string]string, targetMap map[string]*string, mutableResourceAttributes pcommon.Map) { - for entityField, shortName := range entityMap { - if val, ok := mutableResourceAttributes.Get(entityField); ok { - if strVal := val.Str(); strVal != "" { - targetMap[shortName] = aws.String(strVal) - } - mutableResourceAttributes.Remove(entityField) - } - } -} diff --git a/exporter/awsemfexporter/metric_translator_test.go b/exporter/awsemfexporter/metric_translator_test.go index 60479c88a769..867fa727ad03 100644 --- a/exporter/awsemfexporter/metric_translator_test.go +++ b/exporter/awsemfexporter/metric_translator_test.go @@ -6,6 +6,7 @@ package awsemfexporter import ( "errors" "fmt" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/entity" "math" "reflect" "sort" @@ -39,7 +40,7 @@ func createTestResourceMetricsHelper(numMetrics int) pmetric.ResourceMetrics { rm.Resource().Attributes().PutStr("ClusterName", "myCluster") rm.Resource().Attributes().PutStr("PodName", "myPod") rm.Resource().Attributes().PutStr(attributeReceiver, prometheusReceiver) - rm.Resource().Attributes().PutStr(keyAttributeEntityServiceName, "myServiceName") + rm.Resource().Attributes().PutStr(entity.KeyAttributeEntityServiceName, "myServiceName") sm := rm.ScopeMetrics().AppendEmpty() m := sm.Metrics().AppendEmpty() diff --git a/exporter/awsemfexporter/util.go b/exporter/awsemfexporter/util.go index a820fd4d1ed6..1fadc1ea493e 100644 --- a/exporter/awsemfexporter/util.go +++ b/exporter/awsemfexporter/util.go @@ -5,6 +5,7 @@ package awsemfexporter // import "github.com/open-telemetry/opentelemetry-collec import ( "fmt" + "github.com/aws/aws-sdk-go/aws" "sort" "strings" "time" @@ -172,3 +173,14 @@ func attrMaptoStringMap(attrMap pcommon.Map) map[string]string { }) return strMap } + +func processAttributes(entityMap map[string]string, targetMap map[string]*string, mutableResourceAttributes pcommon.Map) { + for entityField, shortName := range entityMap { + if val, ok := mutableResourceAttributes.Get(entityField); ok { + if strVal := val.Str(); strVal != "" { + targetMap[shortName] = aws.String(strVal) + } + mutableResourceAttributes.Remove(entityField) + } + } +} diff --git a/exporter/awsemfexporter/util_test.go b/exporter/awsemfexporter/util_test.go index dc5a3dc3735b..bfc70aca783b 100644 --- a/exporter/awsemfexporter/util_test.go +++ b/exporter/awsemfexporter/util_test.go @@ -366,3 +366,39 @@ func TestGetLogInfo(t *testing.T) { } } + +func TestProcessAttributes(t *testing.T) { + testCases := []struct { + name string + key string + value string + expected string + }{ + { + name: "Non-empty string value", + key: "testKey", + value: "testValue", + expected: "testValue", + }, + { + name: "Empty string value", + key: "emptyKey", + value: "", + expected: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + attrs := pcommon.NewMap() + attrs.PutStr(tc.key, tc.value) + + processAttributes(make(map[string]string), make(map[string]*string), attrs) + + val, ok := attrs.Get(tc.key) + assert.True(t, ok, "Key should exist in the map") + actualVal := val.Str() + assert.Equal(t, tc.expected, actualVal, "Value should match the expected value") + }) + } +} diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index ea2da8f02f31..039ee6dd68a4 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -136,12 +136,7 @@ type eventBatch struct { } // Create a new log event batch if needed. -func newEventBatch(key StreamKey, logger *zap.Logger) *eventBatch { - logger.Info("in newEventBatch function") - logger.Info("logGroupName: " + key.LogGroupName) - logger.Info("logStreamName: " + key.LogStreamName) - logger.Info("p.entity type: " + fmt.Sprintf("%T", key.Entity)) - logger.Info("entity: " + key.Entity.GoString()) +func newEventBatch(key StreamKey) *eventBatch { return &eventBatch{ putLogEventsInput: &cloudwatchlogs.PutLogEventsInput{ @@ -252,7 +247,7 @@ func newLogPusher(streamKey StreamKey, svcStructuredLog: svcStructuredLog, logger: logger, } - pusher.logEventBatch = newEventBatch(streamKey, logger) + pusher.logEventBatch = newEventBatch(streamKey) return pusher } @@ -279,9 +274,7 @@ func (p *logPusher) AddLogEntry(logEvent *Event) error { } func (p *logPusher) ForceFlush() error { - p.logger.Info("in force flush method for the log pusher") prevBatch := p.renewEventBatch() - p.logger.Info("renewedEventBatch. The entity here is " + prevBatch.putLogEventsInput.Entity.GoString()) if prevBatch != nil { return p.pushEventBatch(prevBatch) } @@ -328,7 +321,7 @@ func (p *logPusher) addLogEvent(logEvent *Event) *eventBatch { LogGroupName: *p.logGroupName, LogStreamName: *p.logStreamName, Entity: p.entity, - }, p.logger) + }) } currentBatch.append(logEvent) p.logEventBatch = currentBatch @@ -339,17 +332,13 @@ func (p *logPusher) addLogEvent(logEvent *Event) *eventBatch { func (p *logPusher) renewEventBatch() *eventBatch { var prevBatch *eventBatch - p.logger.Info("renewing EventBatch, just before the if statement") if len(p.logEventBatch.putLogEventsInput.LogEvents) > 0 { - p.logger.Info("renewing EventBatch. The entity here is " + p.entity.GoString()) - p.logger.Info("p.entity type: " + fmt.Sprintf("%T", p.entity)) prevBatch = p.logEventBatch p.logEventBatch = newEventBatch(StreamKey{ LogGroupName: *p.logGroupName, LogStreamName: *p.logStreamName, Entity: p.entity, - }, p.logger) - p.logger.Info("renewed EventBatch. The entity here is " + p.logEventBatch.putLogEventsInput.Entity.GoString()) + }) } return prevBatch diff --git a/internal/aws/cwlogs/pusher_test.go b/internal/aws/cwlogs/pusher_test.go index df32a762142b..4aaab00dbef4 100644 --- a/internal/aws/cwlogs/pusher_test.go +++ b/internal/aws/cwlogs/pusher_test.go @@ -132,7 +132,7 @@ func TestPusher_newLogEventBatch(t *testing.T) { LogGroupName: logGroup, LogStreamName: logStreamName, Entity: &entity, - }, zap.NewExample()) + }) assert.Equal(t, int64(0), logEventBatch.maxTimestampMs) assert.Equal(t, int64(0), logEventBatch.minTimestampMs) assert.Equal(t, 0, logEventBatch.byteTotal) From d8030e9437e6e63615804c9df246b3573b375a9d Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Sun, 8 Sep 2024 21:40:16 -0400 Subject: [PATCH 25/45] fixes lint issues, stray log lines --- exporter/awsemfexporter/emf_exporter_test.go | 37 +++++++++---------- exporter/awsemfexporter/grouped_metric.go | 3 -- exporter/awsemfexporter/metric_translator.go | 16 ++++---- .../awsemfexporter/metric_translator_test.go | 3 +- exporter/awsemfexporter/util.go | 2 +- internal/aws/cwlogs/cwlog_client_test.go | 6 +-- internal/aws/cwlogs/pusher.go | 2 +- 7 files changed, 30 insertions(+), 39 deletions(-) diff --git a/exporter/awsemfexporter/emf_exporter_test.go b/exporter/awsemfexporter/emf_exporter_test.go index 02984e817f3d..fb12292a64df 100644 --- a/exporter/awsemfexporter/emf_exporter_test.go +++ b/exporter/awsemfexporter/emf_exporter_test.go @@ -6,7 +6,6 @@ package awsemfexporter import ( "context" "errors" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/entity" "os" "testing" @@ -199,7 +198,7 @@ func TestConsumeMetricsWithLogGroupStreamConfig(t *testing.T) { LogGroupName: expCfg.LogGroupName, LogStreamName: expCfg.LogStreamName, Entity: &cloudwatchlogs.Entity{KeyAttributes: map[string]*string{ - entity.EntityType: aws.String(entity.Service), + entityType: aws.String(service), }}, } expectedStreamKeyHash := streamKey.Hash() @@ -225,11 +224,11 @@ func TestConsumeMetricsWithLogGroupStreamValidPlaceholder(t *testing.T) { metricNames: []string{"metric_1", "metric_2"}, metricValues: [][]float64{{100}, {4}}, resourceAttributeMap: map[string]any{ - "aws.ecs.cluster.name": "test-cluster-name", - "aws.ecs.task.id": "test-task-id", - entity.KeyAttributeEntityServiceName: "myService", - entity.KeyAttributeEntityDeploymentEnvironment: "myEnvironment", - entity.AttributeEntityCluster: "test-cluster-name", + "aws.ecs.cluster.name": "test-cluster-name", + "aws.ecs.task.id": "test-task-id", + keyAttributeEntityServiceName: "myService", + keyAttributeEntityDeploymentEnvironment: "myEnvironment", + attributeEntityCluster: "test-cluster-name", }, }) require.Error(t, exp.pushMetricsData(ctx, md)) @@ -242,7 +241,7 @@ func TestConsumeMetricsWithLogGroupStreamValidPlaceholder(t *testing.T) { "Cluster": aws.String("test-cluster-name"), }, KeyAttributes: map[string]*string{ - "Type": aws.String(entity.Service), + "Type": aws.String(service), "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), }}, @@ -266,7 +265,7 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { assert.NotNil(t, exp) var entity = &cloudwatchlogs.Entity{ KeyAttributes: map[string]*string{ - "Type": aws.String(entity.Service), + "Type": aws.String(service), "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), }, @@ -276,10 +275,10 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { metricNames: []string{"metric_1", "metric_2"}, metricValues: [][]float64{{100}, {4}}, resourceAttributeMap: map[string]any{ - "aws.ecs.cluster.name": "test-cluster-name", - "aws.ecs.task.id": "test-task-id", - entity.KeyAttributeEntityServiceName: "myService", - entity.KeyAttributeEntityDeploymentEnvironment: "myEnvironment", + "aws.ecs.cluster.name": "test-cluster-name", + "aws.ecs.task.id": "test-task-id", + keyAttributeEntityServiceName: "myService", + keyAttributeEntityDeploymentEnvironment: "myEnvironment", }, }) require.Error(t, exp.pushMetricsData(ctx, md)) @@ -311,10 +310,10 @@ func TestConsumeMetricsWithWrongPlaceholder(t *testing.T) { metricNames: []string{"metric_1", "metric_2"}, metricValues: [][]float64{{100}, {4}}, resourceAttributeMap: map[string]any{ - "aws.ecs.cluster.name": "test-cluster-name", - "aws.ecs.task.id": "test-task-id", - entity.KeyAttributeEntityServiceName: "myService", - entity.KeyAttributeEntityDeploymentEnvironment: "myEnvironment", + "aws.ecs.cluster.name": "test-cluster-name", + "aws.ecs.task.id": "test-task-id", + keyAttributeEntityServiceName: "myService", + keyAttributeEntityDeploymentEnvironment: "myEnvironment", }, }) require.Error(t, exp.pushMetricsData(ctx, md)) @@ -324,7 +323,7 @@ func TestConsumeMetricsWithWrongPlaceholder(t *testing.T) { LogStreamName: expCfg.LogStreamName, Entity: &cloudwatchlogs.Entity{ KeyAttributes: map[string]*string{ - "Type": aws.String(entity.Service), + "Type": aws.String(service), "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), }, @@ -360,7 +359,7 @@ func TestPushMetricsDataWithErr(t *testing.T) { LogStreamName: "test-logStreamName", Entity: &cloudwatchlogs.Entity{ KeyAttributes: map[string]*string{ - "Type": aws.String(entity.Service), + "Type": aws.String(service), }, }, } diff --git a/exporter/awsemfexporter/grouped_metric.go b/exporter/awsemfexporter/grouped_metric.go index c95186f88129..dd3426cdc2e9 100644 --- a/exporter/awsemfexporter/grouped_metric.go +++ b/exporter/awsemfexporter/grouped_metric.go @@ -56,7 +56,6 @@ func addToGroupedMetric( if !retained { continue } - config.logger.Info("Retained") for _, dp := range dps { labels := dp.labels @@ -82,12 +81,10 @@ func addToGroupedMetric( value: dp.value, unit: translateUnit(pmd, descriptor), } - config.logger.Info("created the metric") if dp.timestampMs > 0 { metadata.timestampMs = dp.timestampMs } - config.logger.Info("set the timestamp") // Extra params to use when grouping metrics groupKey := aws.NewKey(metadata.groupedMetricMetadata, labels) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index e54abb078258..4c4b35c3f053 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -43,13 +43,13 @@ const ( deploymentEnvironment = "Environment" entityType = "Type" service = "Service" - AttributeEntityCluster = "aws.entity.k8s.cluster.name" + attributeEntityCluster = "aws.entity.k8s.cluster.name" cluster = "Cluster" - AttributeEntityNamespace = "aws.entity.k8s.namespace.name" + attributeEntityNamespace = "aws.entity.k8s.namespace.name" namespace = "Namespace" - AttributeEntityWorkload = "aws.entity.k8s.workload.name" + attributeEntityWorkload = "aws.entity.k8s.workload.name" workload = "Workload" - AttributeEntityNode = "aws.entity.k8s.node.name" + attributeEntityNode = "aws.entity.k8s.node.name" node = "Node" ) @@ -59,10 +59,10 @@ var keyAttributeEntityToShortNameMap = map[string]string{ } var attributeEntityToShortNameMap = map[string]string{ - AttributeEntityCluster: cluster, - AttributeEntityNamespace: namespace, - AttributeEntityWorkload: workload, - AttributeEntityNode: node, + attributeEntityCluster: cluster, + attributeEntityNamespace: namespace, + attributeEntityWorkload: workload, + attributeEntityNode: node, } var errMissingMetricsForEnhancedContainerInsights = errors.New("nil event detected with EnhancedContainerInsights enabled") diff --git a/exporter/awsemfexporter/metric_translator_test.go b/exporter/awsemfexporter/metric_translator_test.go index 867fa727ad03..60479c88a769 100644 --- a/exporter/awsemfexporter/metric_translator_test.go +++ b/exporter/awsemfexporter/metric_translator_test.go @@ -6,7 +6,6 @@ package awsemfexporter import ( "errors" "fmt" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/entity" "math" "reflect" "sort" @@ -40,7 +39,7 @@ func createTestResourceMetricsHelper(numMetrics int) pmetric.ResourceMetrics { rm.Resource().Attributes().PutStr("ClusterName", "myCluster") rm.Resource().Attributes().PutStr("PodName", "myPod") rm.Resource().Attributes().PutStr(attributeReceiver, prometheusReceiver) - rm.Resource().Attributes().PutStr(entity.KeyAttributeEntityServiceName, "myServiceName") + rm.Resource().Attributes().PutStr(keyAttributeEntityServiceName, "myServiceName") sm := rm.ScopeMetrics().AppendEmpty() m := sm.Metrics().AppendEmpty() diff --git a/exporter/awsemfexporter/util.go b/exporter/awsemfexporter/util.go index 1fadc1ea493e..a80fb6cd934b 100644 --- a/exporter/awsemfexporter/util.go +++ b/exporter/awsemfexporter/util.go @@ -5,11 +5,11 @@ package awsemfexporter // import "github.com/open-telemetry/opentelemetry-collec import ( "fmt" - "github.com/aws/aws-sdk-go/aws" "sort" "strings" "time" + "github.com/aws/aws-sdk-go/aws" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pmetric" conventions "go.opentelemetry.io/collector/semconv/v1.6.1" diff --git a/internal/aws/cwlogs/cwlog_client_test.go b/internal/aws/cwlogs/cwlog_client_test.go index 2f63d96a8296..7dd2e5858b1b 100644 --- a/internal/aws/cwlogs/cwlog_client_test.go +++ b/internal/aws/cwlogs/cwlog_client_test.go @@ -84,12 +84,8 @@ var logGroup = "logGroup" var logStreamName = "logStream" var entity = cloudwatchlogs.Entity{ - Attributes: map[string]*string{ - "PlatformType": aws.String("AWS::EC2"), - "EC2.InstanceId": aws.String("i-123456789"), - "EC2.AutoScalingGroup": aws.String("test-group"), - }, KeyAttributes: map[string]*string{ + "Type": aws.String("Service"), "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), }, diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index 039ee6dd68a4..822b4c266840 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -303,7 +303,7 @@ func (p *logPusher) pushEventBatch(req any) error { zap.Int("NumOfLogEvents", len(putLogEventsInput.LogEvents)), zap.Float64("LogEventsSize", float64(logEventBatch.byteTotal)/float64(1024)), zap.Int64("Time", time.Since(startTime).Nanoseconds()/int64(time.Millisecond)), - zap.String("Entity", logEventBatch.putLogEventsInput.Entity.GoString())) + ) return nil } From bc5ceae6eca67e9965c56987f6c2548f11df72ef Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Sun, 8 Sep 2024 22:56:56 -0400 Subject: [PATCH 26/45] fixes linting, testing --- exporter/awsemfexporter/metric_translator.go | 8 ++--- .../awsemfexporter/metric_translator_test.go | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 4c4b35c3f053..0c49af56ba8b 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -36,7 +36,7 @@ const ( attributeReceiver = "receiver" fieldPrometheusMetricType = "prom_metric_type" - //Entity fields + // Entity fields keyAttributeEntityServiceName = "aws.entity.service.name" serviceName = "Name" keyAttributeEntityDeploymentEnvironment = "aws.entity.deployment.environment" @@ -155,7 +155,6 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri cWNamespace := getNamespace(rm, config.Namespace) logGroup, logStream, patternReplaceSucceeded := getLogInfo(rm, cWNamespace, config) deltaInitialValue := config.RetainInitialValueOfDeltaMetric - resourceAttributes := rm.Resource().Attributes() ilms := rm.ScopeMetrics() var metricReceiver string @@ -172,7 +171,8 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri } } - entity, resourceAttributes := fetchEntityFields(resourceAttributes) + entity, resourceAttributes := fetchEntityFields(rm.Resource().Attributes()) + resourceAttributes.CopyTo(rm.Resource().Attributes()) for j := 0; j < ilms.Len(); j++ { ilm := ilms.At(j) @@ -523,7 +523,6 @@ func translateCWMetricToEMF(cWMetric *cWMetrics, config *Config) (*cwlogs.Event, pleMsg, err := json.Marshal(fieldMap) if err != nil { - config.logger.Error("error with marshalling the fieldMap: ", zap.Error(err)) return nil, err } @@ -531,7 +530,6 @@ func translateCWMetricToEMF(cWMetric *cWMetrics, config *Config) (*cwlogs.Event, if len(metricsMap) > 0 { metricsMsg, err := json.Marshal(metricsMap) if err != nil { - config.logger.Error("error with marshalling the metricsMap: ", zap.Error(err)) return nil, err } metricsMsg[0] = ',' diff --git a/exporter/awsemfexporter/metric_translator_test.go b/exporter/awsemfexporter/metric_translator_test.go index 60479c88a769..9114b410856d 100644 --- a/exporter/awsemfexporter/metric_translator_test.go +++ b/exporter/awsemfexporter/metric_translator_test.go @@ -12,6 +12,8 @@ import ( "testing" "time" + "github.com/aws/aws-sdk-go/aws" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" @@ -2575,6 +2577,35 @@ func TestTranslateOtToGroupedMetricForInitialDeltaValue(t *testing.T) { } } +func TestFetchEntityFields(t *testing.T) { + resourceMetrics := pmetric.NewResourceMetrics() + resourceMetrics.Resource().Attributes().PutStr(keyAttributeEntityDeploymentEnvironment, "my-environment") + resourceMetrics.Resource().Attributes().PutStr(keyAttributeEntityServiceName, "my-service") + resourceMetrics.Resource().Attributes().PutStr(attributeEntityNode, "my-node") + resourceMetrics.Resource().Attributes().PutStr(attributeEntityCluster, "my-cluster") + resourceMetrics.Resource().Attributes().PutStr(attributeEntityNamespace, "my-namespace") + resourceMetrics.Resource().Attributes().PutStr(attributeEntityWorkload, "my-workload") + + expectedEntity := cloudwatchlogs.Entity{KeyAttributes: map[string]*string{ + entityType: aws.String(service), + serviceName: aws.String("my-service"), + deploymentEnvironment: aws.String("my-environment"), + }, + Attributes: map[string]*string{ + node: aws.String("my-node"), + cluster: aws.String("my-cluster"), + namespace: aws.String("my-namespace"), + workload: aws.String("my-workload"), + }, + } + + entity, attrs := fetchEntityFields(resourceMetrics.Resource().Attributes()) + assert.Equal(t, expectedEntity, entity) + attrs.CopyTo(resourceMetrics.Resource().Attributes()) + assert.Equal(t, 0, resourceMetrics.Resource().Attributes().Len()) + +} + func generateTestMetrics(tm testMetric) pmetric.Metrics { md := pmetric.NewMetrics() now := time.Now() From 81300e347120661c7e2d728eb960c4146e423b24 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Sun, 8 Sep 2024 23:03:17 -0400 Subject: [PATCH 27/45] removes commented line --- internal/aws/cwlogs/pusher.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index 822b4c266840..64e18e8a587c 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -81,7 +81,6 @@ func (sk *StreamKey) Hash() string { attributes, keyAttributes, ) - //hash := sha256.Sum256([]byte(data)) return data } From 33c203ec41c5271d507b95e2245c42545405c99a Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Mon, 9 Sep 2024 10:25:07 -0400 Subject: [PATCH 28/45] fixes lint for comment --- exporter/awsemfexporter/metric_translator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 0c49af56ba8b..0c0871aedc0a 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -206,8 +206,8 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri } func fetchEntityFields(resourceAttributes pcommon.Map) (cloudwatchlogs.Entity, pcommon.Map) { - //the original resourceAttributes map is immutable, so we need to create a mutable copy - //to remove the entity fields from the attributes + // the original resourceAttributes map is immutable, so we need to create a mutable copy + // to remove the entity fields from the attributes mutableResourceAttributes := pcommon.NewMap() resourceAttributes.CopyTo(mutableResourceAttributes) serviceKeyAttr := map[string]*string{ From ee93dd907d40550e5680118b0088acb36ed5749f Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Mon, 9 Sep 2024 10:54:02 -0400 Subject: [PATCH 29/45] fixes more linting issues --- exporter/awsemfexporter/emf_exporter_test.go | 2 +- exporter/awsemfexporter/metric_translator_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/exporter/awsemfexporter/emf_exporter_test.go b/exporter/awsemfexporter/emf_exporter_test.go index fb12292a64df..59c976cc02d8 100644 --- a/exporter/awsemfexporter/emf_exporter_test.go +++ b/exporter/awsemfexporter/emf_exporter_test.go @@ -12,7 +12,6 @@ import ( "github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -25,6 +24,7 @@ import ( "go.uber.org/zap/zaptest/observer" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" ) const defaultRetryCount = 1 diff --git a/exporter/awsemfexporter/metric_translator_test.go b/exporter/awsemfexporter/metric_translator_test.go index 9114b410856d..12ac242cb8c4 100644 --- a/exporter/awsemfexporter/metric_translator_test.go +++ b/exporter/awsemfexporter/metric_translator_test.go @@ -13,7 +13,6 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" @@ -24,6 +23,7 @@ import ( "go.uber.org/zap/zaptest/observer" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs/sdk/service/cloudwatchlogs" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/occonventions" ) From 7fd20c42f9edebdd585827571684e152fa5727a7 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 10 Sep 2024 11:18:17 -0400 Subject: [PATCH 30/45] adds comment explaining processAttributes function --- exporter/awsemfexporter/util.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/exporter/awsemfexporter/util.go b/exporter/awsemfexporter/util.go index a80fb6cd934b..61e3fcfbb57d 100644 --- a/exporter/awsemfexporter/util.go +++ b/exporter/awsemfexporter/util.go @@ -174,6 +174,8 @@ func attrMaptoStringMap(attrMap pcommon.Map) map[string]string { return strMap } +// processAttributes fetches the aws.entity fields and creates an entity to be sent at the PutLogEvent call. It also +// removes the entity attributes so that it is not tagged as a dimension, and reduces the size of the PLE payload. func processAttributes(entityMap map[string]string, targetMap map[string]*string, mutableResourceAttributes pcommon.Map) { for entityField, shortName := range entityMap { if val, ok := mutableResourceAttributes.Get(entityField); ok { From 5a08c19b9d0f4fd21d5072e9cad24669da421912 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Thu, 12 Sep 2024 09:44:51 -0400 Subject: [PATCH 31/45] adds more to unit tests, fixes variable name --- exporter/awsemfexporter/metric_translator.go | 6 +- exporter/awsemfexporter/util_test.go | 104 +++++++++++++++---- 2 files changed, 87 insertions(+), 23 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 0c0871aedc0a..40430d17bfb9 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -210,16 +210,16 @@ func fetchEntityFields(resourceAttributes pcommon.Map) (cloudwatchlogs.Entity, p // to remove the entity fields from the attributes mutableResourceAttributes := pcommon.NewMap() resourceAttributes.CopyTo(mutableResourceAttributes) - serviceKeyAttr := map[string]*string{ + keyAttributesMap := map[string]*string{ entityType: aws.String(service), } attributeMap := map[string]*string{} - processAttributes(keyAttributeEntityToShortNameMap, serviceKeyAttr, mutableResourceAttributes) + processAttributes(keyAttributeEntityToShortNameMap, keyAttributesMap, mutableResourceAttributes) processAttributes(attributeEntityToShortNameMap, attributeMap, mutableResourceAttributes) return cloudwatchlogs.Entity{ - KeyAttributes: serviceKeyAttr, + KeyAttributes: keyAttributesMap, Attributes: attributeMap, }, mutableResourceAttributes } diff --git a/exporter/awsemfexporter/util_test.go b/exporter/awsemfexporter/util_test.go index bfc70aca783b..377f7381d0af 100644 --- a/exporter/awsemfexporter/util_test.go +++ b/exporter/awsemfexporter/util_test.go @@ -4,6 +4,7 @@ package awsemfexporter import ( + "github.com/aws/aws-sdk-go/aws" "testing" "github.com/stretchr/testify/assert" @@ -369,36 +370,99 @@ func TestGetLogInfo(t *testing.T) { func TestProcessAttributes(t *testing.T) { testCases := []struct { - name string - key string - value string - expected string + name string + entityMap []map[string]string + resourceAttributes map[string]any + wantedAttributes map[string]*string + leftoverAttributes map[string]any }{ { - name: "Non-empty string value", - key: "testKey", - value: "testValue", - expected: "testValue", + name: "key_attributes", + entityMap: []map[string]string{keyAttributeEntityToShortNameMap}, + resourceAttributes: map[string]any{ + keyAttributeEntityServiceName: "my-service", + keyAttributeEntityDeploymentEnvironment: "my-environment", + }, + wantedAttributes: map[string]*string{ + serviceName: aws.String("my-service"), + deploymentEnvironment: aws.String("my-environment"), + }, + leftoverAttributes: make(map[string]any), }, { - name: "Empty string value", - key: "emptyKey", - value: "", - expected: "", + name: "non-key_attributes", + entityMap: []map[string]string{attributeEntityToShortNameMap}, + resourceAttributes: map[string]any{ + attributeEntityCluster: "my-cluster", + attributeEntityNamespace: "my-namespace", + attributeEntityNode: "my-node", + attributeEntityWorkload: "my-workload", + }, + wantedAttributes: map[string]*string{ + cluster: aws.String("my-cluster"), + namespace: aws.String("my-namespace"), + node: aws.String("my-node"), + workload: aws.String("my-workload"), + }, + leftoverAttributes: make(map[string]any), + }, + { + name: "key_and_non_key_attributes", + entityMap: []map[string]string{keyAttributeEntityToShortNameMap, attributeEntityToShortNameMap}, + resourceAttributes: map[string]any{ + keyAttributeEntityServiceName: "my-service", + keyAttributeEntityDeploymentEnvironment: "my-environment", + attributeEntityCluster: "my-cluster", + attributeEntityNamespace: "my-namespace", + attributeEntityNode: "my-node", + attributeEntityWorkload: "my-workload", + }, + wantedAttributes: map[string]*string{ + serviceName: aws.String("my-service"), + deploymentEnvironment: aws.String("my-environment"), + cluster: aws.String("my-cluster"), + namespace: aws.String("my-namespace"), + node: aws.String("my-node"), + workload: aws.String("my-workload"), + }, + leftoverAttributes: make(map[string]any), + }, + { + name: "key_and_non_key_attributes_plus_extras", + entityMap: []map[string]string{keyAttributeEntityToShortNameMap, attributeEntityToShortNameMap}, + resourceAttributes: map[string]any{ + "extra_attribute": "extra_value", + keyAttributeEntityServiceName: "my-service", + keyAttributeEntityDeploymentEnvironment: "my-environment", + attributeEntityCluster: "my-cluster", + attributeEntityNamespace: "my-namespace", + attributeEntityNode: "my-node", + attributeEntityWorkload: "my-workload", + }, + wantedAttributes: map[string]*string{ + serviceName: aws.String("my-service"), + deploymentEnvironment: aws.String("my-environment"), + cluster: aws.String("my-cluster"), + namespace: aws.String("my-namespace"), + node: aws.String("my-node"), + workload: aws.String("my-workload"), + }, + leftoverAttributes: map[string]any{ + "extra_attribute": "extra_value", + }, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { attrs := pcommon.NewMap() - attrs.PutStr(tc.key, tc.value) - - processAttributes(make(map[string]string), make(map[string]*string), attrs) - - val, ok := attrs.Get(tc.key) - assert.True(t, ok, "Key should exist in the map") - actualVal := val.Str() - assert.Equal(t, tc.expected, actualVal, "Value should match the expected value") + attrs.FromRaw(tc.resourceAttributes) + targetMap := make(map[string]*string) + for _, entityMap := range tc.entityMap { + processAttributes(entityMap, targetMap, attrs) + } + assert.Equal(t, tc.leftoverAttributes, attrs.AsRaw()) + assert.Equal(t, tc.wantedAttributes, targetMap) }) } } From 8c22b7d968d9b4a9a4004bc7c4c326c9c9bf95c6 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Thu, 12 Sep 2024 11:20:04 -0400 Subject: [PATCH 32/45] address lint check finding for error check in test --- exporter/awsemfexporter/util_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exporter/awsemfexporter/util_test.go b/exporter/awsemfexporter/util_test.go index 377f7381d0af..f52542e594d7 100644 --- a/exporter/awsemfexporter/util_test.go +++ b/exporter/awsemfexporter/util_test.go @@ -456,7 +456,8 @@ func TestProcessAttributes(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { attrs := pcommon.NewMap() - attrs.FromRaw(tc.resourceAttributes) + err := attrs.FromRaw(tc.resourceAttributes) + assert.Nil(t, err) targetMap := make(map[string]*string) for _, entityMap := range tc.entityMap { processAttributes(entityMap, targetMap, attrs) From 3cbf1d6115f9a8cacc84fe0b28b6a5c2e84e3b68 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 17 Sep 2024 13:48:50 -0400 Subject: [PATCH 33/45] adds attributes for resource types. Refactors pusher map logic to be bounded --- exporter/awsemfexporter/bounded_pusher_map.go | 71 +++++++++++++++++++ exporter/awsemfexporter/emf_exporter.go | 21 +++--- exporter/awsemfexporter/metric_translator.go | 15 ++-- internal/aws/cwlogs/cwlog_client_test.go | 4 ++ 4 files changed, 98 insertions(+), 13 deletions(-) create mode 100644 exporter/awsemfexporter/bounded_pusher_map.go diff --git a/exporter/awsemfexporter/bounded_pusher_map.go b/exporter/awsemfexporter/bounded_pusher_map.go new file mode 100644 index 000000000000..98fc7935dad5 --- /dev/null +++ b/exporter/awsemfexporter/bounded_pusher_map.go @@ -0,0 +1,71 @@ +package awsemfexporter + +import ( + "errors" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" + "go.uber.org/zap" + "time" +) + +const ( + pusherMapLimit = 1000 +) + +type boundedPusherMap struct { + pusherMap map[string]cwlogs.Pusher + limit int + stalePusherTracker map[string]time.Time +} + +func newBoundedPusherMap() boundedPusherMap { + return boundedPusherMap{ + pusherMap: make(map[string]cwlogs.Pusher), + limit: pusherMapLimit, + stalePusherTracker: make(map[string]time.Time), + } +} + +func (bpm *boundedPusherMap) add(key string, pusher cwlogs.Pusher, logger *zap.Logger) { + err := bpm.evictStalePushers() + if err != nil { + logger.Error("Error with evicting stale pushers", zap.Error(err)) + return + } + bpm.pusherMap[key] = pusher + bpm.stalePusherTracker[key] = time.Now() +} + +func (bpm *boundedPusherMap) get(key string) (cwlogs.Pusher, bool) { + pusher, ok := bpm.pusherMap[key] + if ok { + bpm.stalePusherTracker[key] = time.Now() + } + return pusher, ok +} + +func (bpm *boundedPusherMap) evictStalePushers() error { + if len(bpm.pusherMap) < bpm.limit { + return nil + } + now := time.Now() + for key, lastUsed := range bpm.stalePusherTracker { + if now.Sub(lastUsed) > time.Hour { + delete(bpm.pusherMap, key) + delete(bpm.stalePusherTracker, key) + } + } + // Ideally, we should now be below the pusher limit. If we aren't, especially after deleting pushers older than an hour, + // we should log an error. + if len(bpm.pusherMap) >= bpm.limit { + return errors.New("too many emf pushers being created. Dropping the request") + } + return nil +} + +func (bpm *boundedPusherMap) listAllPushers() []cwlogs.Pusher { + var pushers []cwlogs.Pusher + for _, pusher := range bpm.pusherMap { + pushers = append(pushers, pusher) + } + return pushers +} diff --git a/exporter/awsemfexporter/emf_exporter.go b/exporter/awsemfexporter/emf_exporter.go index 68d55e2ad62a..2915205b4636 100644 --- a/exporter/awsemfexporter/emf_exporter.go +++ b/exporter/awsemfexporter/emf_exporter.go @@ -7,9 +7,6 @@ import ( "context" "errors" "fmt" - "strings" - "sync" - "github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/google/uuid" @@ -19,6 +16,8 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pmetric" "go.uber.org/zap" + "strings" + "sync" "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsemfexporter/internal/appsignals" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/awsutil" @@ -36,7 +35,7 @@ const ( ) type emfExporter struct { - pusherMap map[string]cwlogs.Pusher + boundedPusherMap boundedPusherMap svcStructuredLog *cwlogs.Client config *Config @@ -86,8 +85,8 @@ func newEmfExporter(config *Config, set exporter.Settings) (*emfExporter, error) metricTranslator: newMetricTranslator(*config), retryCnt: *awsConfig.MaxRetries, collectorID: collectorIdentifier.String(), - pusherMap: map[string]cwlogs.Pusher{}, processResourceLabels: func(map[string]string) {}, + boundedPusherMap: newBoundedPusherMap(), } if config.IsAppSignalsEnabled() { @@ -179,11 +178,15 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e func (emf *emfExporter) getPusher(key cwlogs.StreamKey) cwlogs.Pusher { var ok bool + var pusher cwlogs.Pusher hash := key.Hash() - if _, ok = emf.pusherMap[hash]; !ok { - emf.pusherMap[hash] = cwlogs.NewPusher(key, emf.retryCnt, *emf.svcStructuredLog, emf.config.logger) + if _, ok = emf.boundedPusherMap.get(hash); !ok { + pusher = cwlogs.NewPusher(key, emf.retryCnt, *emf.svcStructuredLog, emf.config.logger) + emf.boundedPusherMap.add(hash, pusher, emf.config.logger) } - return emf.pusherMap[hash] + + //return emf.pusherMap[hash] + return pusher } func (emf *emfExporter) listPushers() []cwlogs.Pusher { @@ -191,7 +194,7 @@ func (emf *emfExporter) listPushers() []cwlogs.Pusher { defer emf.pusherMapLock.Unlock() var pushers []cwlogs.Pusher - for _, pusher := range emf.pusherMap { + for _, pusher := range emf.boundedPusherMap.listAllPushers() { pushers = append(pushers, pusher) } return pushers diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 40430d17bfb9..437ddf4b338b 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -11,7 +11,6 @@ import ( "strings" "time" - "github.com/aws/aws-sdk-go/aws" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pmetric" "go.uber.org/multierr" @@ -41,8 +40,14 @@ const ( serviceName = "Name" keyAttributeEntityDeploymentEnvironment = "aws.entity.deployment.environment" deploymentEnvironment = "Environment" + keyAttributeEntityType = "aws.entity.type" entityType = "Type" service = "Service" + resource = "Resource" + keyAttributeEntityResourceType = "aws.entity.resource.type" + resourceType = "ResourceType" + keyAttributeEntityIdentifier = "aws.entity.resource.identifier" + identifier = "Identifier" attributeEntityCluster = "aws.entity.k8s.cluster.name" cluster = "Cluster" attributeEntityNamespace = "aws.entity.k8s.namespace.name" @@ -54,6 +59,9 @@ const ( ) var keyAttributeEntityToShortNameMap = map[string]string{ + keyAttributeEntityType: entityType, + keyAttributeEntityResourceType: resourceType, + keyAttributeEntityIdentifier: identifier, keyAttributeEntityServiceName: serviceName, keyAttributeEntityDeploymentEnvironment: deploymentEnvironment, } @@ -63,6 +71,7 @@ var attributeEntityToShortNameMap = map[string]string{ attributeEntityNamespace: namespace, attributeEntityWorkload: workload, attributeEntityNode: node, + // TODO: add attributes for EC2 } var errMissingMetricsForEnhancedContainerInsights = errors.New("nil event detected with EnhancedContainerInsights enabled") @@ -210,9 +219,7 @@ func fetchEntityFields(resourceAttributes pcommon.Map) (cloudwatchlogs.Entity, p // to remove the entity fields from the attributes mutableResourceAttributes := pcommon.NewMap() resourceAttributes.CopyTo(mutableResourceAttributes) - keyAttributesMap := map[string]*string{ - entityType: aws.String(service), - } + keyAttributesMap := map[string]*string{} attributeMap := map[string]*string{} processAttributes(keyAttributeEntityToShortNameMap, keyAttributesMap, mutableResourceAttributes) diff --git a/internal/aws/cwlogs/cwlog_client_test.go b/internal/aws/cwlogs/cwlog_client_test.go index 7dd2e5858b1b..9c84d98c50fe 100644 --- a/internal/aws/cwlogs/cwlog_client_test.go +++ b/internal/aws/cwlogs/cwlog_client_test.go @@ -89,6 +89,10 @@ var entity = cloudwatchlogs.Entity{ "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), }, + Attributes: map[string]*string{ + "Instance": aws.String("i-1234567"), + "Type": aws.String("AWS::EC2"), + }, } var emptySequenceToken = "" From 56b7329535707e3d49ce95b5358e5caa5efdbed9 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 17 Sep 2024 14:37:31 -0400 Subject: [PATCH 34/45] attempting to fix the resource attributes issue. --- exporter/awsemfexporter/metric_translator.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 437ddf4b338b..5648cdcc75ee 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -179,9 +179,11 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri metricReceiver = containerInsightsReceiver } } - - entity, resourceAttributes := fetchEntityFields(rm.Resource().Attributes()) - resourceAttributes.CopyTo(rm.Resource().Attributes()) + newResourceMetrics := pmetric.NewResourceMetrics() + rm.CopyTo(newResourceMetrics) + entity, _ := fetchEntityFields(newResourceMetrics.Resource().Attributes()) + //resourceAttributes.CopyTo(rm.Resource().Attributes()) + rm = newResourceMetrics for j := 0; j < ilms.Len(); j++ { ilm := ilms.At(j) From 820e414526d12117d47118113a30dcddc8bc81b6 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 17 Sep 2024 16:25:10 -0400 Subject: [PATCH 35/45] fixes tests, and bug with fetching existing pusher --- exporter/awsemfexporter/emf_exporter.go | 5 ++- exporter/awsemfexporter/emf_exporter_test.go | 32 ++++++++----------- exporter/awsemfexporter/metric_translator.go | 17 ++++------ .../awsemfexporter/metric_translator_test.go | 6 ++-- 4 files changed, 26 insertions(+), 34 deletions(-) diff --git a/exporter/awsemfexporter/emf_exporter.go b/exporter/awsemfexporter/emf_exporter.go index 2915205b4636..8c70e2b50e19 100644 --- a/exporter/awsemfexporter/emf_exporter.go +++ b/exporter/awsemfexporter/emf_exporter.go @@ -178,14 +178,13 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e func (emf *emfExporter) getPusher(key cwlogs.StreamKey) cwlogs.Pusher { var ok bool - var pusher cwlogs.Pusher hash := key.Hash() - if _, ok = emf.boundedPusherMap.get(hash); !ok { + var pusher cwlogs.Pusher + if pusher, ok = emf.boundedPusherMap.get(hash); !ok { pusher = cwlogs.NewPusher(key, emf.retryCnt, *emf.svcStructuredLog, emf.config.logger) emf.boundedPusherMap.add(hash, pusher, emf.config.logger) } - //return emf.pusherMap[hash] return pusher } diff --git a/exporter/awsemfexporter/emf_exporter_test.go b/exporter/awsemfexporter/emf_exporter_test.go index 59c976cc02d8..88c2df879b14 100644 --- a/exporter/awsemfexporter/emf_exporter_test.go +++ b/exporter/awsemfexporter/emf_exporter_test.go @@ -197,12 +197,10 @@ func TestConsumeMetricsWithLogGroupStreamConfig(t *testing.T) { streamKey := &cwlogs.StreamKey{ LogGroupName: expCfg.LogGroupName, LogStreamName: expCfg.LogStreamName, - Entity: &cloudwatchlogs.Entity{KeyAttributes: map[string]*string{ - entityType: aws.String(service), - }}, + Entity: &cloudwatchlogs.Entity{}, } expectedStreamKeyHash := streamKey.Hash() - pusherMap, ok := exp.pusherMap[expectedStreamKeyHash] + pusherMap, ok := exp.boundedPusherMap.get(expectedStreamKeyHash) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -229,6 +227,7 @@ func TestConsumeMetricsWithLogGroupStreamValidPlaceholder(t *testing.T) { keyAttributeEntityServiceName: "myService", keyAttributeEntityDeploymentEnvironment: "myEnvironment", attributeEntityCluster: "test-cluster-name", + keyAttributeEntityType: "Service", }, }) require.Error(t, exp.pushMetricsData(ctx, md)) @@ -241,12 +240,12 @@ func TestConsumeMetricsWithLogGroupStreamValidPlaceholder(t *testing.T) { "Cluster": aws.String("test-cluster-name"), }, KeyAttributes: map[string]*string{ - "Type": aws.String(service), + "Type": aws.String("Service"), "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), }}, } - pusherMap, ok := exp.pusherMap[streamKey.Hash()] + pusherMap, ok := exp.boundedPusherMap.get(streamKey.Hash()) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -265,7 +264,7 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { assert.NotNil(t, exp) var entity = &cloudwatchlogs.Entity{ KeyAttributes: map[string]*string{ - "Type": aws.String(service), + "Type": aws.String("Service"), "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), }, @@ -279,6 +278,7 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { "aws.ecs.task.id": "test-task-id", keyAttributeEntityServiceName: "myService", keyAttributeEntityDeploymentEnvironment: "myEnvironment", + keyAttributeEntityType: service, }, }) require.Error(t, exp.pushMetricsData(ctx, md)) @@ -288,7 +288,7 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { LogStreamName: "test-task-id", Entity: entity, } - pusherMap, ok := exp.pusherMap[streamKey.Hash()] + pusherMap, ok := exp.boundedPusherMap.get(streamKey.Hash()) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -312,6 +312,7 @@ func TestConsumeMetricsWithWrongPlaceholder(t *testing.T) { resourceAttributeMap: map[string]any{ "aws.ecs.cluster.name": "test-cluster-name", "aws.ecs.task.id": "test-task-id", + keyAttributeEntityType: service, keyAttributeEntityServiceName: "myService", keyAttributeEntityDeploymentEnvironment: "myEnvironment", }, @@ -323,13 +324,13 @@ func TestConsumeMetricsWithWrongPlaceholder(t *testing.T) { LogStreamName: expCfg.LogStreamName, Entity: &cloudwatchlogs.Entity{ KeyAttributes: map[string]*string{ - "Type": aws.String(service), + "Type": aws.String("Service"), "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), }, }, } - pusherMap, ok := exp.pusherMap[streamKey.Hash()] + pusherMap, ok := exp.boundedPusherMap.get(streamKey.Hash()) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -353,18 +354,13 @@ func TestPushMetricsDataWithErr(t *testing.T) { logPusher.On("ForceFlush", nil).Return("some error").Once() logPusher.On("ForceFlush", nil).Return("").Once() logPusher.On("ForceFlush", nil).Return("some error").Once() - exp.pusherMap = map[string]cwlogs.Pusher{} + exp.boundedPusherMap = newBoundedPusherMap() streamKey := cwlogs.StreamKey{ LogGroupName: "test-logGroupName", LogStreamName: "test-logStreamName", - Entity: &cloudwatchlogs.Entity{ - KeyAttributes: map[string]*string{ - "Type": aws.String(service), - }, - }, + Entity: &cloudwatchlogs.Entity{}, } - exp.pusherMap[streamKey.Hash()] = logPusher - + exp.boundedPusherMap.add(streamKey.Hash(), logPusher, zap.NewExample()) md := generateTestMetrics(testMetric{ metricNames: []string{"metric_1", "metric_2"}, metricValues: [][]float64{{100}, {4}}, diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 5648cdcc75ee..54614570dc5b 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -179,10 +179,11 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri metricReceiver = containerInsightsReceiver } } + // the original resourceAttributes map is immutable, so we need to create a mutable copy of the resource metrics + // to remove the entity fields from the attributes newResourceMetrics := pmetric.NewResourceMetrics() rm.CopyTo(newResourceMetrics) - entity, _ := fetchEntityFields(newResourceMetrics.Resource().Attributes()) - //resourceAttributes.CopyTo(rm.Resource().Attributes()) + entity := fetchEntityFields(newResourceMetrics.Resource().Attributes()) rm = newResourceMetrics for j := 0; j < ilms.Len(); j++ { @@ -216,21 +217,17 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri return nil } -func fetchEntityFields(resourceAttributes pcommon.Map) (cloudwatchlogs.Entity, pcommon.Map) { - // the original resourceAttributes map is immutable, so we need to create a mutable copy - // to remove the entity fields from the attributes - mutableResourceAttributes := pcommon.NewMap() - resourceAttributes.CopyTo(mutableResourceAttributes) +func fetchEntityFields(resourceAttributes pcommon.Map) cloudwatchlogs.Entity { keyAttributesMap := map[string]*string{} attributeMap := map[string]*string{} - processAttributes(keyAttributeEntityToShortNameMap, keyAttributesMap, mutableResourceAttributes) - processAttributes(attributeEntityToShortNameMap, attributeMap, mutableResourceAttributes) + processAttributes(keyAttributeEntityToShortNameMap, keyAttributesMap, resourceAttributes) + processAttributes(attributeEntityToShortNameMap, attributeMap, resourceAttributes) return cloudwatchlogs.Entity{ KeyAttributes: keyAttributesMap, Attributes: attributeMap, - }, mutableResourceAttributes + } } // translateGroupedMetricToCWMetric converts Grouped Metric format to CloudWatch Metric format. diff --git a/exporter/awsemfexporter/metric_translator_test.go b/exporter/awsemfexporter/metric_translator_test.go index 12ac242cb8c4..246b552b0c6f 100644 --- a/exporter/awsemfexporter/metric_translator_test.go +++ b/exporter/awsemfexporter/metric_translator_test.go @@ -2579,6 +2579,7 @@ func TestTranslateOtToGroupedMetricForInitialDeltaValue(t *testing.T) { func TestFetchEntityFields(t *testing.T) { resourceMetrics := pmetric.NewResourceMetrics() + resourceMetrics.Resource().Attributes().PutStr(keyAttributeEntityType, "Service") resourceMetrics.Resource().Attributes().PutStr(keyAttributeEntityDeploymentEnvironment, "my-environment") resourceMetrics.Resource().Attributes().PutStr(keyAttributeEntityServiceName, "my-service") resourceMetrics.Resource().Attributes().PutStr(attributeEntityNode, "my-node") @@ -2598,10 +2599,9 @@ func TestFetchEntityFields(t *testing.T) { workload: aws.String("my-workload"), }, } - - entity, attrs := fetchEntityFields(resourceMetrics.Resource().Attributes()) + assert.Equal(t, 7, resourceMetrics.Resource().Attributes().Len()) + entity := fetchEntityFields(resourceMetrics.Resource().Attributes()) assert.Equal(t, expectedEntity, entity) - attrs.CopyTo(resourceMetrics.Resource().Attributes()) assert.Equal(t, 0, resourceMetrics.Resource().Attributes().Len()) } From 76f15470bbb5b90c1777de770f4bcf3d133b7a7b Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 17 Sep 2024 16:29:54 -0400 Subject: [PATCH 36/45] adds debug line for entity --- internal/aws/cwlogs/pusher.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index 64e18e8a587c..b6cbe837e273 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -298,10 +298,11 @@ func (p *logPusher) pushEventBatch(req any) error { return err } - p.logger.Debug("logpusher: publish log events successfully.", + p.logger.Info("logpusher: publish log events successfully.", zap.Int("NumOfLogEvents", len(putLogEventsInput.LogEvents)), zap.Float64("LogEventsSize", float64(logEventBatch.byteTotal)/float64(1024)), zap.Int64("Time", time.Since(startTime).Nanoseconds()/int64(time.Millisecond)), + zap.String("Entity", p.entity.GoString()), ) return nil From 776ac6ad319ee6aa0be276be09b9cd20fa9165e5 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 17 Sep 2024 17:13:30 -0400 Subject: [PATCH 37/45] Revert "adds debug line for entity" This reverts commit 76f15470bbb5b90c1777de770f4bcf3d133b7a7b. --- internal/aws/cwlogs/pusher.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index b6cbe837e273..64e18e8a587c 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -298,11 +298,10 @@ func (p *logPusher) pushEventBatch(req any) error { return err } - p.logger.Info("logpusher: publish log events successfully.", + p.logger.Debug("logpusher: publish log events successfully.", zap.Int("NumOfLogEvents", len(putLogEventsInput.LogEvents)), zap.Float64("LogEventsSize", float64(logEventBatch.byteTotal)/float64(1024)), zap.Int64("Time", time.Since(startTime).Nanoseconds()/int64(time.Millisecond)), - zap.String("Entity", p.entity.GoString()), ) return nil From 0742057acb163e1994ca9191fa4ed91b8f847b7a Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 17 Sep 2024 17:30:18 -0400 Subject: [PATCH 38/45] fixes lint check --- exporter/awsemfexporter/bounded_pusher_map.go | 4 +++- exporter/awsemfexporter/bounded_pusher_map_test.go | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 exporter/awsemfexporter/bounded_pusher_map_test.go diff --git a/exporter/awsemfexporter/bounded_pusher_map.go b/exporter/awsemfexporter/bounded_pusher_map.go index 98fc7935dad5..9c30df1a67c8 100644 --- a/exporter/awsemfexporter/bounded_pusher_map.go +++ b/exporter/awsemfexporter/bounded_pusher_map.go @@ -1,5 +1,7 @@ -package awsemfexporter +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 +package awsemfexporter // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsemfexporter" import ( "errors" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" diff --git a/exporter/awsemfexporter/bounded_pusher_map_test.go b/exporter/awsemfexporter/bounded_pusher_map_test.go new file mode 100644 index 000000000000..fc482c86b136 --- /dev/null +++ b/exporter/awsemfexporter/bounded_pusher_map_test.go @@ -0,0 +1 @@ +package awsemfexporter From f6d5d885bfba83542109687eded94528e5481bd5 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Tue, 17 Sep 2024 17:32:26 -0400 Subject: [PATCH 39/45] fixes lint check --- exporter/awsemfexporter/bounded_pusher_map.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/exporter/awsemfexporter/bounded_pusher_map.go b/exporter/awsemfexporter/bounded_pusher_map.go index 9c30df1a67c8..98fc7935dad5 100644 --- a/exporter/awsemfexporter/bounded_pusher_map.go +++ b/exporter/awsemfexporter/bounded_pusher_map.go @@ -1,7 +1,5 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 +package awsemfexporter -package awsemfexporter // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsemfexporter" import ( "errors" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" From b6bb0724bf67d75533a9f44f95847a7de55e8f61 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Wed, 18 Sep 2024 00:07:00 -0400 Subject: [PATCH 40/45] adds unit tests for bounded pusher map --- exporter/awsemfexporter/bounded_pusher_map.go | 19 +-- .../awsemfexporter/bounded_pusher_map_test.go | 128 ++++++++++++++++++ exporter/awsemfexporter/emf_exporter.go | 10 +- exporter/awsemfexporter/emf_exporter_test.go | 12 +- 4 files changed, 150 insertions(+), 19 deletions(-) diff --git a/exporter/awsemfexporter/bounded_pusher_map.go b/exporter/awsemfexporter/bounded_pusher_map.go index 98fc7935dad5..6725ea9417ff 100644 --- a/exporter/awsemfexporter/bounded_pusher_map.go +++ b/exporter/awsemfexporter/bounded_pusher_map.go @@ -1,3 +1,6 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + package awsemfexporter import ( @@ -11,22 +14,22 @@ const ( pusherMapLimit = 1000 ) -type boundedPusherMap struct { +type BoundedPusherMap struct { pusherMap map[string]cwlogs.Pusher limit int stalePusherTracker map[string]time.Time } -func newBoundedPusherMap() boundedPusherMap { - return boundedPusherMap{ +func NewBoundedPusherMap() BoundedPusherMap { + return BoundedPusherMap{ pusherMap: make(map[string]cwlogs.Pusher), limit: pusherMapLimit, stalePusherTracker: make(map[string]time.Time), } } -func (bpm *boundedPusherMap) add(key string, pusher cwlogs.Pusher, logger *zap.Logger) { - err := bpm.evictStalePushers() +func (bpm *BoundedPusherMap) Add(key string, pusher cwlogs.Pusher, logger *zap.Logger) { + err := bpm.EvictStalePushers() if err != nil { logger.Error("Error with evicting stale pushers", zap.Error(err)) return @@ -35,7 +38,7 @@ func (bpm *boundedPusherMap) add(key string, pusher cwlogs.Pusher, logger *zap.L bpm.stalePusherTracker[key] = time.Now() } -func (bpm *boundedPusherMap) get(key string) (cwlogs.Pusher, bool) { +func (bpm *BoundedPusherMap) Get(key string) (cwlogs.Pusher, bool) { pusher, ok := bpm.pusherMap[key] if ok { bpm.stalePusherTracker[key] = time.Now() @@ -43,7 +46,7 @@ func (bpm *boundedPusherMap) get(key string) (cwlogs.Pusher, bool) { return pusher, ok } -func (bpm *boundedPusherMap) evictStalePushers() error { +func (bpm *BoundedPusherMap) EvictStalePushers() error { if len(bpm.pusherMap) < bpm.limit { return nil } @@ -62,7 +65,7 @@ func (bpm *boundedPusherMap) evictStalePushers() error { return nil } -func (bpm *boundedPusherMap) listAllPushers() []cwlogs.Pusher { +func (bpm *BoundedPusherMap) ListAllPushers() []cwlogs.Pusher { var pushers []cwlogs.Pusher for _, pusher := range bpm.pusherMap { pushers = append(pushers, pusher) diff --git a/exporter/awsemfexporter/bounded_pusher_map_test.go b/exporter/awsemfexporter/bounded_pusher_map_test.go index fc482c86b136..c56c01a56c1a 100644 --- a/exporter/awsemfexporter/bounded_pusher_map_test.go +++ b/exporter/awsemfexporter/bounded_pusher_map_test.go @@ -1 +1,129 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + package awsemfexporter + +import ( + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" + "github.com/stretchr/testify/assert" + "go.uber.org/zap" + "testing" + "time" +) + +// MockPusher implements the cwlogs.Pusher interface for testing +type MockPusher struct{} + +func (m MockPusher) AddLogEntry(_ *cwlogs.Event) error { + return nil +} + +func (m MockPusher) ForceFlush() error { + return nil +} + +func TestNewBoundedPusherMap(t *testing.T) { + bpm := NewBoundedPusherMap() + assert.Equal(t, pusherMapLimit, bpm.limit) + assert.Empty(t, bpm.pusherMap) + assert.Empty(t, bpm.stalePusherTracker) +} + +func TestBoundedPusherMap_Add(t *testing.T) { + bpm := NewBoundedPusherMap() + logger := zap.NewNop() + pusher := MockPusher{} + + bpm.Add("key1", pusher, logger) + assert.Len(t, bpm.pusherMap, 1) + assert.Len(t, bpm.stalePusherTracker, 1) + assert.Contains(t, bpm.pusherMap, "key1") + assert.Contains(t, bpm.stalePusherTracker, "key1") +} + +func TestBoundedPusherMap_Get(t *testing.T) { + bpm := NewBoundedPusherMap() + pusher := MockPusher{} + bpm.pusherMap["key1"] = pusher + bpm.stalePusherTracker["key1"] = time.Now().Add(-30 * time.Minute) + + // Test getting an existing pusher + gotPusher, ok := bpm.Get("key1") + assert.True(t, ok) + assert.Equal(t, pusher, gotPusher) + assert.True(t, bpm.stalePusherTracker["key1"].After(time.Now().Add(-1*time.Second))) + + // Test getting a non-existent pusher + gotPusher, ok = bpm.Get("key2") + assert.False(t, ok) + assert.Nil(t, gotPusher) +} + +func TestBoundedPusherMap_EvictStalePushers(t *testing.T) { + bpm := NewBoundedPusherMap() + bpm.limit = 2 + pusher := MockPusher{} + + // Add two pushers, one stale and one fresh + bpm.pusherMap["stale"] = pusher + bpm.stalePusherTracker["stale"] = time.Now().Add(-2 * time.Hour) + bpm.pusherMap["fresh"] = pusher + bpm.stalePusherTracker["fresh"] = time.Now() + + err := bpm.EvictStalePushers() + assert.NoError(t, err) + assert.Len(t, bpm.pusherMap, 1) + assert.Len(t, bpm.stalePusherTracker, 1) + assert.NotContains(t, bpm.pusherMap, "stale") + assert.NotContains(t, bpm.stalePusherTracker, "stale") + assert.Contains(t, bpm.pusherMap, "fresh") + assert.Contains(t, bpm.stalePusherTracker, "fresh") +} + +func TestBoundedPusherMap_EvictStalePushers_Error(t *testing.T) { + bpm := NewBoundedPusherMap() + bpm.limit = 2 + pusher := MockPusher{} + + // Add two fresh pushers + bpm.pusherMap["key1"] = pusher + bpm.pusherMap["key2"] = pusher + bpm.stalePusherTracker["key1"] = time.Now() + bpm.stalePusherTracker["key2"] = time.Now() + + err := bpm.EvictStalePushers() + assert.Error(t, err) + assert.Equal(t, "too many emf pushers being created. Dropping the request", err.Error()) +} + +func TestBoundedPusherMap_ListAllPushers(t *testing.T) { + bpm := NewBoundedPusherMap() + pusher1 := MockPusher{} + pusher2 := MockPusher{} + bpm.Add("key1", pusher1, zap.NewExample()) + bpm.Add("key2", pusher2, zap.NewExample()) + + pushers := bpm.ListAllPushers() + assert.Len(t, pushers, 2) + assert.Contains(t, pushers, pusher1) + assert.Contains(t, pushers, pusher2) +} + +func TestBoundedPusherMap_Add_EvictionError(t *testing.T) { + bpm := NewBoundedPusherMap() + bpm.limit = 1 + logger := zap.NewNop() + pusher := MockPusher{} + + // Add one pusher to reach the limit + bpm.Add("key1", pusher, logger) + + // Try to add another pusher, which should trigger eviction + bpm.Add("key2", pusher, logger) + + // Check that the second pusher was not added due to eviction error + assert.Len(t, bpm.pusherMap, 1) + assert.Len(t, bpm.stalePusherTracker, 1) + assert.Contains(t, bpm.pusherMap, "key1") + assert.NotContains(t, bpm.pusherMap, "key2") +} diff --git a/exporter/awsemfexporter/emf_exporter.go b/exporter/awsemfexporter/emf_exporter.go index 8c70e2b50e19..2eb7ebbc1673 100644 --- a/exporter/awsemfexporter/emf_exporter.go +++ b/exporter/awsemfexporter/emf_exporter.go @@ -35,7 +35,7 @@ const ( ) type emfExporter struct { - boundedPusherMap boundedPusherMap + boundedPusherMap BoundedPusherMap svcStructuredLog *cwlogs.Client config *Config @@ -86,7 +86,7 @@ func newEmfExporter(config *Config, set exporter.Settings) (*emfExporter, error) retryCnt: *awsConfig.MaxRetries, collectorID: collectorIdentifier.String(), processResourceLabels: func(map[string]string) {}, - boundedPusherMap: newBoundedPusherMap(), + boundedPusherMap: NewBoundedPusherMap(), } if config.IsAppSignalsEnabled() { @@ -180,9 +180,9 @@ func (emf *emfExporter) getPusher(key cwlogs.StreamKey) cwlogs.Pusher { var ok bool hash := key.Hash() var pusher cwlogs.Pusher - if pusher, ok = emf.boundedPusherMap.get(hash); !ok { + if pusher, ok = emf.boundedPusherMap.Get(hash); !ok { pusher = cwlogs.NewPusher(key, emf.retryCnt, *emf.svcStructuredLog, emf.config.logger) - emf.boundedPusherMap.add(hash, pusher, emf.config.logger) + emf.boundedPusherMap.Add(hash, pusher, emf.config.logger) } return pusher @@ -193,7 +193,7 @@ func (emf *emfExporter) listPushers() []cwlogs.Pusher { defer emf.pusherMapLock.Unlock() var pushers []cwlogs.Pusher - for _, pusher := range emf.boundedPusherMap.listAllPushers() { + for _, pusher := range emf.boundedPusherMap.ListAllPushers() { pushers = append(pushers, pusher) } return pushers diff --git a/exporter/awsemfexporter/emf_exporter_test.go b/exporter/awsemfexporter/emf_exporter_test.go index 88c2df879b14..f6a66b4d67f5 100644 --- a/exporter/awsemfexporter/emf_exporter_test.go +++ b/exporter/awsemfexporter/emf_exporter_test.go @@ -200,7 +200,7 @@ func TestConsumeMetricsWithLogGroupStreamConfig(t *testing.T) { Entity: &cloudwatchlogs.Entity{}, } expectedStreamKeyHash := streamKey.Hash() - pusherMap, ok := exp.boundedPusherMap.get(expectedStreamKeyHash) + pusherMap, ok := exp.boundedPusherMap.Get(expectedStreamKeyHash) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -245,7 +245,7 @@ func TestConsumeMetricsWithLogGroupStreamValidPlaceholder(t *testing.T) { "Environment": aws.String("myEnvironment"), }}, } - pusherMap, ok := exp.boundedPusherMap.get(streamKey.Hash()) + pusherMap, ok := exp.boundedPusherMap.Get(streamKey.Hash()) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -288,7 +288,7 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { LogStreamName: "test-task-id", Entity: entity, } - pusherMap, ok := exp.boundedPusherMap.get(streamKey.Hash()) + pusherMap, ok := exp.boundedPusherMap.Get(streamKey.Hash()) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -330,7 +330,7 @@ func TestConsumeMetricsWithWrongPlaceholder(t *testing.T) { }, }, } - pusherMap, ok := exp.boundedPusherMap.get(streamKey.Hash()) + pusherMap, ok := exp.boundedPusherMap.Get(streamKey.Hash()) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -354,13 +354,13 @@ func TestPushMetricsDataWithErr(t *testing.T) { logPusher.On("ForceFlush", nil).Return("some error").Once() logPusher.On("ForceFlush", nil).Return("").Once() logPusher.On("ForceFlush", nil).Return("some error").Once() - exp.boundedPusherMap = newBoundedPusherMap() + exp.boundedPusherMap = NewBoundedPusherMap() streamKey := cwlogs.StreamKey{ LogGroupName: "test-logGroupName", LogStreamName: "test-logStreamName", Entity: &cloudwatchlogs.Entity{}, } - exp.boundedPusherMap.add(streamKey.Hash(), logPusher, zap.NewExample()) + exp.boundedPusherMap.Add(streamKey.Hash(), logPusher, zap.NewExample()) md := generateTestMetrics(testMetric{ metricNames: []string{"metric_1", "metric_2"}, metricValues: [][]float64{{100}, {4}}, From 2eb0318df8168294063b24b3ceadc8c5bc196788 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Wed, 18 Sep 2024 00:09:03 -0400 Subject: [PATCH 41/45] lint issues --- exporter/awsemfexporter/bounded_pusher_map.go | 6 ++++-- exporter/awsemfexporter/bounded_pusher_map_test.go | 8 +++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/exporter/awsemfexporter/bounded_pusher_map.go b/exporter/awsemfexporter/bounded_pusher_map.go index 6725ea9417ff..dec5ac3a096a 100644 --- a/exporter/awsemfexporter/bounded_pusher_map.go +++ b/exporter/awsemfexporter/bounded_pusher_map.go @@ -5,9 +5,11 @@ package awsemfexporter import ( "errors" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" - "go.uber.org/zap" "time" + + "go.uber.org/zap" + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" ) const ( diff --git a/exporter/awsemfexporter/bounded_pusher_map_test.go b/exporter/awsemfexporter/bounded_pusher_map_test.go index c56c01a56c1a..5f6c2dfa9564 100644 --- a/exporter/awsemfexporter/bounded_pusher_map_test.go +++ b/exporter/awsemfexporter/bounded_pusher_map_test.go @@ -4,11 +4,13 @@ package awsemfexporter import ( - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" - "github.com/stretchr/testify/assert" - "go.uber.org/zap" "testing" "time" + + "github.com/stretchr/testify/assert" + "go.uber.org/zap" + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" ) // MockPusher implements the cwlogs.Pusher interface for testing From e81b01dcac031cc96aa69133776ab93b3cccbfd4 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Wed, 18 Sep 2024 00:57:57 -0400 Subject: [PATCH 42/45] fixes logic to remove entity attributes --- exporter/awsemfexporter/emf_exporter.go | 21 +++++++++++++----- exporter/awsemfexporter/emf_exporter_test.go | 22 +++++++++++++++++++ exporter/awsemfexporter/metric_translator.go | 7 +----- .../awsemfexporter/metric_translator_test.go | 2 -- exporter/awsemfexporter/util.go | 2 +- exporter/awsemfexporter/util_test.go | 8 ------- 6 files changed, 39 insertions(+), 23 deletions(-) diff --git a/exporter/awsemfexporter/emf_exporter.go b/exporter/awsemfexporter/emf_exporter.go index 2eb7ebbc1673..104051a7144f 100644 --- a/exporter/awsemfexporter/emf_exporter.go +++ b/exporter/awsemfexporter/emf_exporter.go @@ -103,7 +103,11 @@ func newEmfExporter(config *Config, set exporter.Settings) (*emfExporter, error) } func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) error { - rms := md.ResourceMetrics() + // the original resourceAttributes map is immutable, so we need to create a mutable copy of the resource metrics + // to remove the entity fields from the attributes + mutableMetrics := pmetric.NewMetrics() + md.CopyTo(mutableMetrics) + rms := mutableMetrics.ResourceMetrics() labels := map[string]string{} for i := 0; i < rms.Len(); i++ { rm := rms.At(i) @@ -127,6 +131,7 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e if err != nil { return err } + rms.At(i).Resource().Attributes().RemoveIf(filterEntityAttributes()) } for _, groupedMetric := range groupedMetrics { @@ -176,6 +181,14 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e return nil } +func filterEntityAttributes() func(s string, _ pcommon.Value) bool { + return func(s string, _ pcommon.Value) bool { + _, containsKeyAttribute := keyAttributeEntityToShortNameMap[s] + _, containsAttribute := keyAttributeEntityToShortNameMap[s] + return containsKeyAttribute || containsAttribute + } +} + func (emf *emfExporter) getPusher(key cwlogs.StreamKey) cwlogs.Pusher { var ok bool hash := key.Hash() @@ -192,11 +205,7 @@ func (emf *emfExporter) listPushers() []cwlogs.Pusher { emf.pusherMapLock.Lock() defer emf.pusherMapLock.Unlock() - var pushers []cwlogs.Pusher - for _, pusher := range emf.boundedPusherMap.ListAllPushers() { - pushers = append(pushers, pusher) - } - return pushers + return emf.boundedPusherMap.ListAllPushers() } func (emf *emfExporter) start(_ context.Context, host component.Host) error { diff --git a/exporter/awsemfexporter/emf_exporter_test.go b/exporter/awsemfexporter/emf_exporter_test.go index f6a66b4d67f5..b890dabafd97 100644 --- a/exporter/awsemfexporter/emf_exporter_test.go +++ b/exporter/awsemfexporter/emf_exporter_test.go @@ -75,6 +75,28 @@ func TestConsumeMetrics(t *testing.T) { require.NoError(t, exp.shutdown(ctx)) } +func TestFilterEntities(t *testing.T) { + md := generateTestMetrics(testMetric{ + metricNames: []string{"metric_1", "metric_2"}, + metricValues: [][]float64{{100}, {4}}, + resourceAttributeMap: map[string]any{ + "aws.ecs.cluster.name": "test-cluster-name", + "aws.ecs.task.id": "test-task-id", + keyAttributeEntityType: service, + keyAttributeEntityServiceName: "myService", + keyAttributeEntityDeploymentEnvironment: "myEnvironment", + }, + }) + + rms := md.ResourceMetrics() + + for i := 0; i < rms.Len(); i++ { + assert.Equal(t, 5, rms.At(i).Resource().Attributes().Len()) + rms.At(i).Resource().Attributes().RemoveIf(filterEntityAttributes()) + assert.Equal(t, 2, rms.At(i).Resource().Attributes().Len()) + } +} + func TestConsumeMetricsWithNaNValues(t *testing.T) { tests := []struct { testName string diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 54614570dc5b..2733effbaaca 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -179,12 +179,7 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri metricReceiver = containerInsightsReceiver } } - // the original resourceAttributes map is immutable, so we need to create a mutable copy of the resource metrics - // to remove the entity fields from the attributes - newResourceMetrics := pmetric.NewResourceMetrics() - rm.CopyTo(newResourceMetrics) - entity := fetchEntityFields(newResourceMetrics.Resource().Attributes()) - rm = newResourceMetrics + entity := fetchEntityFields(rm.Resource().Attributes()) for j := 0; j < ilms.Len(); j++ { ilm := ilms.At(j) diff --git a/exporter/awsemfexporter/metric_translator_test.go b/exporter/awsemfexporter/metric_translator_test.go index 246b552b0c6f..54848f80780f 100644 --- a/exporter/awsemfexporter/metric_translator_test.go +++ b/exporter/awsemfexporter/metric_translator_test.go @@ -2599,10 +2599,8 @@ func TestFetchEntityFields(t *testing.T) { workload: aws.String("my-workload"), }, } - assert.Equal(t, 7, resourceMetrics.Resource().Attributes().Len()) entity := fetchEntityFields(resourceMetrics.Resource().Attributes()) assert.Equal(t, expectedEntity, entity) - assert.Equal(t, 0, resourceMetrics.Resource().Attributes().Len()) } diff --git a/exporter/awsemfexporter/util.go b/exporter/awsemfexporter/util.go index 61e3fcfbb57d..12a6d46aeb38 100644 --- a/exporter/awsemfexporter/util.go +++ b/exporter/awsemfexporter/util.go @@ -182,7 +182,7 @@ func processAttributes(entityMap map[string]string, targetMap map[string]*string if strVal := val.Str(); strVal != "" { targetMap[shortName] = aws.String(strVal) } - mutableResourceAttributes.Remove(entityField) + //mutableResourceAttributes.Remove(entityField) } } } diff --git a/exporter/awsemfexporter/util_test.go b/exporter/awsemfexporter/util_test.go index f52542e594d7..73e0ab9b6bae 100644 --- a/exporter/awsemfexporter/util_test.go +++ b/exporter/awsemfexporter/util_test.go @@ -374,7 +374,6 @@ func TestProcessAttributes(t *testing.T) { entityMap []map[string]string resourceAttributes map[string]any wantedAttributes map[string]*string - leftoverAttributes map[string]any }{ { name: "key_attributes", @@ -387,7 +386,6 @@ func TestProcessAttributes(t *testing.T) { serviceName: aws.String("my-service"), deploymentEnvironment: aws.String("my-environment"), }, - leftoverAttributes: make(map[string]any), }, { name: "non-key_attributes", @@ -404,7 +402,6 @@ func TestProcessAttributes(t *testing.T) { node: aws.String("my-node"), workload: aws.String("my-workload"), }, - leftoverAttributes: make(map[string]any), }, { name: "key_and_non_key_attributes", @@ -425,7 +422,6 @@ func TestProcessAttributes(t *testing.T) { node: aws.String("my-node"), workload: aws.String("my-workload"), }, - leftoverAttributes: make(map[string]any), }, { name: "key_and_non_key_attributes_plus_extras", @@ -447,9 +443,6 @@ func TestProcessAttributes(t *testing.T) { node: aws.String("my-node"), workload: aws.String("my-workload"), }, - leftoverAttributes: map[string]any{ - "extra_attribute": "extra_value", - }, }, } @@ -462,7 +455,6 @@ func TestProcessAttributes(t *testing.T) { for _, entityMap := range tc.entityMap { processAttributes(entityMap, targetMap, attrs) } - assert.Equal(t, tc.leftoverAttributes, attrs.AsRaw()) assert.Equal(t, tc.wantedAttributes, targetMap) }) } From 84eb70f00ce9ffcd0ce91449df2fa874ba1a4a6d Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Wed, 18 Sep 2024 12:33:21 -0400 Subject: [PATCH 43/45] reverts entity attribute refactoring --- exporter/awsemfexporter/emf_exporter.go | 15 +------------ exporter/awsemfexporter/emf_exporter_test.go | 22 -------------------- exporter/awsemfexporter/metric_translator.go | 9 ++++++-- exporter/awsemfexporter/util.go | 2 +- exporter/awsemfexporter/util_test.go | 8 +++++++ 5 files changed, 17 insertions(+), 39 deletions(-) diff --git a/exporter/awsemfexporter/emf_exporter.go b/exporter/awsemfexporter/emf_exporter.go index 104051a7144f..b44df9dc36a5 100644 --- a/exporter/awsemfexporter/emf_exporter.go +++ b/exporter/awsemfexporter/emf_exporter.go @@ -103,11 +103,7 @@ func newEmfExporter(config *Config, set exporter.Settings) (*emfExporter, error) } func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) error { - // the original resourceAttributes map is immutable, so we need to create a mutable copy of the resource metrics - // to remove the entity fields from the attributes - mutableMetrics := pmetric.NewMetrics() - md.CopyTo(mutableMetrics) - rms := mutableMetrics.ResourceMetrics() + rms := md.ResourceMetrics() labels := map[string]string{} for i := 0; i < rms.Len(); i++ { rm := rms.At(i) @@ -131,7 +127,6 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e if err != nil { return err } - rms.At(i).Resource().Attributes().RemoveIf(filterEntityAttributes()) } for _, groupedMetric := range groupedMetrics { @@ -181,14 +176,6 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e return nil } -func filterEntityAttributes() func(s string, _ pcommon.Value) bool { - return func(s string, _ pcommon.Value) bool { - _, containsKeyAttribute := keyAttributeEntityToShortNameMap[s] - _, containsAttribute := keyAttributeEntityToShortNameMap[s] - return containsKeyAttribute || containsAttribute - } -} - func (emf *emfExporter) getPusher(key cwlogs.StreamKey) cwlogs.Pusher { var ok bool hash := key.Hash() diff --git a/exporter/awsemfexporter/emf_exporter_test.go b/exporter/awsemfexporter/emf_exporter_test.go index b890dabafd97..f6a66b4d67f5 100644 --- a/exporter/awsemfexporter/emf_exporter_test.go +++ b/exporter/awsemfexporter/emf_exporter_test.go @@ -75,28 +75,6 @@ func TestConsumeMetrics(t *testing.T) { require.NoError(t, exp.shutdown(ctx)) } -func TestFilterEntities(t *testing.T) { - md := generateTestMetrics(testMetric{ - metricNames: []string{"metric_1", "metric_2"}, - metricValues: [][]float64{{100}, {4}}, - resourceAttributeMap: map[string]any{ - "aws.ecs.cluster.name": "test-cluster-name", - "aws.ecs.task.id": "test-task-id", - keyAttributeEntityType: service, - keyAttributeEntityServiceName: "myService", - keyAttributeEntityDeploymentEnvironment: "myEnvironment", - }, - }) - - rms := md.ResourceMetrics() - - for i := 0; i < rms.Len(); i++ { - assert.Equal(t, 5, rms.At(i).Resource().Attributes().Len()) - rms.At(i).Resource().Attributes().RemoveIf(filterEntityAttributes()) - assert.Equal(t, 2, rms.At(i).Resource().Attributes().Len()) - } -} - func TestConsumeMetricsWithNaNValues(t *testing.T) { tests := []struct { testName string diff --git a/exporter/awsemfexporter/metric_translator.go b/exporter/awsemfexporter/metric_translator.go index 2733effbaaca..a81553d1df08 100644 --- a/exporter/awsemfexporter/metric_translator.go +++ b/exporter/awsemfexporter/metric_translator.go @@ -165,7 +165,6 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri logGroup, logStream, patternReplaceSucceeded := getLogInfo(rm, cWNamespace, config) deltaInitialValue := config.RetainInitialValueOfDeltaMetric - ilms := rm.ScopeMetrics() var metricReceiver string if receiver, ok := rm.Resource().Attributes().Get(attributeReceiver); ok { metricReceiver = receiver.Str() @@ -179,7 +178,13 @@ func (mt metricTranslator) translateOTelToGroupedMetric(rm pmetric.ResourceMetri metricReceiver = containerInsightsReceiver } } - entity := fetchEntityFields(rm.Resource().Attributes()) + // the original resourceAttributes map is immutable, so we need to create a mutable copy of the resource metrics + // to remove the entity fields from the attributes + mutableMetrics := pmetric.NewResourceMetrics() + rm.CopyTo(mutableMetrics) + entity := fetchEntityFields(mutableMetrics.Resource().Attributes()) + + ilms := mutableMetrics.ScopeMetrics() for j := 0; j < ilms.Len(); j++ { ilm := ilms.At(j) diff --git a/exporter/awsemfexporter/util.go b/exporter/awsemfexporter/util.go index 12a6d46aeb38..61e3fcfbb57d 100644 --- a/exporter/awsemfexporter/util.go +++ b/exporter/awsemfexporter/util.go @@ -182,7 +182,7 @@ func processAttributes(entityMap map[string]string, targetMap map[string]*string if strVal := val.Str(); strVal != "" { targetMap[shortName] = aws.String(strVal) } - //mutableResourceAttributes.Remove(entityField) + mutableResourceAttributes.Remove(entityField) } } } diff --git a/exporter/awsemfexporter/util_test.go b/exporter/awsemfexporter/util_test.go index 73e0ab9b6bae..f52542e594d7 100644 --- a/exporter/awsemfexporter/util_test.go +++ b/exporter/awsemfexporter/util_test.go @@ -374,6 +374,7 @@ func TestProcessAttributes(t *testing.T) { entityMap []map[string]string resourceAttributes map[string]any wantedAttributes map[string]*string + leftoverAttributes map[string]any }{ { name: "key_attributes", @@ -386,6 +387,7 @@ func TestProcessAttributes(t *testing.T) { serviceName: aws.String("my-service"), deploymentEnvironment: aws.String("my-environment"), }, + leftoverAttributes: make(map[string]any), }, { name: "non-key_attributes", @@ -402,6 +404,7 @@ func TestProcessAttributes(t *testing.T) { node: aws.String("my-node"), workload: aws.String("my-workload"), }, + leftoverAttributes: make(map[string]any), }, { name: "key_and_non_key_attributes", @@ -422,6 +425,7 @@ func TestProcessAttributes(t *testing.T) { node: aws.String("my-node"), workload: aws.String("my-workload"), }, + leftoverAttributes: make(map[string]any), }, { name: "key_and_non_key_attributes_plus_extras", @@ -443,6 +447,9 @@ func TestProcessAttributes(t *testing.T) { node: aws.String("my-node"), workload: aws.String("my-workload"), }, + leftoverAttributes: map[string]any{ + "extra_attribute": "extra_value", + }, }, } @@ -455,6 +462,7 @@ func TestProcessAttributes(t *testing.T) { for _, entityMap := range tc.entityMap { processAttributes(entityMap, targetMap, attrs) } + assert.Equal(t, tc.leftoverAttributes, attrs.AsRaw()) assert.Equal(t, tc.wantedAttributes, targetMap) }) } From a1cd2fc703a3ccce913262690a13aae149911f55 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Wed, 18 Sep 2024 12:42:47 -0400 Subject: [PATCH 44/45] lint check. Adds breadcrumb to update hash function --- exporter/awsemfexporter/emf_exporter.go | 5 +++-- exporter/awsemfexporter/util_test.go | 2 +- internal/aws/cwlogs/pusher.go | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/exporter/awsemfexporter/emf_exporter.go b/exporter/awsemfexporter/emf_exporter.go index b44df9dc36a5..f14ebfb3aeb7 100644 --- a/exporter/awsemfexporter/emf_exporter.go +++ b/exporter/awsemfexporter/emf_exporter.go @@ -7,6 +7,9 @@ import ( "context" "errors" "fmt" + "strings" + "sync" + "github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/google/uuid" @@ -16,8 +19,6 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pmetric" "go.uber.org/zap" - "strings" - "sync" "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsemfexporter/internal/appsignals" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/awsutil" diff --git a/exporter/awsemfexporter/util_test.go b/exporter/awsemfexporter/util_test.go index f52542e594d7..2e5164912a36 100644 --- a/exporter/awsemfexporter/util_test.go +++ b/exporter/awsemfexporter/util_test.go @@ -4,9 +4,9 @@ package awsemfexporter import ( - "github.com/aws/aws-sdk-go/aws" "testing" + "github.com/aws/aws-sdk-go/aws" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pmetric" diff --git a/internal/aws/cwlogs/pusher.go b/internal/aws/cwlogs/pusher.go index 64e18e8a587c..5009fb3b673d 100644 --- a/internal/aws/cwlogs/pusher.go +++ b/internal/aws/cwlogs/pusher.go @@ -55,7 +55,7 @@ func NewEvent(timestampMs int64, message string) *Event { return event } -// Uniquely identify a cloudwatch logs stream +// Uniquely identify a cloudwatch logs stream. Any changes to this struct will require updates to Hash type StreamKey struct { LogGroupName string LogStreamName string From 00e53b0ba9243bb69813c0c1b8a1bf852a4de3f6 Mon Sep 17 00:00:00 2001 From: Dinakar Chappa Date: Thu, 19 Sep 2024 02:49:57 -0400 Subject: [PATCH 45/45] removes bounded pusher map in favor of lru. Adds some unit tests and comments, small fixes. --- exporter/awsemfexporter/bounded_pusher_map.go | 76 ---------- .../awsemfexporter/bounded_pusher_map_test.go | 131 ------------------ exporter/awsemfexporter/emf_exporter.go | 19 ++- exporter/awsemfexporter/emf_exporter_test.go | 37 +++-- exporter/awsemfexporter/go.mod | 1 + exporter/awsemfexporter/go.sum | 2 + internal/aws/cwlogs/pusher_test.go | 30 ++++ internal/aws/cwlogs/utils.go | 6 +- 8 files changed, 74 insertions(+), 228 deletions(-) delete mode 100644 exporter/awsemfexporter/bounded_pusher_map.go delete mode 100644 exporter/awsemfexporter/bounded_pusher_map_test.go diff --git a/exporter/awsemfexporter/bounded_pusher_map.go b/exporter/awsemfexporter/bounded_pusher_map.go deleted file mode 100644 index dec5ac3a096a..000000000000 --- a/exporter/awsemfexporter/bounded_pusher_map.go +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package awsemfexporter - -import ( - "errors" - "time" - - "go.uber.org/zap" - - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" -) - -const ( - pusherMapLimit = 1000 -) - -type BoundedPusherMap struct { - pusherMap map[string]cwlogs.Pusher - limit int - stalePusherTracker map[string]time.Time -} - -func NewBoundedPusherMap() BoundedPusherMap { - return BoundedPusherMap{ - pusherMap: make(map[string]cwlogs.Pusher), - limit: pusherMapLimit, - stalePusherTracker: make(map[string]time.Time), - } -} - -func (bpm *BoundedPusherMap) Add(key string, pusher cwlogs.Pusher, logger *zap.Logger) { - err := bpm.EvictStalePushers() - if err != nil { - logger.Error("Error with evicting stale pushers", zap.Error(err)) - return - } - bpm.pusherMap[key] = pusher - bpm.stalePusherTracker[key] = time.Now() -} - -func (bpm *BoundedPusherMap) Get(key string) (cwlogs.Pusher, bool) { - pusher, ok := bpm.pusherMap[key] - if ok { - bpm.stalePusherTracker[key] = time.Now() - } - return pusher, ok -} - -func (bpm *BoundedPusherMap) EvictStalePushers() error { - if len(bpm.pusherMap) < bpm.limit { - return nil - } - now := time.Now() - for key, lastUsed := range bpm.stalePusherTracker { - if now.Sub(lastUsed) > time.Hour { - delete(bpm.pusherMap, key) - delete(bpm.stalePusherTracker, key) - } - } - // Ideally, we should now be below the pusher limit. If we aren't, especially after deleting pushers older than an hour, - // we should log an error. - if len(bpm.pusherMap) >= bpm.limit { - return errors.New("too many emf pushers being created. Dropping the request") - } - return nil -} - -func (bpm *BoundedPusherMap) ListAllPushers() []cwlogs.Pusher { - var pushers []cwlogs.Pusher - for _, pusher := range bpm.pusherMap { - pushers = append(pushers, pusher) - } - return pushers -} diff --git a/exporter/awsemfexporter/bounded_pusher_map_test.go b/exporter/awsemfexporter/bounded_pusher_map_test.go deleted file mode 100644 index 5f6c2dfa9564..000000000000 --- a/exporter/awsemfexporter/bounded_pusher_map_test.go +++ /dev/null @@ -1,131 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package awsemfexporter - -import ( - "testing" - "time" - - "github.com/stretchr/testify/assert" - "go.uber.org/zap" - - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs" -) - -// MockPusher implements the cwlogs.Pusher interface for testing -type MockPusher struct{} - -func (m MockPusher) AddLogEntry(_ *cwlogs.Event) error { - return nil -} - -func (m MockPusher) ForceFlush() error { - return nil -} - -func TestNewBoundedPusherMap(t *testing.T) { - bpm := NewBoundedPusherMap() - assert.Equal(t, pusherMapLimit, bpm.limit) - assert.Empty(t, bpm.pusherMap) - assert.Empty(t, bpm.stalePusherTracker) -} - -func TestBoundedPusherMap_Add(t *testing.T) { - bpm := NewBoundedPusherMap() - logger := zap.NewNop() - pusher := MockPusher{} - - bpm.Add("key1", pusher, logger) - assert.Len(t, bpm.pusherMap, 1) - assert.Len(t, bpm.stalePusherTracker, 1) - assert.Contains(t, bpm.pusherMap, "key1") - assert.Contains(t, bpm.stalePusherTracker, "key1") -} - -func TestBoundedPusherMap_Get(t *testing.T) { - bpm := NewBoundedPusherMap() - pusher := MockPusher{} - bpm.pusherMap["key1"] = pusher - bpm.stalePusherTracker["key1"] = time.Now().Add(-30 * time.Minute) - - // Test getting an existing pusher - gotPusher, ok := bpm.Get("key1") - assert.True(t, ok) - assert.Equal(t, pusher, gotPusher) - assert.True(t, bpm.stalePusherTracker["key1"].After(time.Now().Add(-1*time.Second))) - - // Test getting a non-existent pusher - gotPusher, ok = bpm.Get("key2") - assert.False(t, ok) - assert.Nil(t, gotPusher) -} - -func TestBoundedPusherMap_EvictStalePushers(t *testing.T) { - bpm := NewBoundedPusherMap() - bpm.limit = 2 - pusher := MockPusher{} - - // Add two pushers, one stale and one fresh - bpm.pusherMap["stale"] = pusher - bpm.stalePusherTracker["stale"] = time.Now().Add(-2 * time.Hour) - bpm.pusherMap["fresh"] = pusher - bpm.stalePusherTracker["fresh"] = time.Now() - - err := bpm.EvictStalePushers() - assert.NoError(t, err) - assert.Len(t, bpm.pusherMap, 1) - assert.Len(t, bpm.stalePusherTracker, 1) - assert.NotContains(t, bpm.pusherMap, "stale") - assert.NotContains(t, bpm.stalePusherTracker, "stale") - assert.Contains(t, bpm.pusherMap, "fresh") - assert.Contains(t, bpm.stalePusherTracker, "fresh") -} - -func TestBoundedPusherMap_EvictStalePushers_Error(t *testing.T) { - bpm := NewBoundedPusherMap() - bpm.limit = 2 - pusher := MockPusher{} - - // Add two fresh pushers - bpm.pusherMap["key1"] = pusher - bpm.pusherMap["key2"] = pusher - bpm.stalePusherTracker["key1"] = time.Now() - bpm.stalePusherTracker["key2"] = time.Now() - - err := bpm.EvictStalePushers() - assert.Error(t, err) - assert.Equal(t, "too many emf pushers being created. Dropping the request", err.Error()) -} - -func TestBoundedPusherMap_ListAllPushers(t *testing.T) { - bpm := NewBoundedPusherMap() - pusher1 := MockPusher{} - pusher2 := MockPusher{} - bpm.Add("key1", pusher1, zap.NewExample()) - bpm.Add("key2", pusher2, zap.NewExample()) - - pushers := bpm.ListAllPushers() - assert.Len(t, pushers, 2) - assert.Contains(t, pushers, pusher1) - assert.Contains(t, pushers, pusher2) -} - -func TestBoundedPusherMap_Add_EvictionError(t *testing.T) { - bpm := NewBoundedPusherMap() - bpm.limit = 1 - logger := zap.NewNop() - pusher := MockPusher{} - - // Add one pusher to reach the limit - bpm.Add("key1", pusher, logger) - - // Try to add another pusher, which should trigger eviction - bpm.Add("key2", pusher, logger) - - // Check that the second pusher was not added due to eviction error - assert.Len(t, bpm.pusherMap, 1) - assert.Len(t, bpm.stalePusherTracker, 1) - assert.Contains(t, bpm.pusherMap, "key1") - assert.NotContains(t, bpm.pusherMap, "key2") -} diff --git a/exporter/awsemfexporter/emf_exporter.go b/exporter/awsemfexporter/emf_exporter.go index f14ebfb3aeb7..94431d608bdb 100644 --- a/exporter/awsemfexporter/emf_exporter.go +++ b/exporter/awsemfexporter/emf_exporter.go @@ -13,6 +13,7 @@ import ( "github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/google/uuid" + lru "github.com/hashicorp/golang-lru/v2" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/consumer/consumererror" "go.opentelemetry.io/collector/exporter" @@ -33,10 +34,12 @@ const ( // AppSignals EMF config appSignalsMetricNamespace = "ApplicationSignals" appSignalsLogGroupNamePrefix = "/aws/application-signals/" + + pusherMapLimit = 1000 ) type emfExporter struct { - boundedPusherMap BoundedPusherMap + pusherMap *lru.Cache[string, cwlogs.Pusher] svcStructuredLog *cwlogs.Client config *Config @@ -80,6 +83,12 @@ func newEmfExporter(config *Config, set exporter.Settings) (*emfExporter, error) return nil, err } + boundedPusherMap, err := lru.New[string, cwlogs.Pusher](pusherMapLimit) + + if err != nil { + return nil, err + } + emfExporter := &emfExporter{ svcStructuredLog: svcStructuredLog, config: config, @@ -87,7 +96,7 @@ func newEmfExporter(config *Config, set exporter.Settings) (*emfExporter, error) retryCnt: *awsConfig.MaxRetries, collectorID: collectorIdentifier.String(), processResourceLabels: func(map[string]string) {}, - boundedPusherMap: NewBoundedPusherMap(), + pusherMap: boundedPusherMap, } if config.IsAppSignalsEnabled() { @@ -181,9 +190,9 @@ func (emf *emfExporter) getPusher(key cwlogs.StreamKey) cwlogs.Pusher { var ok bool hash := key.Hash() var pusher cwlogs.Pusher - if pusher, ok = emf.boundedPusherMap.Get(hash); !ok { + if pusher, ok = emf.pusherMap.Get(hash); !ok { pusher = cwlogs.NewPusher(key, emf.retryCnt, *emf.svcStructuredLog, emf.config.logger) - emf.boundedPusherMap.Add(hash, pusher, emf.config.logger) + emf.pusherMap.Add(hash, pusher) } return pusher @@ -193,7 +202,7 @@ func (emf *emfExporter) listPushers() []cwlogs.Pusher { emf.pusherMapLock.Lock() defer emf.pusherMapLock.Unlock() - return emf.boundedPusherMap.ListAllPushers() + return emf.pusherMap.Values() } func (emf *emfExporter) start(_ context.Context, host component.Host) error { diff --git a/exporter/awsemfexporter/emf_exporter_test.go b/exporter/awsemfexporter/emf_exporter_test.go index f6a66b4d67f5..5837ea5c4205 100644 --- a/exporter/awsemfexporter/emf_exporter_test.go +++ b/exporter/awsemfexporter/emf_exporter_test.go @@ -12,6 +12,7 @@ import ( "github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" + lru "github.com/hashicorp/golang-lru/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -197,10 +198,13 @@ func TestConsumeMetricsWithLogGroupStreamConfig(t *testing.T) { streamKey := &cwlogs.StreamKey{ LogGroupName: expCfg.LogGroupName, LogStreamName: expCfg.LogStreamName, - Entity: &cloudwatchlogs.Entity{}, + Entity: &cloudwatchlogs.Entity{ + KeyAttributes: map[string]*string{}, + Attributes: map[string]*string{}, + }, } expectedStreamKeyHash := streamKey.Hash() - pusherMap, ok := exp.boundedPusherMap.Get(expectedStreamKeyHash) + pusherMap, ok := exp.pusherMap.Get(expectedStreamKeyHash) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -236,16 +240,17 @@ func TestConsumeMetricsWithLogGroupStreamValidPlaceholder(t *testing.T) { LogGroupName: "/aws/ecs/containerinsights/test-cluster-name/performance", LogStreamName: "test-task-id", Entity: &cloudwatchlogs.Entity{ - Attributes: map[string]*string{ - "Cluster": aws.String("test-cluster-name"), - }, KeyAttributes: map[string]*string{ "Type": aws.String("Service"), "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), - }}, + }, + Attributes: map[string]*string{ + "Cluster": aws.String("test-cluster-name"), + }, + }, } - pusherMap, ok := exp.boundedPusherMap.Get(streamKey.Hash()) + pusherMap, ok := exp.pusherMap.Get(streamKey.Hash()) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -268,6 +273,7 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), }, + Attributes: map[string]*string{}, } md := generateTestMetrics(testMetric{ @@ -288,7 +294,7 @@ func TestConsumeMetricsWithOnlyLogStreamPlaceholder(t *testing.T) { LogStreamName: "test-task-id", Entity: entity, } - pusherMap, ok := exp.boundedPusherMap.Get(streamKey.Hash()) + pusherMap, ok := exp.pusherMap.Get(streamKey.Hash()) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -324,13 +330,14 @@ func TestConsumeMetricsWithWrongPlaceholder(t *testing.T) { LogStreamName: expCfg.LogStreamName, Entity: &cloudwatchlogs.Entity{ KeyAttributes: map[string]*string{ - "Type": aws.String("Service"), "Name": aws.String("myService"), "Environment": aws.String("myEnvironment"), + "Type": aws.String("Service"), }, + Attributes: map[string]*string{}, }, } - pusherMap, ok := exp.boundedPusherMap.Get(streamKey.Hash()) + pusherMap, ok := exp.pusherMap.Get(streamKey.Hash()) assert.True(t, ok) assert.NotNil(t, pusherMap) } @@ -354,13 +361,17 @@ func TestPushMetricsDataWithErr(t *testing.T) { logPusher.On("ForceFlush", nil).Return("some error").Once() logPusher.On("ForceFlush", nil).Return("").Once() logPusher.On("ForceFlush", nil).Return("some error").Once() - exp.boundedPusherMap = NewBoundedPusherMap() + exp.pusherMap, err = lru.New[string, cwlogs.Pusher](pusherMapLimit) + assert.Nil(t, err) streamKey := cwlogs.StreamKey{ LogGroupName: "test-logGroupName", LogStreamName: "test-logStreamName", - Entity: &cloudwatchlogs.Entity{}, + Entity: &cloudwatchlogs.Entity{ + Attributes: map[string]*string{}, + KeyAttributes: map[string]*string{}, + }, } - exp.boundedPusherMap.Add(streamKey.Hash(), logPusher, zap.NewExample()) + exp.pusherMap.Add(streamKey.Hash(), logPusher) md := generateTestMetrics(testMetric{ metricNames: []string{"metric_1", "metric_2"}, metricValues: [][]float64{{100}, {4}}, diff --git a/exporter/awsemfexporter/go.mod b/exporter/awsemfexporter/go.mod index 5b13b3931fbf..02d5377dd383 100644 --- a/exporter/awsemfexporter/go.mod +++ b/exporter/awsemfexporter/go.mod @@ -6,6 +6,7 @@ require ( github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware v0.0.0-20240419190856-2f880467f335 github.com/aws/aws-sdk-go v1.53.11 github.com/google/uuid v1.6.0 + github.com/hashicorp/golang-lru/v2 v2.0.7 github.com/jellydator/ttlcache/v3 v3.2.0 github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/awsutil v0.103.0 github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/cwlogs v0.103.0 diff --git a/exporter/awsemfexporter/go.sum b/exporter/awsemfexporter/go.sum index 245fdca1ee52..354bd4f8e403 100644 --- a/exporter/awsemfexporter/go.sum +++ b/exporter/awsemfexporter/go.sum @@ -47,6 +47,8 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/hashicorp/go-version v1.7.0 h1:5tqGy27NaOTB8yJKUZELlFAS/LTKJkrmONwQKeRZfjY= github.com/hashicorp/go-version v1.7.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= +github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= +github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/jellydator/ttlcache/v3 v3.2.0 h1:6lqVJ8X3ZaUwvzENqPAobDsXNExfUJd61u++uW8a3LE= github.com/jellydator/ttlcache/v3 v3.2.0/go.mod h1:hi7MGFdMAwZna5n2tuvh63DvFLzVKySzCVW6+0gA2n4= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= diff --git a/internal/aws/cwlogs/pusher_test.go b/internal/aws/cwlogs/pusher_test.go index 4aaab00dbef4..d11c68372d44 100644 --- a/internal/aws/cwlogs/pusher_test.go +++ b/internal/aws/cwlogs/pusher_test.go @@ -6,6 +6,7 @@ package cwlogs import ( "fmt" "math/rand" + "reflect" "strings" "testing" "time" @@ -280,3 +281,32 @@ func TestMultiStreamPusher(t *testing.T) { assert.Equal(t, "foo", *inputs[1].LogGroupName) assert.Equal(t, "bar2", *inputs[1].LogStreamName) } + +func TestStreamKeyFieldCount(t *testing.T) { + expectedFields := map[string]string{ + "LogGroupName": "string", + "LogStreamName": "string", + "Entity": "*cloudwatchlogs.Entity", + } + testStructFields(t, reflect.TypeOf(StreamKey{}), expectedFields, "StreamKey") +} + +func TestEntityFieldCount(t *testing.T) { + expectedFields := map[string]string{ + "_": "struct {}", + "KeyAttributes": "map[string]*string", + "Attributes": "map[string]*string", + } + testStructFields(t, reflect.TypeOf(cloudwatchlogs.Entity{}), expectedFields, "Entity") +} + +func testStructFields(t *testing.T, structType reflect.Type, expectedFields map[string]string, structName string) { + assert.Equal(t, len(expectedFields), structType.NumField(), "%s should have exactly %d fields", structName, len(expectedFields)) + + for i := 0; i < structType.NumField(); i++ { + field := structType.Field(i) + expectedType, exists := expectedFields[field.Name] + assert.True(t, exists, "Unexpected field in %s: %s", structName, field.Name) + assert.Equal(t, expectedType, field.Type.String(), "Incorrect type for field %s in %s", field.Name, structName) + } +} diff --git a/internal/aws/cwlogs/utils.go b/internal/aws/cwlogs/utils.go index 5b6cfb41a052..6aea976a6bd9 100644 --- a/internal/aws/cwlogs/utils.go +++ b/internal/aws/cwlogs/utils.go @@ -76,12 +76,12 @@ func mapToString(m map[string]*string) string { if m == nil { return "" } - var pairs []string + pairs := make([]string, 0, len(m)) for k, v := range m { if v == nil { - pairs = append(pairs, fmt.Sprintf("%s:", k)) + pairs = append(pairs, k+":") } else { - pairs = append(pairs, fmt.Sprintf("%s:%s", k, *v)) + pairs = append(pairs, k+":"+*v) } } sort.Strings(pairs) // Ensure a consistent order