From f63981a68274e181747c37cca16fcdb5f3c096bc Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Wed, 8 Mar 2023 21:17:55 +0000 Subject: [PATCH 01/12] Update metrics tests --- .../testcases/testcases_metrics.go | 5 +- ...sic_counter_metrics_compressed_expect.json | 693 ++++++++++++++++++ .../metrics/basic_counter_metrics_expect.json | 10 +- 3 files changed, 706 insertions(+), 2 deletions(-) create mode 100644 exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_compressed_expect.json diff --git a/exporter/collector/integrationtest/testcases/testcases_metrics.go b/exporter/collector/integrationtest/testcases/testcases_metrics.go index d4bab8212..f55501393 100644 --- a/exporter/collector/integrationtest/testcases/testcases_metrics.go +++ b/exporter/collector/integrationtest/testcases/testcases_metrics.go @@ -31,6 +31,9 @@ var MetricsTestCases = []TestCase{ Name: "Basic Counter", OTLPInputFixturePath: "testdata/fixtures/metrics/basic_counter_metrics.json", ExpectFixturePath: "testdata/fixtures/metrics/basic_counter_metrics_expect.json", + ConfigureCollector: func(cfg *collector.Config) { + cfg.MetricConfig.InstrumentationLibraryLabels = true + }, }, { Name: "Basic Counter with not found return code", @@ -221,7 +224,7 @@ var MetricsTestCases = []TestCase{ { Name: "Basic Counter with gzip compression", OTLPInputFixturePath: "testdata/fixtures/metrics/basic_counter_metrics.json", - ExpectFixturePath: "testdata/fixtures/metrics/basic_counter_metrics_expect.json", + ExpectFixturePath: "testdata/fixtures/metrics/basic_counter_metrics_compressed_expect.json", ConfigureCollector: func(cfg *collector.Config) { cfg.MetricConfig.ClientConfig.Compression = "gzip" }, diff --git a/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_compressed_expect.json b/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_compressed_expect.json new file mode 100644 index 000000000..b16561243 --- /dev/null +++ b/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_compressed_expect.json @@ -0,0 +1,693 @@ +{ + "createTimeSeriesRequests": [ + { + "name": "projects/fakeprojectid", + "timeSeries": [ + { + "metric": { + "type": "workload.googleapis.com/testcounter", + "labels": { + "foo": "bar" + } + }, + "resource": { + "type": "generic_node", + "labels": { + "location": "global", + "namespace": "", + "node_id": "" + } + }, + "metricKind": "CUMULATIVE", + "valueType": "INT64", + "points": [ + { + "interval": { + "endTime": "1970-01-01T00:00:00Z", + "startTime": "1970-01-01T00:00:00Z" + }, + "value": { + "int64Value": "253" + } + } + ], + "unit": "1" + } + ] + } + ], + "createMetricDescriptorRequests": [ + { + "name": "projects/fakeprojectid", + "metricDescriptor": { + "name": "testcounter", + "type": "workload.googleapis.com/testcounter", + "labels": [ + { + "key": "foo" + } + ], + "metricKind": "CUMULATIVE", + "valueType": "INT64", + "unit": "1", + "description": "This is a test counter", + "displayName": "testcounter" + } + } + ], + "selfObservabilityMetrics": { + "createTimeSeriesRequests": [ + { + "name": "projects/myproject", + "timeSeries": [ + { + "metric": { + "type": "custom.googleapis.com/opencensus/googlecloudmonitoring/point_count", + "labels": { + "status": "OK" + } + }, + "resource": { + "type": "global" + }, + "points": [ + { + "interval": { + "endTime": "1970-01-01T00:00:00Z", + "startTime": "1970-01-01T00:00:00Z" + }, + "value": { + "int64Value": "1" + } + } + ] + }, + { + "metric": { + "type": "custom.googleapis.com/opencensus/grpc.io/client/completed_rpcs", + "labels": { + "grpc_client_method": "google.monitoring.v3.MetricService/CreateMetricDescriptor", + "grpc_client_status": "OK" + } + }, + "resource": { + "type": "global" + }, + "points": [ + { + "interval": { + "endTime": "1970-01-01T00:00:00Z", + "startTime": "1970-01-01T00:00:00Z" + }, + "value": { + "int64Value": "1" + } + } + ] + }, + { + "metric": { + "type": "custom.googleapis.com/opencensus/grpc.io/client/completed_rpcs", + "labels": { + "grpc_client_method": "google.monitoring.v3.MetricService/CreateTimeSeries", + "grpc_client_status": "OK" + } + }, + "resource": { + "type": "global" + }, + "points": [ + { + "interval": { + "endTime": "1970-01-01T00:00:00Z", + "startTime": "1970-01-01T00:00:00Z" + }, + "value": { + "int64Value": "1" + } + } + ] + }, + { + "metric": { + "type": "custom.googleapis.com/opencensus/grpc.io/client/received_bytes_per_rpc", + "labels": { + "grpc_client_method": "google.monitoring.v3.MetricService/CreateMetricDescriptor" + } + }, + "resource": { + "type": "global" + }, + "points": [ + { + "interval": { + "endTime": "1970-01-01T00:00:00Z", + "startTime": "1970-01-01T00:00:00Z" + }, + "value": { + "distributionValue": { + "bucketOptions": { + "explicitBuckets": { + "bounds": [ + 0, + 1024, + 2048, + 4096, + 16384, + 65536, + 262144, + 1048576, + 4194304, + 16777216, + 67108864, + 268435456, + 1073741824, + 4294967296 + ] + } + }, + "bucketCounts": [ + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0" + ] + } + } + } + ] + }, + { + "metric": { + "type": "custom.googleapis.com/opencensus/grpc.io/client/received_bytes_per_rpc", + "labels": { + "grpc_client_method": "google.monitoring.v3.MetricService/CreateTimeSeries" + } + }, + "resource": { + "type": "global" + }, + "points": [ + { + "interval": { + "endTime": "1970-01-01T00:00:00Z", + "startTime": "1970-01-01T00:00:00Z" + }, + "value": { + "distributionValue": { + "bucketOptions": { + "explicitBuckets": { + "bounds": [ + 0, + 1024, + 2048, + 4096, + 16384, + 65536, + 262144, + 1048576, + 4194304, + 16777216, + 67108864, + 268435456, + 1073741824, + 4294967296 + ] + } + }, + "bucketCounts": [ + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0" + ] + } + } + } + ] + }, + { + "metric": { + "type": "custom.googleapis.com/opencensus/grpc.io/client/roundtrip_latency", + "labels": { + "grpc_client_method": "google.monitoring.v3.MetricService/CreateMetricDescriptor" + } + }, + "resource": { + "type": "global" + }, + "points": [ + { + "interval": { + "endTime": "1970-01-01T00:00:00Z", + "startTime": "1970-01-01T00:00:00Z" + }, + "value": { + "distributionValue": { + "bucketOptions": { + "explicitBuckets": { + "bounds": [ + 0, + 0.01, + 0.05, + 0.1, + 0.3, + 0.6, + 0.8, + 1, + 2, + 3, + 4, + 5, + 6, + 8, + 10, + 13, + 16, + 20, + 25, + 30, + 40, + 50, + 65, + 80, + 100, + 130, + 160, + 200, + 250, + 300, + 400, + 500, + 650, + 800, + 1000, + 2000, + 5000, + 10000, + 20000, + 50000, + 100000 + ] + } + }, + "bucketCounts": [ + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0" + ] + } + } + } + ] + }, + { + "metric": { + "type": "custom.googleapis.com/opencensus/grpc.io/client/roundtrip_latency", + "labels": { + "grpc_client_method": "google.monitoring.v3.MetricService/CreateTimeSeries" + } + }, + "resource": { + "type": "global" + }, + "points": [ + { + "interval": { + "endTime": "1970-01-01T00:00:00Z", + "startTime": "1970-01-01T00:00:00Z" + }, + "value": { + "distributionValue": { + "bucketOptions": { + "explicitBuckets": { + "bounds": [ + 0, + 0.01, + 0.05, + 0.1, + 0.3, + 0.6, + 0.8, + 1, + 2, + 3, + 4, + 5, + 6, + 8, + 10, + 13, + 16, + 20, + 25, + 30, + 40, + 50, + 65, + 80, + 100, + 130, + 160, + 200, + 250, + 300, + 400, + 500, + 650, + 800, + 1000, + 2000, + 5000, + 10000, + 20000, + 50000, + 100000 + ] + } + }, + "bucketCounts": [ + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0" + ] + } + } + } + ] + }, + { + "metric": { + "type": "custom.googleapis.com/opencensus/grpc.io/client/sent_bytes_per_rpc", + "labels": { + "grpc_client_method": "google.monitoring.v3.MetricService/CreateMetricDescriptor" + } + }, + "resource": { + "type": "global" + }, + "points": [ + { + "interval": { + "endTime": "1970-01-01T00:00:00Z", + "startTime": "1970-01-01T00:00:00Z" + }, + "value": { + "distributionValue": { + "bucketOptions": { + "explicitBuckets": { + "bounds": [ + 0, + 1024, + 2048, + 4096, + 16384, + 65536, + 262144, + 1048576, + 4194304, + 16777216, + 67108864, + 268435456, + 1073741824, + 4294967296 + ] + } + }, + "bucketCounts": [ + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0" + ] + } + } + } + ] + }, + { + "metric": { + "type": "custom.googleapis.com/opencensus/grpc.io/client/sent_bytes_per_rpc", + "labels": { + "grpc_client_method": "google.monitoring.v3.MetricService/CreateTimeSeries" + } + }, + "resource": { + "type": "global" + }, + "points": [ + { + "interval": { + "endTime": "1970-01-01T00:00:00Z", + "startTime": "1970-01-01T00:00:00Z" + }, + "value": { + "distributionValue": { + "bucketOptions": { + "explicitBuckets": { + "bounds": [ + 0, + 1024, + 2048, + 4096, + 16384, + 65536, + 262144, + 1048576, + 4194304, + 16777216, + 67108864, + 268435456, + 1073741824, + 4294967296 + ] + } + }, + "bucketCounts": [ + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0", + "0" + ] + } + } + } + ] + } + ] + } + ], + "createMetricDescriptorRequests": [ + { + "name": "projects/myproject", + "metricDescriptor": { + "name": "projects/myproject/metricDescriptors/custom.googleapis.com/opencensus/googlecloudmonitoring/point_count", + "type": "custom.googleapis.com/opencensus/googlecloudmonitoring/point_count", + "labels": [ + { + "key": "status" + } + ], + "metricKind": "CUMULATIVE", + "valueType": "INT64", + "unit": "1", + "description": "Count of metric points written to Cloud Monitoring.", + "displayName": "OpenCensus/googlecloudmonitoring/point_count" + } + }, + { + "name": "projects/myproject", + "metricDescriptor": { + "name": "projects/myproject/metricDescriptors/custom.googleapis.com/opencensus/grpc.io/client/completed_rpcs", + "type": "custom.googleapis.com/opencensus/grpc.io/client/completed_rpcs", + "labels": [ + { + "key": "grpc_client_method" + }, + { + "key": "grpc_client_status" + } + ], + "metricKind": "CUMULATIVE", + "valueType": "INT64", + "unit": "1", + "description": "Count of RPCs by method and status.", + "displayName": "OpenCensus/grpc.io/client/completed_rpcs" + } + }, + { + "name": "projects/myproject", + "metricDescriptor": { + "name": "projects/myproject/metricDescriptors/custom.googleapis.com/opencensus/grpc.io/client/received_bytes_per_rpc", + "type": "custom.googleapis.com/opencensus/grpc.io/client/received_bytes_per_rpc", + "labels": [ + { + "key": "grpc_client_method" + } + ], + "metricKind": "CUMULATIVE", + "valueType": "DISTRIBUTION", + "unit": "By", + "description": "Distribution of bytes received per RPC, by method.", + "displayName": "OpenCensus/grpc.io/client/received_bytes_per_rpc" + } + }, + { + "name": "projects/myproject", + "metricDescriptor": { + "name": "projects/myproject/metricDescriptors/custom.googleapis.com/opencensus/grpc.io/client/roundtrip_latency", + "type": "custom.googleapis.com/opencensus/grpc.io/client/roundtrip_latency", + "labels": [ + { + "key": "grpc_client_method" + } + ], + "metricKind": "CUMULATIVE", + "valueType": "DISTRIBUTION", + "unit": "ms", + "description": "Distribution of round-trip latency, by method.", + "displayName": "OpenCensus/grpc.io/client/roundtrip_latency" + } + }, + { + "name": "projects/myproject", + "metricDescriptor": { + "name": "projects/myproject/metricDescriptors/custom.googleapis.com/opencensus/grpc.io/client/sent_bytes_per_rpc", + "type": "custom.googleapis.com/opencensus/grpc.io/client/sent_bytes_per_rpc", + "labels": [ + { + "key": "grpc_client_method" + } + ], + "metricKind": "CUMULATIVE", + "valueType": "DISTRIBUTION", + "unit": "By", + "description": "Distribution of bytes sent per RPC, by method.", + "displayName": "OpenCensus/grpc.io/client/sent_bytes_per_rpc" + } + } + ] + } +} diff --git a/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_expect.json b/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_expect.json index b16561243..b220ee8b7 100644 --- a/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_expect.json +++ b/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_expect.json @@ -7,7 +7,9 @@ "metric": { "type": "workload.googleapis.com/testcounter", "labels": { - "foo": "bar" + "foo": "bar", + "instrumentation_source": "", + "instrumentation_version": "" } }, "resource": { @@ -45,6 +47,12 @@ "labels": [ { "key": "foo" + }, + { + "key": "instrumentation_source" + }, + { + "key": "instrumentation_version" } ], "metricKind": "CUMULATIVE", From b157f27d96a37072601401efc312a41d4fc8afe9 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Tue, 14 Mar 2023 15:27:35 +0000 Subject: [PATCH 02/12] Add check for duplicate fixture overwriting --- .../cmd/recordfixtures/main.go | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/exporter/collector/integrationtest/cmd/recordfixtures/main.go b/exporter/collector/integrationtest/cmd/recordfixtures/main.go index 3c517899f..c6c01d778 100644 --- a/exporter/collector/integrationtest/cmd/recordfixtures/main.go +++ b/exporter/collector/integrationtest/cmd/recordfixtures/main.go @@ -54,12 +54,23 @@ func main() { startTime := endTime.Add(-time.Second) t := &FakeTesting{} - recordLogs(ctx, t, endTime) - recordMetrics(ctx, t, startTime, endTime) - recordTraces(ctx, t, startTime, endTime) + // track the output fixtures to catch tests overwriting another test's fixtures + outputFixtures := make(map[string]string) + + recordLogs(ctx, t, endTime, outputFixtures) + recordMetrics(ctx, t, startTime, endTime, outputFixtures) + recordTraces(ctx, t, startTime, endTime, outputFixtures) } -func recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.Time) { +func checkDuplicate(test testcases.TestCase, outputFixtures map[string]string) error { + if duplicate, ok := outputFixtures[test.ExpectFixturePath]; ok { + return fmt.Errorf("test %s writes to the same fixture %s as test %s", test.Name, test.ExpectFixturePath, duplicate) + } + outputFixtures[test.ExpectFixturePath] = test.Name + return nil +} + +func recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.Time, outputFixtures map[string]string) { testServer, err := cloudmock.NewTracesTestServer() if err != nil { panic(err) @@ -73,6 +84,8 @@ func recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.T continue } + require.NoError(t, checkDuplicate(test, outputFixtures)) + func() { traces := test.LoadOTLPTracesInput(t, startTime, endTime) testServerExporter := integrationtest.NewTraceTestExporter(ctx, t, testServer, test.CreateTraceConfig()) @@ -89,7 +102,7 @@ func recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.T } } -func recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time) { +func recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time, outputFixtures map[string]string) { testServer, err := cloudmock.NewLoggingTestServer() if err != nil { panic(err) @@ -101,6 +114,9 @@ func recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time) { if test.Skip { continue } + + require.NoError(t, checkDuplicate(test, outputFixtures)) + func() { logs := test.LoadOTLPLogsInput(t, timestamp) testServerExporter := integrationtest.NewLogTestExporter(ctx, t, testServer, test.CreateLogConfig(), test.ConfigureLogsExporter) @@ -117,7 +133,7 @@ func recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time) { } } -func recordMetrics(ctx context.Context, t *FakeTesting, startTime, endTime time.Time) { +func recordMetrics(ctx context.Context, t *FakeTesting, startTime, endTime time.Time, outputFixtures map[string]string) { testServer, err := cloudmock.NewMetricTestServer() if err != nil { panic(err) @@ -130,6 +146,9 @@ func recordMetrics(ctx context.Context, t *FakeTesting, startTime, endTime time. if test.Skip { continue } + + require.NoError(t, checkDuplicate(test, outputFixtures)) + func() { metrics := test.LoadOTLPMetricsInput(t, startTime, endTime) testServerExporter := integrationtest.NewMetricTestExporter(ctx, t, testServer, test.CreateCollectorMetricConfig()) From 389c848c82af1642a90fb87b48c1fc7ae559c08b Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Tue, 14 Mar 2023 15:31:43 +0000 Subject: [PATCH 03/12] (metrics) Only add instrumentation labels if present --- .../metrics/basic_counter_metrics_expect.json | 10 +--------- exporter/collector/metrics.go | 14 ++++++++++---- exporter/collector/metrics_test.go | 2 +- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_expect.json b/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_expect.json index b220ee8b7..b16561243 100644 --- a/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_expect.json +++ b/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_counter_metrics_expect.json @@ -7,9 +7,7 @@ "metric": { "type": "workload.googleapis.com/testcounter", "labels": { - "foo": "bar", - "instrumentation_source": "", - "instrumentation_version": "" + "foo": "bar" } }, "resource": { @@ -47,12 +45,6 @@ "labels": [ { "key": "foo" - }, - { - "key": "instrumentation_source" - }, - { - "key": "instrumentation_version" } ], "metricKind": "CUMULATIVE", diff --git a/exporter/collector/metrics.go b/exporter/collector/metrics.go index ea2c39c1b..888ad3d9e 100644 --- a/exporter/collector/metrics.go +++ b/exporter/collector/metrics.go @@ -388,13 +388,19 @@ func (me *MetricsExporter) createServiceTimeSeries(ctx context.Context, req *mon } func (m *metricMapper) instrumentationScopeToLabels(is pcommon.InstrumentationScope) labels { + isLabels := make(labels) if !m.cfg.MetricConfig.InstrumentationLibraryLabels { - return labels{} + return isLabels } - return labels{ - "instrumentation_source": sanitizeUTF8(is.Name()), - "instrumentation_version": sanitizeUTF8(is.Version()), + instrumentationSource := sanitizeUTF8(is.Name()) + if len(instrumentationSource) > 0 { + isLabels["instrumentation_source"] = instrumentationSource } + instrumentationVersion := sanitizeUTF8(is.Version()) + if len(instrumentationVersion) > 0 { + isLabels["instrumentation_version"] = instrumentationVersion + } + return isLabels } func (m *metricMapper) metricToTimeSeries( diff --git a/exporter/collector/metrics_test.go b/exporter/collector/metrics_test.go index 1eec5e151..8a87041d2 100644 --- a/exporter/collector/metrics_test.go +++ b/exporter/collector/metrics_test.go @@ -1961,7 +1961,7 @@ func TestInstrumentationScopeToLabels(t *testing.T) { name: "notSetInInstrumentationScope", metricConfig: MetricConfig{InstrumentationLibraryLabels: true}, InstrumentationScope: pcommon.NewInstrumentationScope(), - output: labels{"instrumentation_source": "", "instrumentation_version": ""}, + output: labels{}, }} for _, test := range tests { From 79354cd129921f872603e2063e7407661d1ee2a1 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Tue, 14 Mar 2023 15:59:29 +0000 Subject: [PATCH 04/12] (logs) Only add service labels if set --- .../fixtures/logs/logs_apache_access.json | 12 ++ .../logs_apache_access_batches_expected.json | 125 ++++++++++++------ .../logs/logs_apache_access_expected.json | 49 ++++--- exporter/collector/monitoredresource.go | 4 +- 4 files changed, 134 insertions(+), 56 deletions(-) diff --git a/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access.json b/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access.json index b84d0f6d4..e80cbfeb4 100644 --- a/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access.json +++ b/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access.json @@ -8,6 +8,18 @@ "value": { "stringValue": "gcp_compute_engine" } + }, + { + "key": "service.name", + "value": { + "stringValue": "apache_service" + } + }, + { + "key": "service.namespace", + "value": { + "stringValue": "" + } } ] }, diff --git a/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_batches_expected.json b/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_batches_expected.json index f1ec73990..d5c1a8800 100644 --- a/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_batches_expected.json +++ b/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_batches_expected.json @@ -23,6 +23,7 @@ }, "labels": { "log.file.name": "test.log", + "service_name": "apache_service", "single-attribute": "foobar" } } @@ -51,7 +52,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } } ], @@ -79,9 +81,15 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } - }, + } + ], + "partialSuccess": true + }, + { + "entries": [ { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -102,7 +110,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } } ], @@ -130,9 +139,15 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } - }, + } + ], + "partialSuccess": true + }, + { + "entries": [ { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -153,7 +168,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } } ], @@ -181,9 +197,15 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } - }, + } + ], + "partialSuccess": true + }, + { + "entries": [ { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -204,7 +226,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } } ], @@ -232,9 +255,15 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } - }, + } + ], + "partialSuccess": true + }, + { + "entries": [ { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -255,7 +284,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } } ], @@ -283,9 +313,15 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } - }, + } + ], + "partialSuccess": true + }, + { + "entries": [ { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -306,7 +342,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } } ], @@ -334,9 +371,15 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } - }, + } + ], + "partialSuccess": true + }, + { + "entries": [ { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -357,7 +400,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } } ], @@ -385,9 +429,15 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } - }, + } + ], + "partialSuccess": true + }, + { + "entries": [ { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -397,25 +447,13 @@ "zone": "" } }, - "textPayload": "127.0.0.2 - - [26/Apr/2022:22:54:38 +0800] \"GET / HTTP/1.1\" 200 4429", + "textPayload": "127.0.0.1 - - [26/Apr/2022:22:54:43 +0800] \"-\" 408 -", "timestamp": "1970-01-01T00:00:00Z", - "httpRequest": { - "requestMethod": "GET", - "requestUrl": "/", - "status": 200, - "responseSize": "4429", - "remoteIp": "127.0.0.2", - "protocol": "HTTP/1.1" - }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } - } - ], - "partialSuccess": true - }, - { - "entries": [ + }, { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -425,10 +463,19 @@ "zone": "" } }, - "textPayload": "127.0.0.1 - - [26/Apr/2022:22:54:43 +0800] \"-\" 408 -", + "textPayload": "127.0.0.2 - - [26/Apr/2022:22:54:38 +0800] \"GET / HTTP/1.1\" 200 4429", "timestamp": "1970-01-01T00:00:00Z", + "httpRequest": { + "requestMethod": "GET", + "requestUrl": "/", + "status": 200, + "responseSize": "4429", + "remoteIp": "127.0.0.2", + "protocol": "HTTP/1.1" + }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } } ], diff --git a/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_expected.json b/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_expected.json index 6002299b0..1efd33951 100644 --- a/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_expected.json +++ b/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_expected.json @@ -23,6 +23,7 @@ }, "labels": { "log.file.name": "test.log", + "service_name": "apache_service", "single-attribute": "foobar" } }, @@ -46,7 +47,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -69,7 +71,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -92,7 +95,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -115,7 +119,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -138,7 +143,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -161,7 +167,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -184,7 +191,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -207,7 +215,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -230,7 +239,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -253,7 +263,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -276,7 +287,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -299,7 +311,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -322,7 +335,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -345,7 +359,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -360,7 +375,8 @@ "textPayload": "127.0.0.1 - - [26/Apr/2022:22:54:43 +0800] \"-\" 408 -", "timestamp": "1970-01-01T00:00:00Z", "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } }, { @@ -383,7 +399,8 @@ "protocol": "HTTP/1.1" }, "labels": { - "log.file.name": "test.log" + "log.file.name": "test.log", + "service_name": "apache_service" } } ], diff --git a/exporter/collector/monitoredresource.go b/exporter/collector/monitoredresource.go index 99206c013..0e98093d6 100644 --- a/exporter/collector/monitoredresource.go +++ b/exporter/collector/monitoredresource.go @@ -73,7 +73,9 @@ func resourceToLabels( (k == semconv.AttributeServiceName || k == semconv.AttributeServiceNamespace || k == semconv.AttributeServiceInstanceID) { - v.CopyTo(attrs.PutEmpty(k)) + if len(v.AsString()) > 0 { + v.CopyTo(attrs.PutEmpty(k)) + } return true } // Matches one of the resource filters From 44cb069d86dd8a665981ec6e6d7c306929fca8d9 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Tue, 14 Mar 2023 16:02:11 +0000 Subject: [PATCH 05/12] (metrics) Only add service labels if set --- .../testdata/fixtures/metrics/basic_prometheus_metrics.json | 6 ++++++ exporter/metric/option.go | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_prometheus_metrics.json b/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_prometheus_metrics.json index 5415ab78e..387fdaacd 100644 --- a/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_prometheus_metrics.json +++ b/exporter/collector/integrationtest/testdata/fixtures/metrics/basic_prometheus_metrics.json @@ -15,6 +15,12 @@ "stringValue":"localhost:2222" } }, + { + "key":"service.namespace", + "value":{ + "stringValue":"" + } + }, { "key":"net.host.port", "value":{ diff --git a/exporter/metric/option.go b/exporter/metric/option.go index fab442f2d..9726f4a4d 100644 --- a/exporter/metric/option.go +++ b/exporter/metric/option.go @@ -122,9 +122,9 @@ func WithFilteredResourceAttributes(filter attribute.Filter) func(o *options) { // DefaultResourceAttributesFilter is the default filter applied to resource // attributes. func DefaultResourceAttributesFilter(kv attribute.KeyValue) bool { - return kv.Key == semconv.ServiceNameKey || + return (kv.Key == semconv.ServiceNameKey || kv.Key == semconv.ServiceNamespaceKey || - kv.Key == semconv.ServiceInstanceIDKey + kv.Key == semconv.ServiceInstanceIDKey) && len(kv.Value.AsString()) > 0 } // NoAttributes can be passed to WithFilteredResourceAttributes to disable From 6f8418ddd978c3a73d0c7792ccb84fb16ee3d262 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Tue, 14 Mar 2023 16:14:24 +0000 Subject: [PATCH 06/12] (traces) only add service labels if set --- .../testdata/fixtures/traces/traces_basic.json | 6 ++++++ exporter/collector/spandata.go | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic.json b/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic.json index b4795ec38..84762c061 100644 --- a/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic.json +++ b/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic.json @@ -69,6 +69,12 @@ "stringValue": "demo-server" } }, + { + "key": "service.namespace", + "value": { + "stringValue": "" + } + }, { "key": "telemetry.sdk.language", "value": { diff --git a/exporter/collector/spandata.go b/exporter/collector/spandata.go index bca01c7d8..7cf134cc3 100644 --- a/exporter/collector/spandata.go +++ b/exporter/collector/spandata.go @@ -20,6 +20,7 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" + semconv "go.opentelemetry.io/collector/semconv/v1.18.0" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/sdk/instrumentation" @@ -125,6 +126,13 @@ func pdataAttributesToOTAttributes(attrs pcommon.Map, resource pcommon.Resource) otAttrs := make([]attribute.KeyValue, 0, attrs.Len()) appendAttrs := func(m pcommon.Map) { m.Range(func(k string, v pcommon.Value) bool { + if k == semconv.AttributeServiceName || + k == semconv.AttributeServiceNamespace || + k == semconv.AttributeServiceInstanceID { + if len(v.AsString()) == 0 { + return true + } + } switch v.Type() { case pcommon.ValueTypeStr: otAttrs = append(otAttrs, attribute.String(k, v.Str())) From 78352923c27872bf72860fb5b105680665a09645 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Tue, 14 Mar 2023 16:17:40 +0000 Subject: [PATCH 07/12] (traces) only add instrumentation labels if set --- .../fixtures/traces/traces_basic.json | 2 +- .../traces/traces_basic_expected.json | 20 ------------------- exporter/collector/spandata.go | 16 +++++++++++---- 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic.json b/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic.json index 84762c061..01d86b423 100644 --- a/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic.json +++ b/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic.json @@ -656,7 +656,7 @@ { "scope": { "name": "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp", - "version": "semver:0.33.0" + "version": "" }, "spans": [ { diff --git a/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic_expected.json b/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic_expected.json index 5941d50ac..800835644 100644 --- a/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic_expected.json +++ b/exporter/collector/integrationtest/testdata/fixtures/traces/traces_basic_expected.json @@ -791,11 +791,6 @@ "value": "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" } }, - "otel.scope.version": { - "stringValue": { - "value": "semver:0.33.0" - } - }, "process.executable.name": { "stringValue": { "value": "main" @@ -921,11 +916,6 @@ "value": "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" } }, - "otel.scope.version": { - "stringValue": { - "value": "semver:0.33.0" - } - }, "process.executable.name": { "stringValue": { "value": "main" @@ -1051,11 +1041,6 @@ "value": "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" } }, - "otel.scope.version": { - "stringValue": { - "value": "semver:0.33.0" - } - }, "process.executable.name": { "stringValue": { "value": "main" @@ -1181,11 +1166,6 @@ "value": "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" } }, - "otel.scope.version": { - "stringValue": { - "value": "semver:0.33.0" - } - }, "process.executable.name": { "stringValue": { "value": "main" diff --git a/exporter/collector/spandata.go b/exporter/collector/spandata.go index 7cf134cc3..4e3d531c2 100644 --- a/exporter/collector/spandata.go +++ b/exporter/collector/spandata.go @@ -81,10 +81,7 @@ func pdataSpanToOTSpanData( droppedMessageEvents: int(span.DroppedEventsCount()), droppedLinks: int(span.DroppedLinksCount()), resource: r, - instrumentationLibrary: instrumentation.Scope{ - Name: is.Name(), - Version: is.Version(), - }, + instrumentationLibrary: instrumentationScopeLabels(is), status: sdktrace.Status{ Code: pdataStatusCodeToOTCode(status.Code()), Description: status.Message(), @@ -92,6 +89,17 @@ func pdataSpanToOTSpanData( } } +func instrumentationScopeLabels(is pcommon.InstrumentationScope) instrumentation.Scope { + scope := instrumentation.Scope{} + if len(is.Name()) > 0 { + scope.Name = is.Name() + } + if len(is.Version()) > 0 { + scope.Version = is.Version() + } + return scope +} + func pdataSpanKindToOTSpanKind(k ptrace.SpanKind) apitrace.SpanKind { switch k { case ptrace.SpanKindUnspecified: From 0e49607f590bc916d60c2a6581d00992d055b28b Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Tue, 14 Mar 2023 16:42:43 +0000 Subject: [PATCH 08/12] make lint --- exporter/collector/spandata.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/exporter/collector/spandata.go b/exporter/collector/spandata.go index 4e3d531c2..78ff5e092 100644 --- a/exporter/collector/spandata.go +++ b/exporter/collector/spandata.go @@ -68,19 +68,19 @@ func pdataSpanToOTSpanData( status := span.Status() return spanSnapshot{ - spanContext: apitrace.NewSpanContext(sc), - parent: apitrace.NewSpanContext(parentSc), - spanKind: pdataSpanKindToOTSpanKind(span.Kind()), - startTime: startTime, - endTime: endTime, - name: span.Name(), - attributes: pdataAttributesToOTAttributes(span.Attributes(), resource), - links: pdataLinksToOTLinks(span.Links()), - events: pdataEventsToOTMessageEvents(span.Events()), - droppedAttributes: int(span.DroppedAttributesCount()), - droppedMessageEvents: int(span.DroppedEventsCount()), - droppedLinks: int(span.DroppedLinksCount()), - resource: r, + spanContext: apitrace.NewSpanContext(sc), + parent: apitrace.NewSpanContext(parentSc), + spanKind: pdataSpanKindToOTSpanKind(span.Kind()), + startTime: startTime, + endTime: endTime, + name: span.Name(), + attributes: pdataAttributesToOTAttributes(span.Attributes(), resource), + links: pdataLinksToOTLinks(span.Links()), + events: pdataEventsToOTMessageEvents(span.Events()), + droppedAttributes: int(span.DroppedAttributesCount()), + droppedMessageEvents: int(span.DroppedEventsCount()), + droppedLinks: int(span.DroppedLinksCount()), + resource: r, instrumentationLibrary: instrumentationScopeLabels(is), status: sdktrace.Status{ Code: pdataStatusCodeToOTCode(status.Code()), From 2664f6b0bf0e28ffe26730c0ef1d348c3cd49914 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Tue, 14 Mar 2023 19:21:03 +0000 Subject: [PATCH 09/12] object-ify fixture recorder --- .../cmd/recordfixtures/main.go | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/exporter/collector/integrationtest/cmd/recordfixtures/main.go b/exporter/collector/integrationtest/cmd/recordfixtures/main.go index c6c01d778..09083a78f 100644 --- a/exporter/collector/integrationtest/cmd/recordfixtures/main.go +++ b/exporter/collector/integrationtest/cmd/recordfixtures/main.go @@ -48,6 +48,16 @@ func (t *FakeTesting) FailNow() { func (t *FakeTesting) Helper() {} func (t *FakeTesting) Name() string { return "record fixtures" } +type fixtureRecorder map[string]string + +func (fr fixtureRecorder) checkDuplicate(test testcases.TestCase) error { + if duplicate, ok := fr[test.ExpectFixturePath]; ok { + return fmt.Errorf("test %s writes to the same fixture %s as test %s", test.Name, test.ExpectFixturePath, duplicate) + } + fr[test.ExpectFixturePath] = test.Name + return nil +} + func main() { ctx := context.Background() endTime := time.Now() @@ -55,22 +65,14 @@ func main() { t := &FakeTesting{} // track the output fixtures to catch tests overwriting another test's fixtures - outputFixtures := make(map[string]string) + ft := make(fixtureRecorder) - recordLogs(ctx, t, endTime, outputFixtures) - recordMetrics(ctx, t, startTime, endTime, outputFixtures) - recordTraces(ctx, t, startTime, endTime, outputFixtures) -} - -func checkDuplicate(test testcases.TestCase, outputFixtures map[string]string) error { - if duplicate, ok := outputFixtures[test.ExpectFixturePath]; ok { - return fmt.Errorf("test %s writes to the same fixture %s as test %s", test.Name, test.ExpectFixturePath, duplicate) - } - outputFixtures[test.ExpectFixturePath] = test.Name - return nil + ft.recordLogs(ctx, t, endTime) + ft.recordMetrics(ctx, t, startTime, endTime) + ft.recordTraces(ctx, t, startTime, endTime) } -func recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.Time, outputFixtures map[string]string) { +func (fr fixtureRecorder) recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.Time) { testServer, err := cloudmock.NewTracesTestServer() if err != nil { panic(err) @@ -84,7 +86,7 @@ func recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.T continue } - require.NoError(t, checkDuplicate(test, outputFixtures)) + require.NoError(t, fr.checkDuplicate(test)) func() { traces := test.LoadOTLPTracesInput(t, startTime, endTime) @@ -102,7 +104,7 @@ func recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.T } } -func recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time, outputFixtures map[string]string) { +func (fr fixtureRecorder) recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time) { testServer, err := cloudmock.NewLoggingTestServer() if err != nil { panic(err) @@ -115,7 +117,7 @@ func recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time, output continue } - require.NoError(t, checkDuplicate(test, outputFixtures)) + require.NoError(t, fr.checkDuplicate(test)) func() { logs := test.LoadOTLPLogsInput(t, timestamp) @@ -133,7 +135,7 @@ func recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time, output } } -func recordMetrics(ctx context.Context, t *FakeTesting, startTime, endTime time.Time, outputFixtures map[string]string) { +func (fr fixtureRecorder) recordMetrics(ctx context.Context, t *FakeTesting, startTime, endTime time.Time) { testServer, err := cloudmock.NewMetricTestServer() if err != nil { panic(err) @@ -147,7 +149,7 @@ func recordMetrics(ctx context.Context, t *FakeTesting, startTime, endTime time. continue } - require.NoError(t, checkDuplicate(test, outputFixtures)) + require.NoError(t, fr.checkDuplicate(test)) func() { metrics := test.LoadOTLPMetricsInput(t, startTime, endTime) From f6515c3ffc5d6fce3d9b2259b51b31acb51c7468 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Tue, 14 Mar 2023 19:56:12 +0000 Subject: [PATCH 10/12] feedback --- exporter/collector/spandata.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/exporter/collector/spandata.go b/exporter/collector/spandata.go index 78ff5e092..26622c7af 100644 --- a/exporter/collector/spandata.go +++ b/exporter/collector/spandata.go @@ -134,12 +134,11 @@ func pdataAttributesToOTAttributes(attrs pcommon.Map, resource pcommon.Resource) otAttrs := make([]attribute.KeyValue, 0, attrs.Len()) appendAttrs := func(m pcommon.Map) { m.Range(func(k string, v pcommon.Value) bool { - if k == semconv.AttributeServiceName || + if (k == semconv.AttributeServiceName || k == semconv.AttributeServiceNamespace || - k == semconv.AttributeServiceInstanceID { - if len(v.AsString()) == 0 { - return true - } + k == semconv.AttributeServiceInstanceID) && + len(v.AsString()) == 0 { + return true } switch v.Type() { case pcommon.ValueTypeStr: From 3bb133f13c6df313788c88948cb3de7f2db8106e Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Wed, 15 Mar 2023 13:39:31 +0000 Subject: [PATCH 11/12] (logs) Update batch size in fixture test to account for new label --- .../testcases/testcases_logs.go | 2 +- .../logs_apache_access_batches_expected.json | 42 +++---------------- 2 files changed, 7 insertions(+), 37 deletions(-) diff --git a/exporter/collector/integrationtest/testcases/testcases_logs.go b/exporter/collector/integrationtest/testcases/testcases_logs.go index 9ebbcd6f1..9e9c1940c 100644 --- a/exporter/collector/integrationtest/testcases/testcases_logs.go +++ b/exporter/collector/integrationtest/testcases/testcases_logs.go @@ -69,7 +69,7 @@ var LogsTestCases = []TestCase{ ExpectFixturePath: "testdata/fixtures/logs/logs_apache_access_batches_expected.json", ConfigureLogsExporter: &logsutil.ExporterConfig{ MaxEntrySize: 50, - MaxRequestSize: 500, + MaxRequestSize: 550, }, }, } diff --git a/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_batches_expected.json b/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_batches_expected.json index d5c1a8800..b432a6d72 100644 --- a/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_batches_expected.json +++ b/exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access_batches_expected.json @@ -113,12 +113,7 @@ "log.file.name": "test.log", "service_name": "apache_service" } - } - ], - "partialSuccess": true - }, - { - "entries": [ + }, { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -171,12 +166,7 @@ "log.file.name": "test.log", "service_name": "apache_service" } - } - ], - "partialSuccess": true - }, - { - "entries": [ + }, { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -229,12 +219,7 @@ "log.file.name": "test.log", "service_name": "apache_service" } - } - ], - "partialSuccess": true - }, - { - "entries": [ + }, { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -287,12 +272,7 @@ "log.file.name": "test.log", "service_name": "apache_service" } - } - ], - "partialSuccess": true - }, - { - "entries": [ + }, { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -345,12 +325,7 @@ "log.file.name": "test.log", "service_name": "apache_service" } - } - ], - "partialSuccess": true - }, - { - "entries": [ + }, { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { @@ -403,12 +378,7 @@ "log.file.name": "test.log", "service_name": "apache_service" } - } - ], - "partialSuccess": true - }, - { - "entries": [ + }, { "logName": "projects/fakeprojectid/logs/my-log-name-foo", "resource": { From b1f1526c54a1fa6f16c3e72a87a63a73909ce9e4 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Fri, 17 Mar 2023 19:08:14 +0000 Subject: [PATCH 12/12] feedback --- .../collector/integrationtest/cmd/recordfixtures/main.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/exporter/collector/integrationtest/cmd/recordfixtures/main.go b/exporter/collector/integrationtest/cmd/recordfixtures/main.go index 09083a78f..d6c8c7cc2 100644 --- a/exporter/collector/integrationtest/cmd/recordfixtures/main.go +++ b/exporter/collector/integrationtest/cmd/recordfixtures/main.go @@ -65,11 +65,11 @@ func main() { t := &FakeTesting{} // track the output fixtures to catch tests overwriting another test's fixtures - ft := make(fixtureRecorder) + fr := make(fixtureRecorder) - ft.recordLogs(ctx, t, endTime) - ft.recordMetrics(ctx, t, startTime, endTime) - ft.recordTraces(ctx, t, startTime, endTime) + fr.recordLogs(ctx, t, endTime) + fr.recordMetrics(ctx, t, startTime, endTime) + fr.recordTraces(ctx, t, startTime, endTime) } func (fr fixtureRecorder) recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.Time) {