From 79dc2d40cebbd69cd8173008a458522883249436 Mon Sep 17 00:00:00 2001 From: Murphy Chen Date: Thu, 21 Aug 2025 15:05:37 +0800 Subject: [PATCH 1/6] Support inserting service.instance.id as an attribute to spanmetrics metrics --- .chloggen/fix-issue-40400.yaml | 27 +++++ connector/spanmetricsconnector/README.md | 24 ++++- connector/spanmetricsconnector/config.go | 4 +- connector/spanmetricsconnector/connector.go | 11 +- .../spanmetricsconnector/connector_test.go | 100 ++++++++++++++++-- connector/spanmetricsconnector/factory.go | 24 ++++- .../spanmetricsconnector/factory_test.go | 2 + connector/spanmetricsconnector/go.mod | 2 +- 8 files changed, 174 insertions(+), 20 deletions(-) create mode 100644 .chloggen/fix-issue-40400.yaml diff --git a/.chloggen/fix-issue-40400.yaml b/.chloggen/fix-issue-40400.yaml new file mode 100644 index 0000000000000..a4bb50296c6a2 --- /dev/null +++ b/.chloggen/fix-issue-40400.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: spanmetricsconnector + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Support inserting `service.instance.id` as an attribute to spanmetrics metrics + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [40400] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user] diff --git a/connector/spanmetricsconnector/README.md b/connector/spanmetricsconnector/README.md index 975b23f269f32..28469aafe6f2f 100644 --- a/connector/spanmetricsconnector/README.md +++ b/connector/spanmetricsconnector/README.md @@ -55,7 +55,12 @@ across all spans: - `span.name` - `span.kind` - `status.code` +- `service.instance.id` +The `service.instance.id` dimension is intended to add a unique UUID to all metrics, ensuring that the spanmetrics connector +does not violate the **Single Writer Principle** when spanmetrics is used in a multi-deployment model. +Currently, `service.instance.id` must be manually enabled via the feature gate: `connector.spanmetrics.includeServiceInstanceID`. +More detail, please see [Known Limitation: the Single Writer Principle](#known-limitation-the-single-writer-principle) ## Span to Metrics processor to Span to metrics connector @@ -244,9 +249,12 @@ For more example configuration covering various other use cases, please visit th [Connectors README]: https://github.com/open-telemetry/opentelemetry-collector/blob/main/connector/README.md -## Known Limitation: Violation of the Single Writer Principle +## Known Limitation: the Single Writer Principle -The `spanmetricsconnector` currently does not guarantee adherence to the [Single Writer Principle](https://opentelemetry.io/docs/specs/otel/metrics/data-model/#single-writer), which is a core requirement in the OpenTelemetry metrics data model. Depending on how the collector is configured, multiple components may write to the same metric stream. This can result in inconsistent data, metric conflicts, or dropped series in metric backends. +Proper configuration of the `spanmetricsconnector` ensures compliance with the [Single Writer Principle](https://opentelemetry.io/docs/specs/otel/metrics/data-model/#single-writer), +which is a core requirement in the OpenTelemetry metrics data model. Misconfiguration, however, +may allow multiple components to write to the same metric stream, resulting in data inconsistency, +metric conflicts, or the dropping of time series by metric backends. ### Why this happens @@ -260,8 +268,16 @@ This issue typically arises when: To reduce the risk of conflicting writes: -* Avoid using multiple instances of the `spanmetricsconnector` unless metrics are partitioned (e.g., by attribute filtering) so each stream has a single writer -* If multiple pipelines are used, ensure each produces uniquely identified metrics (e.g., inject attributes using a processor) +* Add `resource_metrics_key_attributes` to your configuration. +``` +connectors: + spanmetrics: + resource_metrics_key_attributes: + - service.name + - telemetry.sdk.language + - telemetry.sdk.name +``` +* Manually enabled the feature gate: `connector.spanmetrics.includeServiceInstanceID` to produces uniquely identified metrics. * For exporters like Prometheus, which rely on the single writer assumption, use a dedicated pipeline with a single `spanmetricsconnector` instance More context is available in [GitHub issue #21101](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/21101). diff --git a/connector/spanmetricsconnector/config.go b/connector/spanmetricsconnector/config.go index 38e3543abd6af..a952ab22df91b 100644 --- a/connector/spanmetricsconnector/config.go +++ b/connector/spanmetricsconnector/config.go @@ -11,6 +11,7 @@ import ( "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/confmap/xconfmap" "go.opentelemetry.io/collector/pdata/pmetric" + conventions "go.opentelemetry.io/otel/semconv/v1.27.0" "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metrics" ) @@ -41,6 +42,7 @@ type Config struct { // - span.kind // - span.kind // - status.code + // - collector.instance.id This dimensions never added unless enable feature-gate connector.spanmetrics.includeServiceInstanceID // The dimensions will be fetched from the span's attributes. Examples of some conventionally used attributes: // https://github.com/open-telemetry/opentelemetry-collector/blob/main/model/semconv/opentelemetry.go. Dimensions []Dimension `mapstructure:"dimensions"` @@ -194,7 +196,7 @@ func (c Config) GetDeltaTimestampCacheSize() int { // validateDimensions checks duplicates for reserved dimensions and additional dimensions. func validateDimensions(dimensions []Dimension) error { labelNames := make(map[string]struct{}) - for _, key := range []string{serviceNameKey, spanKindKey, statusCodeKey, spanNameKey} { + for _, key := range []string{serviceNameKey, spanKindKey, statusCodeKey, spanNameKey, string(conventions.ServiceInstanceIDKey)} { labelNames[key] = struct{}{} } diff --git a/connector/spanmetricsconnector/connector.go b/connector/spanmetricsconnector/connector.go index 0d779292c692f..1e3066dd044df 100644 --- a/connector/spanmetricsconnector/connector.go +++ b/connector/spanmetricsconnector/connector.go @@ -86,6 +86,7 @@ type connectorImp struct { // Tracks the last TimestampUnixNano for delta metrics so that they represent an uninterrupted series. Unused for cumulative span metrics. lastDeltaTimestamps *simplelru.LRU[metrics.Key, pcommon.Timestamp] + instanceID string } type resourceMetrics struct { @@ -112,7 +113,7 @@ func newDimensions(cfgDims []Dimension) []utilattri.Dimension { return dims } -func newConnector(logger *zap.Logger, config component.Config, clock clockwork.Clock) (*connectorImp, error) { +func newConnector(logger *zap.Logger, config component.Config, clock clockwork.Clock, instanceID string) (*connectorImp, error) { logger.Info("Building spanmetrics connector") cfg := config.(*Config) if cfg.DimensionsCacheSize != 0 { @@ -155,6 +156,7 @@ func newConnector(logger *zap.Logger, config component.Config, clock clockwork.C callsDimensions: newDimensions(cfg.CallsDimensions), durationDimensions: newDimensions(cfg.Histogram.Dimensions), events: cfg.Events, + instanceID: instanceID, }, nil } @@ -529,7 +531,7 @@ func (p *connectorImp) buildAttributes( instrumentationScope pcommon.InstrumentationScope, ) pcommon.Map { attr := pcommon.NewMap() - attr.EnsureCapacity(4 + len(dimensions)) + attr.EnsureCapacity(5 + len(dimensions)) if !contains(p.config.ExcludeDimensions, serviceNameKey) { attr.PutStr(serviceNameKey, serviceName) } @@ -542,6 +544,11 @@ func (p *connectorImp) buildAttributes( if !contains(p.config.ExcludeDimensions, statusCodeKey) { attr.PutStr(statusCodeKey, traceutil.StatusCodeStr(span.Status().Code())) } + if includeServiceInstanceID.IsEnabled() { + if !contains(p.config.ExcludeDimensions, string(conventions.ServiceInstanceIDKey)) { + attr.PutStr(string(conventions.ServiceInstanceIDKey), p.instanceID) + } + } if contains(p.config.IncludeInstrumentationScope, instrumentationScope.Name()) && instrumentationScope.Name() != "" { attr.PutStr(instrumentationScopeNameKey, instrumentationScope.Name()) diff --git a/connector/spanmetricsconnector/connector_test.go b/connector/spanmetricsconnector/connector_test.go index 6c1682357f18c..ace7816ef7f34 100644 --- a/connector/spanmetricsconnector/connector_test.go +++ b/connector/spanmetricsconnector/connector_test.go @@ -51,6 +51,8 @@ const ( sampleRegion = "us-east-1" sampleDuration = float64(11) + + instanceID = "0044953a-2946-449f-a5c8-2971f2a63928" ) // metricID represents the minimum attributes that uniquely identifies a metric in our tests. @@ -490,7 +492,7 @@ func newConnectorImp(defaultNullValue *string, histogramConfig func() HistogramC MetricsFlushInterval: time.Nanosecond, } - c, err := newConnector(zap.NewNop(), cfg, clock) + c, err := newConnector(zap.NewNop(), cfg, clock, instanceID) if err != nil { return nil, err } @@ -505,7 +507,7 @@ func stringp(str string) *string { func TestBuildKeySameServiceNameCharSequence(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig().(*Config) - c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock()) + c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock(), instanceID) require.NoError(t, err) span0 := ptrace.NewSpan() @@ -525,7 +527,7 @@ func TestBuildKeyExcludeDimensionsAll(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig().(*Config) cfg.ExcludeDimensions = []string{"span.kind", "service.name", "span.name", "status.code"} - c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock()) + c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock(), instanceID) require.NoError(t, err) span0 := ptrace.NewSpan() @@ -538,7 +540,7 @@ func TestBuildKeyExcludeWrongDimensions(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig().(*Config) cfg.ExcludeDimensions = []string{"span.kind", "service.name.wrong.name", "span.name", "status.code"} - c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock()) + c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock(), instanceID) require.NoError(t, err) span0 := ptrace.NewSpan() @@ -550,7 +552,7 @@ func TestBuildKeyExcludeWrongDimensions(t *testing.T) { func TestBuildKeyWithDimensions(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig().(*Config) - c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock()) + c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock(), instanceID) require.NoError(t, err) defaultFoo := pcommon.NewValueStr("bar") @@ -691,7 +693,7 @@ func TestConnectorCapabilities(t *testing.T) { cfg := factory.CreateDefaultConfig().(*Config) // Test - c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock()) + c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock(), instanceID) // Override the default no-op consumer for testing. c.metricsConsumer = new(consumertest.MetricsSink) assert.NoError(t, err) @@ -1519,7 +1521,7 @@ func TestSpanMetrics_Events(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig().(*Config) cfg.Events = tt.eventsConfig - c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock()) + c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock(), instanceID) require.NoError(t, err) err = c.ConsumeTraces(t.Context(), buildSampleTrace()) require.NoError(t, err) @@ -1873,7 +1875,7 @@ func TestSeparateDimensions(t *testing.T) { cfg.Dimensions = []Dimension{{Name: stringAttrName, Default: nil}} cfg.CallsDimensions = []Dimension{{Name: intAttrName, Default: stringp("0")}} cfg.Histogram.Dimensions = []Dimension{{Name: doubleAttrName, Default: stringp("0.0")}} - c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock()) + c, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock(), instanceID) require.NoError(t, err) err = c.ConsumeTraces(t.Context(), buildSampleTrace()) require.NoError(t, err) @@ -2030,7 +2032,7 @@ func TestConnectorWithCardinalityLimit(t *testing.T) { {Name: "region"}, } - connector, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock()) + connector, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock(), instanceID) require.NoError(t, err) require.NotNil(t, connector) @@ -2168,7 +2170,7 @@ func TestConnectorWithCardinalityLimitForEvents(t *testing.T) { {Name: "event.name"}, } - connector, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock()) + connector, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock(), instanceID) require.NoError(t, err) require.NotNil(t, connector) @@ -2251,3 +2253,81 @@ func TestConnectorWithCardinalityLimitForEvents(t *testing.T) { assert.Equal(t, 2, normalCount, "expected 2 normal metrics") assert.Equal(t, 1, overflowCount, "expected 1 overflow metric") } + +func TestBuildAttributesWithFeatureGate(t *testing.T) { + tests := []struct { + name string + instrumentationScope pcommon.InstrumentationScope + config Config + want map[string]string + includeCollectorInstanceID bool + }{ + { + name: "disable includeServiceInstanceID feature-gate", + instrumentationScope: func() pcommon.InstrumentationScope { + scope := pcommon.NewInstrumentationScope() + scope.SetName("express") + scope.SetVersion("1.0.0") + return scope + }(), + config: Config{ + IncludeInstrumentationScope: []string{"express"}, + }, + want: map[string]string{ + serviceNameKey: "test_service", + spanNameKey: "test_span", + spanKindKey: "SPAN_KIND_INTERNAL", + statusCodeKey: "STATUS_CODE_UNSET", + instrumentationScopeNameKey: "express", + instrumentationScopeVersionKey: "1.0.0", + }, + }, + { + name: "enable includeServiceInstanceID feature-gate", + instrumentationScope: func() pcommon.InstrumentationScope { + scope := pcommon.NewInstrumentationScope() + scope.SetName("express") + scope.SetVersion("1.0.0") + return scope + }(), + config: Config{ + IncludeInstrumentationScope: []string{"express"}, + }, + want: map[string]string{ + serviceNameKey: "test_service", + spanNameKey: "test_span", + spanKindKey: "SPAN_KIND_INTERNAL", + statusCodeKey: "STATUS_CODE_UNSET", + instrumentationScopeNameKey: "express", + instrumentationScopeVersionKey: "1.0.0", + string(conventions.ServiceInstanceIDKey): instanceID, + }, + includeCollectorInstanceID: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &connectorImp{config: tt.config, instanceID: instanceID} + if tt.includeCollectorInstanceID { + require.NoError(t, featuregate.GlobalRegistry().Set(includeServiceInstanceID.ID(), true)) + } + defer func() { + require.NoError(t, featuregate.GlobalRegistry().Set(includeServiceInstanceID.ID(), false)) + }() + + span := ptrace.NewSpan() + span.SetName("test_span") + span.SetKind(ptrace.SpanKindInternal) + + attrs := p.buildAttributes("test_service", span, pcommon.NewMap(), nil, tt.instrumentationScope) + + assert.Equal(t, len(tt.want), attrs.Len()) + for k, v := range tt.want { + val, ok := attrs.Get(k) + assert.True(t, ok) + assert.Equal(t, v, val.Str()) + } + }) + } +} diff --git a/connector/spanmetricsconnector/factory.go b/connector/spanmetricsconnector/factory.go index f27b35c2e59a9..2d75499a85745 100644 --- a/connector/spanmetricsconnector/factory.go +++ b/connector/spanmetricsconnector/factory.go @@ -9,11 +9,14 @@ import ( "context" "time" + "github.com/google/uuid" "github.com/jonboulle/clockwork" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/connector" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/featuregate" + "go.opentelemetry.io/collector/pdata/pcommon" + conventions "go.opentelemetry.io/otel/semconv/v1.27.0" "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metadata" ) @@ -21,9 +24,13 @@ import ( const ( DefaultNamespace = "traces.span.metrics" legacyMetricNamesFeatureGateID = "connector.spanmetrics.legacyMetricNames" + includeServiceInstanceIDGateID = "connector.spanmetrics.includeServiceInstanceID" ) -var legacyMetricNamesFeatureGate *featuregate.Gate +var ( + legacyMetricNamesFeatureGate *featuregate.Gate + includeServiceInstanceID *featuregate.Gate +) func init() { // TODO: Remove this feature gate when the legacy metric names are removed. @@ -33,6 +40,12 @@ func init() { featuregate.WithRegisterDescription("When enabled, connector uses legacy metric names."), featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/33227"), ) + includeServiceInstanceID = featuregate.GlobalRegistry().MustRegister( + includeServiceInstanceIDGateID, + featuregate.StageAlpha, + featuregate.WithRegisterDescription("When enabled, connector add service.instance.id to default dimensions."), + featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/40400"), + ) } // NewFactory creates a factory for the spanmetrics connector. @@ -59,7 +72,14 @@ func createDefaultConfig() component.Config { } func createTracesToMetricsConnector(ctx context.Context, params connector.Settings, cfg component.Config, nextConsumer consumer.Metrics) (connector.Traces, error) { - c, err := newConnector(params.Logger, cfg, clockwork.FromContext(ctx)) + instanceID, ok := params.Resource.Attributes().Get(string(conventions.ServiceInstanceIDKey)) + // this never happen, just for test + if !ok { + instanceUUID, _ := uuid.NewRandom() + instanceID = pcommon.NewValueStr(instanceUUID.String()) + } + + c, err := newConnector(params.Logger, cfg, clockwork.FromContext(ctx), instanceID.AsString()) if err != nil { return nil, err } diff --git a/connector/spanmetricsconnector/factory_test.go b/connector/spanmetricsconnector/factory_test.go index e6633f00a775e..df6d2c801b501 100644 --- a/connector/spanmetricsconnector/factory_test.go +++ b/connector/spanmetricsconnector/factory_test.go @@ -13,6 +13,7 @@ import ( "go.opentelemetry.io/collector/connector/connectortest" "go.opentelemetry.io/collector/consumer/consumertest" "go.opentelemetry.io/collector/pdata/pcommon" + conventions "go.opentelemetry.io/otel/semconv/v1.27.0" "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metadata" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" @@ -49,6 +50,7 @@ func TestNewConnector(t *testing.T) { factory := NewFactory() creationParams := connectortest.NewNopSettings(metadata.Type) + creationParams.Resource.Attributes().PutStr(string(conventions.ServiceInstanceIDKey), instanceID) cfg := factory.CreateDefaultConfig().(*Config) cfg.Histogram.Explicit = configoptional.Some(ExplicitHistogramConfig{ Buckets: tc.durationHistogramBuckets, diff --git a/connector/spanmetricsconnector/go.mod b/connector/spanmetricsconnector/go.mod index 774501caef75e..0381d070667ad 100644 --- a/connector/spanmetricsconnector/go.mod +++ b/connector/spanmetricsconnector/go.mod @@ -3,6 +3,7 @@ module github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanm go 1.24 require ( + github.com/google/uuid v1.6.0 github.com/hashicorp/golang-lru/v2 v2.0.7 github.com/jonboulle/clockwork v0.5.0 github.com/lightstep/go-expohisto v1.0.0 @@ -36,7 +37,6 @@ require ( github.com/go-viper/mapstructure/v2 v2.4.0 // indirect github.com/gobwas/glob v0.2.3 // indirect github.com/gogo/protobuf v1.3.2 // indirect - github.com/google/uuid v1.6.0 // indirect github.com/hashicorp/go-version v1.7.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/knadh/koanf/maps v0.1.2 // indirect From 073105f811fa5a3e6e115b01f9fa33b909beec87 Mon Sep 17 00:00:00 2001 From: Murphy Chen Date: Tue, 12 Aug 2025 11:28:16 +0800 Subject: [PATCH 2/6] apply reviewer's suggestion --- .chloggen/fix-issue-40400.yaml | 5 +++-- connector/spanmetricsconnector/factory.go | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.chloggen/fix-issue-40400.yaml b/.chloggen/fix-issue-40400.yaml index a4bb50296c6a2..bd4b3985b833f 100644 --- a/.chloggen/fix-issue-40400.yaml +++ b/.chloggen/fix-issue-40400.yaml @@ -7,7 +7,7 @@ change_type: enhancement component: spanmetricsconnector # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: Support inserting `service.instance.id` as an attribute to spanmetrics metrics +note: Supports adding the `service.instance.id` attribute to data points generated by the spanmetrics connector. # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. issues: [40400] @@ -15,7 +15,8 @@ issues: [40400] # (Optional) One or more lines of additional information to render under the primary note. # These lines will be padded with 2 spaces and then inserted directly into the document. # Use pipe (|) for multiline entries. -subtext: +subtext: | + This feature currently in alpha stage, user should enable it by feature-gate `--feature-gates=+connector.spanmetrics.includeServiceInstanceID` # If your change doesn't affect end users or the exported elements of any package, # you should instead start your pull request title with [chore] or use the "Skip Changelog" label. diff --git a/connector/spanmetricsconnector/factory.go b/connector/spanmetricsconnector/factory.go index 2d75499a85745..b1af5a3217dc6 100644 --- a/connector/spanmetricsconnector/factory.go +++ b/connector/spanmetricsconnector/factory.go @@ -73,7 +73,11 @@ func createDefaultConfig() component.Config { func createTracesToMetricsConnector(ctx context.Context, params connector.Settings, cfg component.Config, nextConsumer consumer.Metrics) (connector.Traces, error) { instanceID, ok := params.Resource.Attributes().Get(string(conventions.ServiceInstanceIDKey)) - // this never happen, just for test + // This never happens: the OpenTelemetry Collector automatically adds this attribute. + // See: https://github.com/open-telemetry/opentelemetry-collector/blob/main/service/internal/resource/config.go#L31 + // + // The fallback logic below exists solely for lifecycle tests in generated_component_test.go, + // where the mocked telemetry setting does not include the service.instance.id attribute. if !ok { instanceUUID, _ := uuid.NewRandom() instanceID = pcommon.NewValueStr(instanceUUID.String()) From e6dbb1c184be4aef335d6dd652732a347c4ba796 Mon Sep 17 00:00:00 2001 From: Murphy Chen Date: Wed, 13 Aug 2025 21:43:20 +0800 Subject: [PATCH 3/6] apply reviewer's suggestion --- connector/spanmetricsconnector/config.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/connector/spanmetricsconnector/config.go b/connector/spanmetricsconnector/config.go index a952ab22df91b..0896ff7ac1908 100644 --- a/connector/spanmetricsconnector/config.go +++ b/connector/spanmetricsconnector/config.go @@ -196,7 +196,12 @@ func (c Config) GetDeltaTimestampCacheSize() int { // validateDimensions checks duplicates for reserved dimensions and additional dimensions. func validateDimensions(dimensions []Dimension) error { labelNames := make(map[string]struct{}) - for _, key := range []string{serviceNameKey, spanKindKey, statusCodeKey, spanNameKey, string(conventions.ServiceInstanceIDKey)} { + intervalLabels := []string{serviceNameKey, spanKindKey, statusCodeKey, spanNameKey} + if includeServiceInstanceID.IsEnabled() { + intervalLabels = append(intervalLabels, string(conventions.ServiceInstanceIDKey)) + } + + for _, key := range intervalLabels { labelNames[key] = struct{}{} } From 383abd31a9f936a44737fcb74af2a1687367b6a8 Mon Sep 17 00:00:00 2001 From: Murphy Chen Date: Thu, 21 Aug 2025 15:16:34 +0800 Subject: [PATCH 4/6] use collector.instance.id --- .chloggen/fix-issue-40400.yaml | 4 ++-- connector/spanmetricsconnector/README.md | 6 ++--- connector/spanmetricsconnector/config.go | 10 ++++----- connector/spanmetricsconnector/connector.go | 7 +++--- .../spanmetricsconnector/connector_test.go | 22 +++++++++---------- connector/spanmetricsconnector/factory.go | 20 ++++++++--------- .../spanmetricsconnector/factory_test.go | 8 +++---- 7 files changed, 36 insertions(+), 41 deletions(-) diff --git a/.chloggen/fix-issue-40400.yaml b/.chloggen/fix-issue-40400.yaml index bd4b3985b833f..f4a5b8af4f0e3 100644 --- a/.chloggen/fix-issue-40400.yaml +++ b/.chloggen/fix-issue-40400.yaml @@ -7,7 +7,7 @@ change_type: enhancement component: spanmetricsconnector # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: Supports adding the `service.instance.id` attribute to data points generated by the spanmetrics connector. +note: Supports adding the `collector.instance.id` attribute to data points generated by the spanmetrics connector. # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. issues: [40400] @@ -16,7 +16,7 @@ issues: [40400] # These lines will be padded with 2 spaces and then inserted directly into the document. # Use pipe (|) for multiline entries. subtext: | - This feature currently in alpha stage, user should enable it by feature-gate `--feature-gates=+connector.spanmetrics.includeServiceInstanceID` + This feature currently in alpha stage, user should enable it by feature-gate `--feature-gates=+connector.spanmetrics.includeCollectorInstanceID` # If your change doesn't affect end users or the exported elements of any package, # you should instead start your pull request title with [chore] or use the "Skip Changelog" label. diff --git a/connector/spanmetricsconnector/README.md b/connector/spanmetricsconnector/README.md index 28469aafe6f2f..cea212b5af16b 100644 --- a/connector/spanmetricsconnector/README.md +++ b/connector/spanmetricsconnector/README.md @@ -55,11 +55,11 @@ across all spans: - `span.name` - `span.kind` - `status.code` -- `service.instance.id` +- `collector.instance.id` -The `service.instance.id` dimension is intended to add a unique UUID to all metrics, ensuring that the spanmetrics connector +The `collector.instance.id` dimension is intended to add a unique UUID to all metrics, ensuring that the spanmetrics connector does not violate the **Single Writer Principle** when spanmetrics is used in a multi-deployment model. -Currently, `service.instance.id` must be manually enabled via the feature gate: `connector.spanmetrics.includeServiceInstanceID`. +Currently, `collector.instance.id` must be manually enabled via the feature gate: `connector.spanmetrics.includeServiceInstanceID`. More detail, please see [Known Limitation: the Single Writer Principle](#known-limitation-the-single-writer-principle) ## Span to Metrics processor to Span to metrics connector diff --git a/connector/spanmetricsconnector/config.go b/connector/spanmetricsconnector/config.go index 0896ff7ac1908..23b396e3cc23c 100644 --- a/connector/spanmetricsconnector/config.go +++ b/connector/spanmetricsconnector/config.go @@ -8,12 +8,10 @@ import ( "fmt" "time" + "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metrics" "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/confmap/xconfmap" "go.opentelemetry.io/collector/pdata/pmetric" - conventions "go.opentelemetry.io/otel/semconv/v1.27.0" - - "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metrics" ) const ( @@ -42,7 +40,7 @@ type Config struct { // - span.kind // - span.kind // - status.code - // - collector.instance.id This dimensions never added unless enable feature-gate connector.spanmetrics.includeServiceInstanceID + // - collector.instance.id This dimensions never added unless enable feature-gate connector.spanmetrics.includeCollectorInstanceID // The dimensions will be fetched from the span's attributes. Examples of some conventionally used attributes: // https://github.com/open-telemetry/opentelemetry-collector/blob/main/model/semconv/opentelemetry.go. Dimensions []Dimension `mapstructure:"dimensions"` @@ -197,8 +195,8 @@ func (c Config) GetDeltaTimestampCacheSize() int { func validateDimensions(dimensions []Dimension) error { labelNames := make(map[string]struct{}) intervalLabels := []string{serviceNameKey, spanKindKey, statusCodeKey, spanNameKey} - if includeServiceInstanceID.IsEnabled() { - intervalLabels = append(intervalLabels, string(conventions.ServiceInstanceIDKey)) + if includeCollectorInstanceID.IsEnabled() { + intervalLabels = append(intervalLabels, collectorInstanceKey) } for _, key := range intervalLabels { diff --git a/connector/spanmetricsconnector/connector.go b/connector/spanmetricsconnector/connector.go index 1e3066dd044df..26ecc414e2b68 100644 --- a/connector/spanmetricsconnector/connector.go +++ b/connector/spanmetricsconnector/connector.go @@ -32,6 +32,7 @@ const ( spanNameKey = "span.name" // OpenTelemetry non-standard constant. spanKindKey = "span.kind" // OpenTelemetry non-standard constant. statusCodeKey = "status.code" // OpenTelemetry non-standard constant. + collectorInstanceKey = "collector.instance.id" // OpenTelemetry non-standard constant. instrumentationScopeNameKey = "span.instrumentation.scope.name" // OpenTelemetry non-standard constant. instrumentationScopeVersionKey = "span.instrumentation.scope.version" // OpenTelemetry non-standard constant. metricKeySeparator = string(byte(0)) @@ -544,9 +545,9 @@ func (p *connectorImp) buildAttributes( if !contains(p.config.ExcludeDimensions, statusCodeKey) { attr.PutStr(statusCodeKey, traceutil.StatusCodeStr(span.Status().Code())) } - if includeServiceInstanceID.IsEnabled() { - if !contains(p.config.ExcludeDimensions, string(conventions.ServiceInstanceIDKey)) { - attr.PutStr(string(conventions.ServiceInstanceIDKey), p.instanceID) + if includeCollectorInstanceID.IsEnabled() { + if !contains(p.config.ExcludeDimensions, collectorInstanceKey) { + attr.PutStr(collectorInstanceKey, p.instanceID) } } diff --git a/connector/spanmetricsconnector/connector_test.go b/connector/spanmetricsconnector/connector_test.go index ace7816ef7f34..f1e1ba727d14a 100644 --- a/connector/spanmetricsconnector/connector_test.go +++ b/connector/spanmetricsconnector/connector_test.go @@ -2263,7 +2263,7 @@ func TestBuildAttributesWithFeatureGate(t *testing.T) { includeCollectorInstanceID bool }{ { - name: "disable includeServiceInstanceID feature-gate", + name: "disable includeCollectorInstanceID feature-gate", instrumentationScope: func() pcommon.InstrumentationScope { scope := pcommon.NewInstrumentationScope() scope.SetName("express") @@ -2283,7 +2283,7 @@ func TestBuildAttributesWithFeatureGate(t *testing.T) { }, }, { - name: "enable includeServiceInstanceID feature-gate", + name: "enable includeCollectorInstanceID feature-gate", instrumentationScope: func() pcommon.InstrumentationScope { scope := pcommon.NewInstrumentationScope() scope.SetName("express") @@ -2294,13 +2294,13 @@ func TestBuildAttributesWithFeatureGate(t *testing.T) { IncludeInstrumentationScope: []string{"express"}, }, want: map[string]string{ - serviceNameKey: "test_service", - spanNameKey: "test_span", - spanKindKey: "SPAN_KIND_INTERNAL", - statusCodeKey: "STATUS_CODE_UNSET", - instrumentationScopeNameKey: "express", - instrumentationScopeVersionKey: "1.0.0", - string(conventions.ServiceInstanceIDKey): instanceID, + serviceNameKey: "test_service", + spanNameKey: "test_span", + spanKindKey: "SPAN_KIND_INTERNAL", + statusCodeKey: "STATUS_CODE_UNSET", + instrumentationScopeNameKey: "express", + instrumentationScopeVersionKey: "1.0.0", + collectorInstanceKey: instanceID, }, includeCollectorInstanceID: true, }, @@ -2310,10 +2310,10 @@ func TestBuildAttributesWithFeatureGate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { p := &connectorImp{config: tt.config, instanceID: instanceID} if tt.includeCollectorInstanceID { - require.NoError(t, featuregate.GlobalRegistry().Set(includeServiceInstanceID.ID(), true)) + require.NoError(t, featuregate.GlobalRegistry().Set(includeCollectorInstanceID.ID(), true)) } defer func() { - require.NoError(t, featuregate.GlobalRegistry().Set(includeServiceInstanceID.ID(), false)) + require.NoError(t, featuregate.GlobalRegistry().Set(includeCollectorInstanceID.ID(), false)) }() span := ptrace.NewSpan() diff --git a/connector/spanmetricsconnector/factory.go b/connector/spanmetricsconnector/factory.go index b1af5a3217dc6..26f334dc3c79b 100644 --- a/connector/spanmetricsconnector/factory.go +++ b/connector/spanmetricsconnector/factory.go @@ -11,25 +11,23 @@ import ( "github.com/google/uuid" "github.com/jonboulle/clockwork" + "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metadata" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/connector" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/featuregate" "go.opentelemetry.io/collector/pdata/pcommon" - conventions "go.opentelemetry.io/otel/semconv/v1.27.0" - - "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metadata" ) const ( - DefaultNamespace = "traces.span.metrics" - legacyMetricNamesFeatureGateID = "connector.spanmetrics.legacyMetricNames" - includeServiceInstanceIDGateID = "connector.spanmetrics.includeServiceInstanceID" + DefaultNamespace = "traces.span.metrics" + legacyMetricNamesFeatureGateID = "connector.spanmetrics.legacyMetricNames" + includeCollectorInstanceIDGateID = "connector.spanmetrics.includeCollectorInstanceID" ) var ( legacyMetricNamesFeatureGate *featuregate.Gate - includeServiceInstanceID *featuregate.Gate + includeCollectorInstanceID *featuregate.Gate ) func init() { @@ -40,10 +38,10 @@ func init() { featuregate.WithRegisterDescription("When enabled, connector uses legacy metric names."), featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/33227"), ) - includeServiceInstanceID = featuregate.GlobalRegistry().MustRegister( - includeServiceInstanceIDGateID, + includeCollectorInstanceID = featuregate.GlobalRegistry().MustRegister( + includeCollectorInstanceIDGateID, featuregate.StageAlpha, - featuregate.WithRegisterDescription("When enabled, connector add service.instance.id to default dimensions."), + featuregate.WithRegisterDescription("When enabled, connector add collector.instance.id to default dimensions."), featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/40400"), ) } @@ -72,7 +70,7 @@ func createDefaultConfig() component.Config { } func createTracesToMetricsConnector(ctx context.Context, params connector.Settings, cfg component.Config, nextConsumer consumer.Metrics) (connector.Traces, error) { - instanceID, ok := params.Resource.Attributes().Get(string(conventions.ServiceInstanceIDKey)) + instanceID, ok := params.Resource.Attributes().Get(collectorInstanceKey) // This never happens: the OpenTelemetry Collector automatically adds this attribute. // See: https://github.com/open-telemetry/opentelemetry-collector/blob/main/service/internal/resource/config.go#L31 // diff --git a/connector/spanmetricsconnector/factory_test.go b/connector/spanmetricsconnector/factory_test.go index df6d2c801b501..e7afb163d5bc7 100644 --- a/connector/spanmetricsconnector/factory_test.go +++ b/connector/spanmetricsconnector/factory_test.go @@ -8,15 +8,13 @@ import ( "testing" "time" + "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metadata" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/connector/connectortest" "go.opentelemetry.io/collector/consumer/consumertest" "go.opentelemetry.io/collector/pdata/pcommon" - conventions "go.opentelemetry.io/otel/semconv/v1.27.0" - - "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metadata" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" ) func TestNewConnector(t *testing.T) { @@ -50,7 +48,7 @@ func TestNewConnector(t *testing.T) { factory := NewFactory() creationParams := connectortest.NewNopSettings(metadata.Type) - creationParams.Resource.Attributes().PutStr(string(conventions.ServiceInstanceIDKey), instanceID) + creationParams.Resource.Attributes().PutStr(collectorInstanceKey, instanceID) cfg := factory.CreateDefaultConfig().(*Config) cfg.Histogram.Explicit = configoptional.Some(ExplicitHistogramConfig{ Buckets: tc.durationHistogramBuckets, From 48f579eb21f2bd2cd277e8d3fc192a1d2f44a3e7 Mon Sep 17 00:00:00 2001 From: Murphy Chen Date: Thu, 21 Aug 2025 15:28:52 +0800 Subject: [PATCH 5/6] fmt --- connector/spanmetricsconnector/config.go | 3 ++- connector/spanmetricsconnector/factory.go | 3 ++- connector/spanmetricsconnector/factory_test.go | 5 +++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/connector/spanmetricsconnector/config.go b/connector/spanmetricsconnector/config.go index 23b396e3cc23c..73f0943fee313 100644 --- a/connector/spanmetricsconnector/config.go +++ b/connector/spanmetricsconnector/config.go @@ -8,10 +8,11 @@ import ( "fmt" "time" - "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metrics" "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/confmap/xconfmap" "go.opentelemetry.io/collector/pdata/pmetric" + + "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metrics" ) const ( diff --git a/connector/spanmetricsconnector/factory.go b/connector/spanmetricsconnector/factory.go index 26f334dc3c79b..6dea8ab1ef626 100644 --- a/connector/spanmetricsconnector/factory.go +++ b/connector/spanmetricsconnector/factory.go @@ -11,12 +11,13 @@ import ( "github.com/google/uuid" "github.com/jonboulle/clockwork" - "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metadata" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/connector" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/featuregate" "go.opentelemetry.io/collector/pdata/pcommon" + + "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metadata" ) const ( diff --git a/connector/spanmetricsconnector/factory_test.go b/connector/spanmetricsconnector/factory_test.go index e7afb163d5bc7..845e847a9347d 100644 --- a/connector/spanmetricsconnector/factory_test.go +++ b/connector/spanmetricsconnector/factory_test.go @@ -8,13 +8,14 @@ import ( "testing" "time" - "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metadata" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/connector/connectortest" "go.opentelemetry.io/collector/consumer/consumertest" "go.opentelemetry.io/collector/pdata/pcommon" + + "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metadata" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" ) func TestNewConnector(t *testing.T) { From da3514e22445ccaa6fbb095e88295ea0b391ed1e Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Wed, 10 Sep 2025 23:01:15 -0400 Subject: [PATCH 6/6] Update connector/spanmetricsconnector/README.md --- connector/spanmetricsconnector/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/connector/spanmetricsconnector/README.md b/connector/spanmetricsconnector/README.md index cea212b5af16b..32cb665db33dd 100644 --- a/connector/spanmetricsconnector/README.md +++ b/connector/spanmetricsconnector/README.md @@ -277,7 +277,7 @@ connectors: - telemetry.sdk.language - telemetry.sdk.name ``` -* Manually enabled the feature gate: `connector.spanmetrics.includeServiceInstanceID` to produces uniquely identified metrics. +* Manually enable the feature gate: `connector.spanmetrics.includeServiceInstanceID` to produce uniquely identified metrics. * For exporters like Prometheus, which rely on the single writer assumption, use a dedicated pipeline with a single `spanmetricsconnector` instance More context is available in [GitHub issue #21101](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/21101).