Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only attach instrumentation scope/service labels when non-empty #613

Merged
merged 12 commits into from
Mar 20, 2023
Merged

Only attach instrumentation scope/service labels when non-empty #613

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f63981a
Update metrics tests
damemi Mar 8, 2023
b157f27
Add check for duplicate fixture overwriting
damemi Mar 14, 2023
389c848
(metrics) Only add instrumentation labels if present
damemi Mar 14, 2023
79354cd
(logs) Only add service labels if set
damemi Mar 14, 2023
44cb069
(metrics) Only add service labels if set
damemi Mar 14, 2023
6f8418d
(traces) only add service labels if set
damemi Mar 14, 2023
7835292
(traces) only add instrumentation labels if set
damemi Mar 14, 2023
0e49607
make lint
damemi Mar 14, 2023
2664f6b
object-ify fixture recorder
damemi Mar 14, 2023
f6515c3
feedback
damemi Mar 14, 2023
3bb133f
(logs) Update batch size in fixture test to account for new label
damemi Mar 15, 2023
b1f1526
feedback
damemi Mar 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions exporter/collector/integrationtest/cmd/recordfixtures/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,31 @@ 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()
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
damemi marked this conversation as resolved.
Show resolved Hide resolved
ft := make(fixtureRecorder)
damemi marked this conversation as resolved.
Show resolved Hide resolved

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) {
func (fr fixtureRecorder) recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.Time) {
testServer, err := cloudmock.NewTracesTestServer()
if err != nil {
panic(err)
Expand All @@ -73,6 +86,8 @@ func recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.T
continue
}

require.NoError(t, fr.checkDuplicate(test))

func() {
traces := test.LoadOTLPTracesInput(t, startTime, endTime)
testServerExporter := integrationtest.NewTraceTestExporter(ctx, t, testServer, test.CreateTraceConfig())
Expand All @@ -89,7 +104,7 @@ func recordTraces(ctx context.Context, t *FakeTesting, startTime, endTime time.T
}
}

func recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time) {
func (fr fixtureRecorder) recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time) {
testServer, err := cloudmock.NewLoggingTestServer()
if err != nil {
panic(err)
Expand All @@ -101,6 +116,9 @@ func recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time) {
if test.Skip {
continue
}

require.NoError(t, fr.checkDuplicate(test))

func() {
logs := test.LoadOTLPLogsInput(t, timestamp)
testServerExporter := integrationtest.NewLogTestExporter(ctx, t, testServer, test.CreateLogConfig(), test.ConfigureLogsExporter)
Expand All @@ -117,7 +135,7 @@ func recordLogs(ctx context.Context, t *FakeTesting, timestamp time.Time) {
}
}

func recordMetrics(ctx context.Context, t *FakeTesting, startTime, endTime time.Time) {
func (fr fixtureRecorder) recordMetrics(ctx context.Context, t *FakeTesting, startTime, endTime time.Time) {
testServer, err := cloudmock.NewMetricTestServer()
if err != nil {
panic(err)
Expand All @@ -130,6 +148,9 @@ func recordMetrics(ctx context.Context, t *FakeTesting, startTime, endTime time.
if test.Skip {
continue
}

require.NoError(t, fr.checkDuplicate(test))

func() {
metrics := test.LoadOTLPMetricsInput(t, startTime, endTime)
testServerExporter := integrationtest.NewMetricTestExporter(ctx, t, testServer, test.CreateCollectorMetricConfig())
Expand Down
2 changes: 1 addition & 1 deletion exporter/collector/integrationtest/testcases/testcases_logs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
}
5 changes: 4 additions & 1 deletion exporter/collector/integrationtest/testcases/testcases_metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
aabmass marked this conversation as resolved.
Show resolved Hide resolved
ConfigureCollector: func(cfg *collector.Config) {
cfg.MetricConfig.ClientConfig.Compression = "gzip"
},
Expand Down
12 changes: 12 additions & 0 deletions exporter/collector/integrationtest/testdata/fixtures/logs/logs_apache_access.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@
"value": {
"stringValue": "gcp_compute_engine"
}
},
{
"key": "service.name",
"value": {
"stringValue": "apache_service"
}
},
{
"key": "service.namespace",
"value": {
"stringValue": ""
}
}
]
},
Expand Down
105 changes: 61 additions & 44 deletions ...collector/integrationtest/testdata/fixtures/logs/logs_apache_access_batches_expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
},
"labels": {
"log.file.name": "test.log",
"service_name": "apache_service",
"single-attribute": "foobar"
}
}
Expand Down Expand Up @@ -51,7 +52,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
}
],
Expand Down Expand Up @@ -79,30 +81,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
}
},
{
"logName": "projects/fakeprojectid/logs/my-log-name-foo",
"resource": {
"type": "gce_instance",
"labels": {
"instance_id": "",
"zone": ""
}
},
"textPayload": "127.0.0.1 - - [26/Apr/2022:22:53:51 +0800] \"GET / HTTP/1.1\" 200 1247",
"timestamp": "1970-01-01T00:00:00Z",
"httpRequest": {
"requestMethod": "GET",
"requestUrl": "/",
"status": 200,
"responseSize": "1247",
"remoteIp": "127.0.0.1",
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
}
],
Expand All @@ -119,7 +99,7 @@
"zone": ""
}
},
"textPayload": "127.0.0.1 - - [26/Apr/2022:22:53:52 +0800] \"GET / HTTP/1.1\" 200 1247",
"textPayload": "127.0.0.1 - - [26/Apr/2022:22:53:51 +0800] \"GET / HTTP/1.1\" 200 1247",
"timestamp": "1970-01-01T00:00:00Z",
"httpRequest": {
"requestMethod": "GET",
Expand All @@ -130,7 +110,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
},
{
Expand All @@ -142,7 +123,7 @@
"zone": ""
}
},
"textPayload": "127.0.0.1 - - [26/Apr/2022:22:53:53 +0800] \"GET / HTTP/1.1\" 200 1247",
"textPayload": "127.0.0.1 - - [26/Apr/2022:22:53:52 +0800] \"GET / HTTP/1.1\" 200 1247",
"timestamp": "1970-01-01T00:00:00Z",
"httpRequest": {
"requestMethod": "GET",
Expand All @@ -153,7 +134,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
}
],
Expand Down Expand Up @@ -181,7 +163,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
},
{
Expand All @@ -204,7 +187,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
}
],
Expand Down Expand Up @@ -232,7 +216,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
},
{
Expand All @@ -244,7 +229,7 @@
"zone": ""
}
},
"textPayload": "127.0.0.1 - - [26/Apr/2022:22:53:54 +0800] \"GET / HTTP/1.1\" 200 1247",
"textPayload": "127.0.0.1 - - [26/Apr/2022:22:53:53 +0800] \"GET / HTTP/1.1\" 200 1247",
"timestamp": "1970-01-01T00:00:00Z",
"httpRequest": {
"requestMethod": "GET",
Expand All @@ -255,7 +240,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
}
],
Expand Down Expand Up @@ -283,7 +269,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
},
{
Expand All @@ -306,7 +293,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
}
],
Expand Down Expand Up @@ -334,7 +322,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
},
{
Expand All @@ -357,7 +346,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
}
],
Expand Down Expand Up @@ -385,7 +375,8 @@
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
},
{
Expand All @@ -397,18 +388,19 @@
"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:53:54 +0800] \"GET / HTTP/1.1\" 200 1247",
"timestamp": "1970-01-01T00:00:00Z",
"httpRequest": {
"requestMethod": "GET",
"requestUrl": "/",
"status": 200,
"responseSize": "4429",
"remoteIp": "127.0.0.2",
"responseSize": "1247",
"remoteIp": "127.0.0.1",
"protocol": "HTTP/1.1"
},
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
}
],
Expand All @@ -428,7 +420,32 @@
"textPayload": "127.0.0.1 - - [26/Apr/2022:22:54:43 +0800] \"-\" 408 -",
aabmass marked this conversation as resolved.
Show resolved Hide resolved
"timestamp": "1970-01-01T00:00:00Z",
"labels": {
"log.file.name": "test.log"
"log.file.name": "test.log",
"service_name": "apache_service"
}
},
{
"logName": "projects/fakeprojectid/logs/my-log-name-foo",
"resource": {
"type": "gce_instance",
"labels": {
"instance_id": "",
"zone": ""
}
},
"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",
"service_name": "apache_service"
}
}
],
Expand Down
Loading