From 6f701306251fd7df0d5e9ca671e16a29e803d6a1 Mon Sep 17 00:00:00 2001 From: Robert Pajak Date: Mon, 13 Jan 2025 11:38:05 +0100 Subject: [PATCH 1/6] config: Fix handling of SchemaUrl --- config/v0.3.0/resource.go | 13 +++++--- config/v0.3.0/resource_test.go | 61 +++++++++++++++++++++------------- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/config/v0.3.0/resource.go b/config/v0.3.0/resource.go index 4983374aa6a..576945b2bf9 100644 --- a/config/v0.3.0/resource.go +++ b/config/v0.3.0/resource.go @@ -47,16 +47,19 @@ func keyVal(k string, v any) attribute.KeyValue { } func newResource(res *Resource) *resource.Resource { - if res == nil || res.Attributes == nil { + if res == nil || res.SchemaUrl == nil && res.Attributes == nil { return resource.Default() } - var attrs []attribute.KeyValue + schemaURL := "" + if res.SchemaUrl != nil { + schemaURL = *res.SchemaUrl + } + + var attrs []attribute.KeyValue for _, v := range res.Attributes { attrs = append(attrs, keyVal(v.Name, v.Value)) } - return resource.NewWithAttributes(*res.SchemaUrl, - attrs..., - ) + return resource.NewWithAttributes(schemaURL, attrs...) } diff --git a/config/v0.3.0/resource_test.go b/config/v0.3.0/resource_test.go index 3a80e7c280a..9dae5b8bed5 100644 --- a/config/v0.3.0/resource_test.go +++ b/config/v0.3.0/resource_test.go @@ -17,28 +17,7 @@ import ( type mockType struct{} func TestNewResource(t *testing.T) { - res := resource.NewWithAttributes(semconv.SchemaURL, - semconv.ServiceName("service-a"), - ) other := mockType{} - resWithAttrs := resource.NewWithAttributes(semconv.SchemaURL, - semconv.ServiceName("service-a"), - attribute.Bool("attr-bool", true), - attribute.String("attr-uint64", fmt.Sprintf("%d", 164)), - attribute.Int64("attr-int64", int64(-164)), - attribute.Float64("attr-float64", float64(64.0)), - attribute.Int64("attr-int8", int64(-18)), - attribute.Int64("attr-uint8", int64(18)), - attribute.Int64("attr-int16", int64(-116)), - attribute.Int64("attr-uint16", int64(116)), - attribute.Int64("attr-int32", int64(-132)), - attribute.Int64("attr-uint32", int64(132)), - attribute.Float64("attr-float32", float64(32.0)), - attribute.Int64("attr-int", int64(-1)), - attribute.String("attr-uint", fmt.Sprintf("%d", 1)), - attribute.String("attr-string", "string-val"), - attribute.String("attr-default", fmt.Sprintf("%v", other)), - ) tests := []struct { name string config *Resource @@ -53,6 +32,24 @@ func TestNewResource(t *testing.T) { config: &Resource{}, wantResource: resource.Default(), }, + { + name: "resource-with-schema", + config: &Resource{ + SchemaUrl: ptr(semconv.SchemaURL), + }, + wantResource: resource.NewWithAttributes(semconv.SchemaURL), + }, + { + name: "resource-with-attributes", + config: &Resource{ + Attributes: []AttributeNameValue{ + {Name: "service.name", Value: "service-a"}, + }, + }, + wantResource: resource.NewWithAttributes("", + semconv.ServiceName("service-a"), + ), + }, { name: "resource-with-attributes-and-schema", config: &Resource{ @@ -61,7 +58,9 @@ func TestNewResource(t *testing.T) { }, SchemaUrl: ptr(semconv.SchemaURL), }, - wantResource: res, + wantResource: resource.NewWithAttributes(semconv.SchemaURL, + semconv.ServiceName("service-a"), + ), }, { name: "resource-with-additional-attributes-and-schema", @@ -86,7 +85,23 @@ func TestNewResource(t *testing.T) { }, SchemaUrl: ptr(semconv.SchemaURL), }, - wantResource: resWithAttrs, + wantResource: resource.NewWithAttributes(semconv.SchemaURL, + semconv.ServiceName("service-a"), + attribute.Bool("attr-bool", true), + attribute.String("attr-uint64", fmt.Sprintf("%d", 164)), + attribute.Int64("attr-int64", int64(-164)), + attribute.Float64("attr-float64", float64(64.0)), + attribute.Int64("attr-int8", int64(-18)), + attribute.Int64("attr-uint8", int64(18)), + attribute.Int64("attr-int16", int64(-116)), + attribute.Int64("attr-uint16", int64(116)), + attribute.Int64("attr-int32", int64(-132)), + attribute.Int64("attr-uint32", int64(132)), + attribute.Float64("attr-float32", float64(32.0)), + attribute.Int64("attr-int", int64(-1)), + attribute.String("attr-uint", fmt.Sprintf("%d", 1)), + attribute.String("attr-string", "string-val"), + attribute.String("attr-default", fmt.Sprintf("%v", other))), }, } for _, tt := range tests { From 1fc70ae15d9689d341c3ed2e9c3ab5627ac00357 Mon Sep 17 00:00:00 2001 From: Robert Pajak Date: Mon, 13 Jan 2025 11:40:55 +0100 Subject: [PATCH 2/6] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 817d890ae92..67581f39f37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix error logged by Jaeger remote sampler on empty or unset `OTEL_TRACES_SAMPLER_ARG` environment variable (#6511) - Relax minimum Go version to 1.22.0 in various modules. (#6595) +- `NewSDK` handles `OpenTelemetryConfiguration.Resource` in `go.opentelemetry.io/contrib/config/v0.3.0` properly. (#6606) From 64038d0ce8246aeed5cf4c7e0cf1a6f74ff45da7 Mon Sep 17 00:00:00 2001 From: Robert Pajak Date: Mon, 13 Jan 2025 11:45:00 +0100 Subject: [PATCH 3/6] Fix --- config/v0.3.0/resource.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/v0.3.0/resource.go b/config/v0.3.0/resource.go index 576945b2bf9..83a4fa9d323 100644 --- a/config/v0.3.0/resource.go +++ b/config/v0.3.0/resource.go @@ -47,7 +47,8 @@ func keyVal(k string, v any) attribute.KeyValue { } func newResource(res *Resource) *resource.Resource { - if res == nil || res.SchemaUrl == nil && res.Attributes == nil { + if res == nil || + res.SchemaUrl == nil && res.Attributes == nil && res.AttributesList == nil && res.Detectors == nil { return resource.Default() } From 90d6886884c8dff6b796afd114dc909567d986f2 Mon Sep 17 00:00:00 2001 From: Robert Pajak Date: Mon, 13 Jan 2025 11:48:47 +0100 Subject: [PATCH 4/6] Honor user input --- CHANGELOG.md | 2 +- config/v0.3.0/resource.go | 3 +-- config/v0.3.0/resource_test.go | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67581f39f37..4485f21f837 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix error logged by Jaeger remote sampler on empty or unset `OTEL_TRACES_SAMPLER_ARG` environment variable (#6511) - Relax minimum Go version to 1.22.0 in various modules. (#6595) -- `NewSDK` handles `OpenTelemetryConfiguration.Resource` in `go.opentelemetry.io/contrib/config/v0.3.0` properly. (#6606) +- `NewSDK` handles `OpenTelemetryConfiguration.Resource` properly in `go.opentelemetry.io/contrib/config/v0.3.0`. (#6606) diff --git a/config/v0.3.0/resource.go b/config/v0.3.0/resource.go index 83a4fa9d323..858dd384d95 100644 --- a/config/v0.3.0/resource.go +++ b/config/v0.3.0/resource.go @@ -47,8 +47,7 @@ func keyVal(k string, v any) attribute.KeyValue { } func newResource(res *Resource) *resource.Resource { - if res == nil || - res.SchemaUrl == nil && res.Attributes == nil && res.AttributesList == nil && res.Detectors == nil { + if res == nil { return resource.Default() } diff --git a/config/v0.3.0/resource_test.go b/config/v0.3.0/resource_test.go index 9dae5b8bed5..f4af030ef4c 100644 --- a/config/v0.3.0/resource_test.go +++ b/config/v0.3.0/resource_test.go @@ -30,7 +30,7 @@ func TestNewResource(t *testing.T) { { name: "resource-no-attributes", config: &Resource{}, - wantResource: resource.Default(), + wantResource: resource.NewSchemaless(), }, { name: "resource-with-schema", From 3e030c5ae0fb2ba3361bbb8c7d26a41ea2fcfe12 Mon Sep 17 00:00:00 2001 From: Robert Pajak Date: Mon, 13 Jan 2025 11:52:26 +0100 Subject: [PATCH 5/6] Refactor --- config/v0.3.0/resource.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/config/v0.3.0/resource.go b/config/v0.3.0/resource.go index 858dd384d95..ddb0af83343 100644 --- a/config/v0.3.0/resource.go +++ b/config/v0.3.0/resource.go @@ -51,15 +51,13 @@ func newResource(res *Resource) *resource.Resource { return resource.Default() } - schemaURL := "" - if res.SchemaUrl != nil { - schemaURL = *res.SchemaUrl - } - var attrs []attribute.KeyValue for _, v := range res.Attributes { attrs = append(attrs, keyVal(v.Name, v.Value)) } - return resource.NewWithAttributes(schemaURL, attrs...) + if res.SchemaUrl == nil { + return resource.NewSchemaless(attrs...) + } + return resource.NewWithAttributes(*res.SchemaUrl, attrs...) } From 0a6bb91b2229c705b343e9580642f3b52645b66a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 14 Jan 2025 16:44:11 +0100 Subject: [PATCH 6/6] Update CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07a934e8955..20033d50e5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix error logged by Jaeger remote sampler on empty or unset `OTEL_TRACES_SAMPLER_ARG` environment variable (#6511) - Relax minimum Go version to 1.22.0 in various modules. (#6595) -- `NewSDK` handles `OpenTelemetryConfiguration.Resource` properly in `go.opentelemetry.io/contrib/config/v0.3.0`. (#6606) +- `NewSDK` handles empty `OpenTelemetryConfiguration.Resource` properly in `go.opentelemetry.io/contrib/config/v0.3.0`. (#6606) +- Fix a possible nil dereference panic in `NewSDK` of `go.opentelemetry.io/contrib/config/v0.3.0`. (#6606)