From 3742c54497b414aaa737045232821d567772e77d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 30 Oct 2024 06:24:32 +0100 Subject: [PATCH 1/2] Make scope attributes as identifying for Tracer (#5924) Towards https://github.com/open-telemetry/opentelemetry-go/issues/3368 --- CHANGELOG.md | 1 + internal/global/trace.go | 8 +++++++- internal/global/trace_test.go | 21 ++++++++++++--------- sdk/trace/provider.go | 9 +++++---- sdk/trace/provider_test.go | 2 +- 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bf75a3cdcb..442597ef242 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` now keeps the metadata already present in the context when `WithHeaders` is used. (#5892) - `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc` now keeps the metadata already present in the context when `WithHeaders` is used. (#5911) - `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc` now keeps the metadata already present in the context when `WithHeaders` is used. (#5915) +- Support scope attributes and make them as identifying for `Tracer` in `go.opentelemetry.io/otel` and `go.opentelemetry.io/otel/sdk/trace`. (#5924) diff --git a/internal/global/trace.go b/internal/global/trace.go index e31f442b48f..ac65262c656 100644 --- a/internal/global/trace.go +++ b/internal/global/trace.go @@ -87,6 +87,7 @@ func (p *tracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T name: name, version: c.InstrumentationVersion(), schema: c.SchemaURL(), + attrs: c.InstrumentationAttributes(), } if p.tracers == nil { @@ -102,7 +103,12 @@ func (p *tracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T return t } -type il struct{ name, version, schema string } +type il struct { + name string + version string + schema string + attrs attribute.Set +} // tracer is a placeholder for a trace.Tracer. // diff --git a/internal/global/trace_test.go b/internal/global/trace_test.go index 0eb34e3a3f6..9ea150fc157 100644 --- a/internal/global/trace_test.go +++ b/internal/global/trace_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/embedded" "go.opentelemetry.io/otel/trace/noop" @@ -237,17 +238,18 @@ func TestSpanContextPropagatedWithNonRecordingSpan(t *testing.T) { } func TestTracerIdentity(t *testing.T) { - type id struct{ name, ver, url string } + type id struct{ name, ver, url, attr string } ids := []id{ - {"name-a", "version-a", "url-a"}, - {"name-a", "version-a", "url-b"}, - {"name-a", "version-b", "url-a"}, - {"name-a", "version-b", "url-b"}, - {"name-b", "version-a", "url-a"}, - {"name-b", "version-a", "url-b"}, - {"name-b", "version-b", "url-a"}, - {"name-b", "version-b", "url-b"}, + {"name-a", "version-a", "url-a", ""}, + {"name-a", "version-a", "url-a", "attr"}, + {"name-a", "version-a", "url-b", ""}, + {"name-a", "version-b", "url-a", ""}, + {"name-a", "version-b", "url-b", ""}, + {"name-b", "version-a", "url-a", ""}, + {"name-b", "version-a", "url-b", ""}, + {"name-b", "version-b", "url-a", ""}, + {"name-b", "version-b", "url-b", ""}, } provider := &tracerProvider{} @@ -256,6 +258,7 @@ func TestTracerIdentity(t *testing.T) { i.name, trace.WithInstrumentationVersion(i.ver), trace.WithSchemaURL(i.url), + trace.WithInstrumentationAttributes(attribute.String("key", i.attr)), ) } diff --git a/sdk/trace/provider.go b/sdk/trace/provider.go index 14c2e5bebda..185aa7c08f7 100644 --- a/sdk/trace/provider.go +++ b/sdk/trace/provider.go @@ -139,9 +139,10 @@ func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T name = defaultTracerName } is := instrumentation.Scope{ - Name: name, - Version: c.InstrumentationVersion(), - SchemaURL: c.SchemaURL(), + Name: name, + Version: c.InstrumentationVersion(), + SchemaURL: c.SchemaURL(), + Attributes: c.InstrumentationAttributes(), } t, ok := func() (trace.Tracer, bool) { @@ -168,7 +169,7 @@ func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T // slowing down all tracing consumers. // - Logging code may be instrumented with tracing and deadlock because it could try // acquiring the same non-reentrant mutex. - global.Info("Tracer created", "name", name, "version", is.Version, "schemaURL", is.SchemaURL) + global.Info("Tracer created", "name", name, "version", is.Version, "schemaURL", is.SchemaURL, "attributes", is.Attributes) } return t } diff --git a/sdk/trace/provider_test.go b/sdk/trace/provider_test.go index 371120fe33f..8bb8dd5fbf7 100644 --- a/sdk/trace/provider_test.go +++ b/sdk/trace/provider_test.go @@ -387,7 +387,7 @@ func TestTracerProviderReturnsSameTracer(t *testing.T) { t0, t1, t2 := p.Tracer("t0"), p.Tracer("t1"), p.Tracer("t0", trace.WithInstrumentationAttributes(attribute.String("foo", "bar"))) assert.NotSame(t, t0, t1) - assert.Same(t, t0, t2) // TODO (#3368): Change to assert.NotSame. + assert.NotSame(t, t0, t2) assert.NotSame(t, t1, t2) t3, t4, t5 := p.Tracer("t0"), p.Tracer("t1"), p.Tracer("t0", trace.WithInstrumentationAttributes(attribute.String("foo", "bar"))) From ee56fb97e0e8ed9d7c9d869aacfd6e1b978313ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 30 Oct 2024 06:29:32 +0100 Subject: [PATCH 2/2] Make scope attributes as identifying for Meter (#5926) Towards https://github.com/open-telemetry/opentelemetry-go/issues/3368 --- CHANGELOG.md | 1 + internal/global/meter.go | 1 + internal/global/meter_test.go | 21 ++++++++++++--------- sdk/metric/provider.go | 8 +++++--- sdk/metric/provider_test.go | 2 +- 5 files changed, 20 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 442597ef242..92e020f5287 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc` now keeps the metadata already present in the context when `WithHeaders` is used. (#5911) - `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc` now keeps the metadata already present in the context when `WithHeaders` is used. (#5915) - Support scope attributes and make them as identifying for `Tracer` in `go.opentelemetry.io/otel` and `go.opentelemetry.io/otel/sdk/trace`. (#5924) +- Support scope attributes and make them as identifying for `Meter` in `go.opentelemetry.io/otel` and `go.opentelemetry.io/otel/sdk/metric`. (#5926) diff --git a/internal/global/meter.go b/internal/global/meter.go index 78520e8d6e9..a6acd8dca66 100644 --- a/internal/global/meter.go +++ b/internal/global/meter.go @@ -67,6 +67,7 @@ func (p *meterProvider) Meter(name string, opts ...metric.MeterOption) metric.Me name: name, version: c.InstrumentationVersion(), schema: c.SchemaURL(), + attrs: c.InstrumentationAttributes(), } if p.meters == nil { diff --git a/internal/global/meter_test.go b/internal/global/meter_test.go index 3a700924e34..b68c363131e 100644 --- a/internal/global/meter_test.go +++ b/internal/global/meter_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/noop" ) @@ -397,17 +398,18 @@ func TestRegistrationDelegation(t *testing.T) { } func TestMeterIdentity(t *testing.T) { - type id struct{ name, ver, url string } + type id struct{ name, ver, url, attr string } ids := []id{ - {"name-a", "version-a", "url-a"}, - {"name-a", "version-a", "url-b"}, - {"name-a", "version-b", "url-a"}, - {"name-a", "version-b", "url-b"}, - {"name-b", "version-a", "url-a"}, - {"name-b", "version-a", "url-b"}, - {"name-b", "version-b", "url-a"}, - {"name-b", "version-b", "url-b"}, + {"name-a", "version-a", "url-a", ""}, + {"name-a", "version-a", "url-a", "attr"}, + {"name-a", "version-a", "url-b", ""}, + {"name-a", "version-b", "url-a", ""}, + {"name-a", "version-b", "url-b", ""}, + {"name-b", "version-a", "url-a", ""}, + {"name-b", "version-a", "url-b", ""}, + {"name-b", "version-b", "url-a", ""}, + {"name-b", "version-b", "url-b", ""}, } provider := &meterProvider{} @@ -416,6 +418,7 @@ func TestMeterIdentity(t *testing.T) { i.name, metric.WithInstrumentationVersion(i.ver), metric.WithSchemaURL(i.url), + metric.WithInstrumentationAttributes(attribute.String("key", i.attr)), ) } diff --git a/sdk/metric/provider.go b/sdk/metric/provider.go index 7b0c0dbf714..2fca89e5a8e 100644 --- a/sdk/metric/provider.go +++ b/sdk/metric/provider.go @@ -76,15 +76,17 @@ func (mp *MeterProvider) Meter(name string, options ...metric.MeterOption) metri c := metric.NewMeterConfig(options...) s := instrumentation.Scope{ - Name: name, - Version: c.InstrumentationVersion(), - SchemaURL: c.SchemaURL(), + Name: name, + Version: c.InstrumentationVersion(), + SchemaURL: c.SchemaURL(), + Attributes: c.InstrumentationAttributes(), } global.Info("Meter created", "Name", s.Name, "Version", s.Version, "SchemaURL", s.SchemaURL, + "Attributes", s.Attributes, ) return mp.meters.Lookup(s, func() *meter { diff --git a/sdk/metric/provider_test.go b/sdk/metric/provider_test.go index e5ef498ad5d..7e9361ee3d6 100644 --- a/sdk/metric/provider_test.go +++ b/sdk/metric/provider_test.go @@ -96,7 +96,7 @@ func TestMeterProviderReturnsSameMeter(t *testing.T) { assert.Same(t, mtr, mp.Meter("")) assert.NotSame(t, mtr, mp.Meter("diff")) - assert.Same(t, mtr, mp.Meter("", api.WithInstrumentationAttributes(attribute.String("k", "v")))) // TODO (#3368): Change to assert.NotSame. + assert.NotSame(t, mtr, mp.Meter("", api.WithInstrumentationAttributes(attribute.String("k", "v")))) } func TestEmptyMeterName(t *testing.T) {