[connector/spanmetrics] Use latest semantic conventions for status code attribute

Author: Machyne

.chloggen/mc_spanmetrics.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: Add a feature gate to use the latest semantic conventions for the status code attribute on generated metrics.
+
+# 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:
+
+# 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: []

connector/spanmetricsconnector/README.md

@@ -54,7 +54,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 

connector/spanmetricsconnector/connector.go

@@ -14,6 +14,7 @@ import (
 	"github.com/lightstep/go-expohisto/structure"
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/consumer"
+	"go.opentelemetry.io/collector/featuregate"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/pmetric"
 	"go.opentelemetry.io/collector/pdata/ptrace"
@@ -27,12 +28,19 @@ import (
 	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil"
 )
 
+var 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`"),
+)
+
 const (
 	serviceNameKey                 = string(conventions.ServiceNameKey)
 	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.
+	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))
@@ -542,8 +550,18 @@ func (p *connectorImp) buildAttributes(
 	if !contains(p.config.ExcludeDimensions, spanKindKey) {
 		attr.PutStr(spanKindKey, traceutil.SpanKindStr(span.Kind()))
 	}
-	if !contains(p.config.ExcludeDimensions, statusCodeKey) {
-		attr.PutStr(statusCodeKey, traceutil.StatusCodeStr(span.Status().Code()))
+	if useOtelStatusCodeAttribute.IsEnabled() {
+		if !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 !contains(p.config.ExcludeDimensions, statusCodeKey) {
+			attr.PutStr(statusCodeKey, traceutil.StatusCodeStr(span.Status().Code()))
+		}
 	}
 	if includeCollectorInstanceID.IsEnabled() {
 		if !contains(p.config.ExcludeDimensions, collectorInstanceKey) {
@@ -595,8 +613,14 @@ func (p *connectorImp) buildKey(serviceName string, span ptrace.Span, optionalDi
 	if !contains(p.config.ExcludeDimensions, spanKindKey) {
 		concatDimensionValue(p.keyBuf, traceutil.SpanKindStr(span.Kind()), true)
 	}
-	if !contains(p.config.ExcludeDimensions, statusCodeKey) {
-		concatDimensionValue(p.keyBuf, traceutil.StatusCodeStr(span.Status().Code()), true)
+	if useOtelStatusCodeAttribute.IsEnabled() {
+		if !contains(p.config.ExcludeDimensions, otelStatusCodeKey) {
+			concatDimensionValue(p.keyBuf, traceutil.StatusCodeStr(span.Status().Code()), true)
+		}
+	} else {
+		if !contains(p.config.ExcludeDimensions, statusCodeKey) {
+			concatDimensionValue(p.keyBuf, traceutil.StatusCodeStr(span.Status().Code()), true)
+		}
 	}
 
 	for _, d := range optionalDims {

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,66 @@ 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)
+
+		var expectedStatusCode string
+		val, ok := rm.Resource().Attributes().Get(serviceNameKey)
+		require.True(tb, ok)
+		serviceName := val.AsString()
+		switch serviceName {
+		case "service-a":
+			expectedStatusCode = "OK"
+		case "service-b":
+			expectedStatusCode = "ERROR"
+		case "service-c":
+			expectedStatusCode = ""
+		default:
+			require.Fail(tb, "Unexpected service name: %s", serviceName)
+		}
+
+		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()
+		for dpi := 0; dpi < dataPoints.Len(); dpi++ {
+			dp := dataPoints.At(dpi)
+			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.Equal(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)
@@ -304,6 +365,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:
@@ -374,6 +437,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)
@@ -774,6 +857,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
 		{
@@ -828,6 +912,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
 		{
@@ -894,6 +988,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())