From 4fd36dcdfd34da1a3a660a57c561a2532eb12b23 Mon Sep 17 00:00:00 2001 From: Matt Cotter Date: Wed, 17 Sep 2025 16:56:16 -0400 Subject: [PATCH] [connector/spanmetrics] Use latest semantic conventions for status code attribute --- .chloggen/mc_spanmetrics.yaml | 33 +++++ connector/spanmetricsconnector/README.md | 2 +- connector/spanmetricsconnector/connector.go | 25 +++- .../spanmetricsconnector/connector_test.go | 114 +++++++++++++++++- connector/spanmetricsconnector/factory.go | 6 + 5 files changed, 171 insertions(+), 9 deletions(-) create mode 100644 .chloggen/mc_spanmetrics.yaml diff --git a/.chloggen/mc_spanmetrics.yaml b/.chloggen/mc_spanmetrics.yaml new file mode 100644 index 0000000000000..f4d4b40ba7b39 --- /dev/null +++ b/.chloggen/mc_spanmetrics.yaml @@ -0,0 +1,33 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: connector/spanmetrics + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: + Add a feature gate to use the latest semantic conventions for the status code attribute on generated metrics. | + This feature gate will replace the `status.code` attribute on the generated RED metrics with `otel.status_code`. | + It will also replace the values `STATUS_CODE_ERROR` and `STATUS_CODE_OK` with `ERROR` and `OK` to align with the latest conventions. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [42103] + +# (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: + This change is made to align with [the latest semantic conventions](https://opentelemetry.io/docs/specs/semconv/registry/attributes/otel/#otel-status-code). | + The feature gate is disabled by default, but can be enabled with `--feature-gates spanmetrics.statusCodeConvention.useOtelPrefix` | + or explicitly disabled with `--feature-gates -spanmetrics.statusCodeConvention.useOtelPrefix`. + +# 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: [] diff --git a/connector/spanmetricsconnector/README.md b/connector/spanmetricsconnector/README.md index 9381b44ced682..b74db0823c442 100644 --- a/connector/spanmetricsconnector/README.md +++ b/connector/spanmetricsconnector/README.md @@ -59,7 +59,7 @@ across all spans: - `service.name` - `span.name` - `span.kind` -- `status.code` +- `status.code` (or `otel.status_code` when the `spanmetrics.statusCodeConvention.useOtelPrefix` feature gate is enabled) - `collector.instance.id` The `collector.instance.id` dimension is intended to add a unique UUID to all metrics, ensuring that the spanmetrics connector diff --git a/connector/spanmetricsconnector/connector.go b/connector/spanmetricsconnector/connector.go index 297e039b06f27..bac96d924c4c3 100644 --- a/connector/spanmetricsconnector/connector.go +++ b/connector/spanmetricsconnector/connector.go @@ -34,6 +34,7 @@ const ( spanKindKey = "span.kind" // OpenTelemetry non-standard constant. statusCodeKey = "status.code" // OpenTelemetry non-standard constant. collectorInstanceKey = "collector.instance.id" // OpenTelemetry non-standard constant. + otelStatusCodeKey = "otel.status_code" // 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)) @@ -535,8 +536,18 @@ func (p *connectorImp) buildAttributes( if !slices.Contains(p.config.ExcludeDimensions, spanKindKey) { attr.PutStr(spanKindKey, traceutil.SpanKindStr(span.Kind())) } - if !slices.Contains(p.config.ExcludeDimensions, statusCodeKey) { - attr.PutStr(statusCodeKey, traceutil.StatusCodeStr(span.Status().Code())) + if useOtelStatusCodeAttribute.IsEnabled() { + if !slices.Contains(p.config.ExcludeDimensions, otelStatusCodeKey) { + if span.Status().Code() == ptrace.StatusCodeError { + attr.PutStr(otelStatusCodeKey, "ERROR") + } else if span.Status().Code() == ptrace.StatusCodeOk { + attr.PutStr(otelStatusCodeKey, "OK") + } + } + } else { + if !slices.Contains(p.config.ExcludeDimensions, statusCodeKey) { + attr.PutStr(statusCodeKey, traceutil.StatusCodeStr(span.Status().Code())) + } } if includeCollectorInstanceID.IsEnabled() { if !slices.Contains(p.config.ExcludeDimensions, collectorInstanceKey) { @@ -588,8 +599,14 @@ func (p *connectorImp) buildKey(serviceName string, span ptrace.Span, optionalDi if !slices.Contains(p.config.ExcludeDimensions, spanKindKey) { concatDimensionValue(p.keyBuf, traceutil.SpanKindStr(span.Kind()), true) } - if !slices.Contains(p.config.ExcludeDimensions, statusCodeKey) { - concatDimensionValue(p.keyBuf, traceutil.StatusCodeStr(span.Status().Code()), true) + if useOtelStatusCodeAttribute.IsEnabled() { + if !slices.Contains(p.config.ExcludeDimensions, otelStatusCodeKey) { + concatDimensionValue(p.keyBuf, traceutil.StatusCodeStr(span.Status().Code()), true) + } + } else { + if !slices.Contains(p.config.ExcludeDimensions, statusCodeKey) { + concatDimensionValue(p.keyBuf, traceutil.StatusCodeStr(span.Status().Code()), true) + } } for _, d := range optionalDims { diff --git a/connector/spanmetricsconnector/connector_test.go b/connector/spanmetricsconnector/connector_test.go index 2ed5aa2671305..2a77366d4ea43 100644 --- a/connector/spanmetricsconnector/connector_test.go +++ b/connector/spanmetricsconnector/connector_test.go @@ -57,10 +57,11 @@ const ( // metricID represents the minimum attributes that uniquely identifies a metric in our tests. type metricID struct { - service string - name string - kind string - statusCode string + service string + name string + kind string + statusCode string + otelStatusCode string } type metricDataPoint interface { @@ -133,6 +134,72 @@ func verifyBadMetricsOkay(testing.TB, pmetric.Metrics) bool { return true // Validating no exception } +func verifyOtelStatusCode(tb testing.TB, input pmetric.Metrics) bool { + require.Equal(tb, 8, input.DataPointCount(), + "Should be 4 for each of call count and latency split into three resource scopes defined by: "+ + "service-a: service-a (server kind) -> service-a (client kind), "+ + "service-b: service-b (server kind), and"+ + "service-c: service-c (server kind)", + ) + + require.Equal(tb, 3, input.ResourceMetrics().Len()) + + for i := 0; i < input.ResourceMetrics().Len(); i++ { + rm := input.ResourceMetrics().At(i) + + ilm := rm.ScopeMetrics() + require.Equal(tb, 1, ilm.Len()) + assert.Equal(tb, "spanmetricsconnector", ilm.At(0).Scope().Name()) + + m := ilm.At(0).Metrics() + require.Equal(tb, 2, m.Len(), "only sum and histogram metric types generated") + + metric := m.At(0) + // validate the sum metrics for simplicity + assert.Equal(tb, pmetric.MetricTypeSum, metric.Type()) + seenMetricIDs := make(map[metricID]bool) + + dataPoints := metric.Sum().DataPoints() + var expectedStatusCode string + var serviceName string + + for dpi := 0; dpi < dataPoints.Len(); dpi++ { + dp := dataPoints.At(dpi) + + val, ok := dp.Attributes().Get(serviceNameKey) + require.True(tb, ok) + if serviceName == "" { + serviceName = val.AsString() + switch serviceName { + case "service-a": + expectedStatusCode = "OK" + case "service-b": + expectedStatusCode = "ERROR" + case "service-c": + expectedStatusCode = "" + default: + require.Fail(tb, fmt.Sprintf("Unexpected service name: %s", serviceName)) + } + } + require.Equal(tb, serviceName, val.AsString()) + + statusCode, ok := dp.Attributes().Get(otelStatusCodeKey) + if expectedStatusCode == "" { + require.False(tb, ok) + } else { + require.True(tb, ok) + assert.Equal(tb, expectedStatusCode, statusCode.AsString()) + } + verifyMetricLabels(tb, dp, seenMetricIDs) + } + for id := range seenMetricIDs { + assert.Equal(tb, expectedStatusCode, id.otelStatusCode) + assert.Empty(tb, id.statusCode) + } + } + return true +} + // verifyConsumeMetricsInputDelta expects one accumulation of metrics, and marked as delta func verifyConsumeMetricsInputDelta(tb testing.TB, input pmetric.Metrics) bool { return verifyConsumeMetricsInput(tb, input, pmetric.AggregationTemporalityDelta, 1) @@ -306,6 +373,8 @@ func verifyMetricLabels(tb testing.TB, dp metricDataPoint, seenMetricIDs map[met mID.kind = v.Str() case statusCodeKey: mID.statusCode = v.Str() + case otelStatusCodeKey: + mID.otelStatusCode = v.Str() case notInSpanAttrName1: assert.Fail(tb, notInSpanAttrName1+" should not be in this metric") default: @@ -376,6 +445,26 @@ func buildSampleTrace() ptrace.Traces { return traces } +// buildTraceWithUnsetStatusCode builds the following trace: +// +// service-c/ping (server) +func appendTraceWithUnsetStatusCode(traces ptrace.Traces) ptrace.Traces { + initServiceSpans( + serviceSpans{ + serviceName: "service-c", + spans: []span{ + { + name: "/ping", + kind: ptrace.SpanKindServer, + statusCode: ptrace.StatusCodeUnset, + traceID: [16]byte{0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F, 0x20}, + spanID: [8]byte{0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28}, + }, + }, + }, traces.ResourceSpans().AppendEmpty()) + return traces +} + func initServiceSpans(serviceSpans serviceSpans, spans ptrace.ResourceSpans) { if serviceSpans.serviceName != "" { spans.Resource().Attributes().PutStr(string(conventions.ServiceNameKey), serviceSpans.serviceName) @@ -782,6 +871,7 @@ func TestConsumeTraces(t *testing.T) { exemplarConfig func() ExemplarsConfig verifier func(tb testing.TB, input pmetric.Metrics) bool traces []ptrace.Traces + statusCodeFeatureGate bool }{ // disabling histogram { @@ -836,6 +926,16 @@ func TestConsumeTraces(t *testing.T) { verifier: verifyBadMetricsOkay, traces: []ptrace.Traces{buildBadSampleTrace()}, }, + { + // Test that the status code is set properly + name: "Test using new status code attribute", + aggregationTemporality: delta, + histogramConfig: exponentialHistogramsConfig, + exemplarConfig: disabledExemplarsConfig, + verifier: verifyOtelStatusCode, + traces: []ptrace.Traces{appendTraceWithUnsetStatusCode(buildSampleTrace())}, + statusCodeFeatureGate: true, + }, // explicit buckets histogram { @@ -902,6 +1002,12 @@ func TestConsumeTraces(t *testing.T) { require.NoError(t, err) // Override the default no-op consumer with metrics sink for testing. p.metricsConsumer = mcon + if tc.statusCodeFeatureGate { + require.NoError(t, featuregate.GlobalRegistry().Set(useOtelStatusCodeAttribute.ID(), true)) + defer func() { + require.NoError(t, featuregate.GlobalRegistry().Set(useOtelStatusCodeAttribute.ID(), false)) + }() + } ctx := metadata.NewIncomingContext(t.Context(), nil) err = p.Start(ctx, componenttest.NewNopHost()) diff --git a/connector/spanmetricsconnector/factory.go b/connector/spanmetricsconnector/factory.go index b5d9252039506..28dc4e8db1711 100644 --- a/connector/spanmetricsconnector/factory.go +++ b/connector/spanmetricsconnector/factory.go @@ -34,6 +34,7 @@ var ( includeCollectorInstanceID *featuregate.Gate useSecondAsDefaultMetricsUnit *featuregate.Gate excludeResourceMetrics *featuregate.Gate + useOtelStatusCodeAttribute *featuregate.Gate ) func init() { @@ -62,6 +63,11 @@ func init() { featuregate.WithRegisterDescription("When enabled, connector will exclude all resource attributes."), featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/42103"), ) + useOtelStatusCodeAttribute = featuregate.GlobalRegistry().MustRegister( + "spanmetrics.statusCodeConvention.useOtelPrefix", + featuregate.StageAlpha, + featuregate.WithRegisterDescription("When enabled, generated metrics will use `otel.status_code=ERROR` instead of `status.code=STATUS_CODE_ERROR`"), + ) } // NewFactory creates a factory for the spanmetrics connector.