diff --git a/pkg/trace/semantics/lookup.go b/pkg/trace/semantics/lookup.go index 44bfe9917bb3..574b79c257b9 100644 --- a/pkg/trace/semantics/lookup.go +++ b/pkg/trace/semantics/lookup.go @@ -97,6 +97,42 @@ type LookupResult struct { StringValue string } +// conditionMatches reports whether a single Condition holds against the accessor's raw +// attribute store. Conditions read attributes by exact key — fallback chains are not +// followed. To gate on a renamed attribute (e.g. rpc.system → rpc.system.name), list +// one fallback entry per condition attribute in mappings.json. +// +// A Condition has up to two predicate fields, all of which must hold (logical AND) +// for the condition to match: +// +// - Present: requires the attribute to be present (true) or absent (false). +// - Eq: requires the attribute's value to equal the literal string. Implies presence. +// +// A Condition with no predicates set always matches (empty conjunction is true). +func conditionMatches[A Accessor](accessor A, c Condition) bool { + value := accessor.GetString(c.Attribute) + found := value != "" + + if c.Present != nil && found != *c.Present { + return false + } + if c.Eq != nil { + if !found || value != *c.Eq { + return false + } + } + return true +} + +func conditionsMatch[A Accessor](accessor A, conditions []Condition) bool { + for _, c := range conditions { + if !conditionMatches(accessor, c) { + return false + } + } + return true +} + // Lookup performs a semantic attribute lookup in precedence order and returns the first match. // For string-typed tags it uses GetString; for numeric-typed tags it uses the typed getter and // formats the result as a string, so LookupString works correctly on any concept regardless of @@ -107,6 +143,9 @@ func Lookup[A Accessor](r Registry, accessor A, concept Concept) (LookupResult, return LookupResult{}, false } for _, tag := range tags { + if len(tag.When) > 0 && !conditionsMatch(accessor, tag.When) { + continue + } switch tag.Type { case ValueTypeInt64: if v, ok := accessor.GetInt64(tag.Name); ok { @@ -143,6 +182,9 @@ func LookupFloat64[A Accessor](r Registry, accessor A, concept Concept) (float64 return 0, false } for _, tag := range tags { + if len(tag.When) > 0 && !conditionsMatch(accessor, tag.When) { + continue + } switch tag.Type { case ValueTypeFloat64: if v, ok := accessor.GetFloat64(tag.Name); ok { @@ -172,6 +214,9 @@ func LookupInt64[A Accessor](r Registry, accessor A, concept Concept) (int64, bo return 0, false } for _, tag := range tags { + if len(tag.When) > 0 && !conditionsMatch(accessor, tag.When) { + continue + } switch tag.Type { case ValueTypeInt64: if v, ok := accessor.GetInt64(tag.Name); ok { diff --git a/pkg/trace/semantics/lookup_test.go b/pkg/trace/semantics/lookup_test.go index e7ec6c07b65c..e83f50eb7d05 100644 --- a/pkg/trace/semantics/lookup_test.go +++ b/pkg/trace/semantics/lookup_test.go @@ -99,6 +99,95 @@ func TestLookup(t *testing.T) { }) } +// TestGRPCStatusCodeConditionalFallback exercises the production rpc.grpc.status_code +// mapping — rpc.response.status_code is only accepted when rpc.system.name=grpc, or as +// a legacy fallback when rpc.system=grpc and rpc.system.name is absent. The legacy +// row's two-condition AND also gives ANDing implicit coverage. +func TestGRPCStatusCodeConditionalFallback(t *testing.T) { + r, err := NewEmbeddedRegistry() + require.NoError(t, err) + + for _, tt := range []struct { + name string + attrs map[string]any + want string + }{ + { + name: "explicit rpc.grpc.status_code wins regardless of system", + attrs: map[string]any{ + "rpc.grpc.status_code": "3", + "rpc.response.status_code": "4", + "rpc.system.name": "jsonrpc", + }, + want: "3", + }, + { + name: "rpc.response.status_code accepted when rpc.system.name=grpc", + attrs: map[string]any{ + "rpc.response.status_code": "DEADLINE_EXCEEDED", + "rpc.system.name": "grpc", + }, + want: "DEADLINE_EXCEEDED", + }, + { + name: "rpc.response.status_code accepted via legacy rpc.system=grpc", + attrs: map[string]any{ + "rpc.response.status_code": "DEADLINE_EXCEEDED", + "rpc.system": "grpc", + }, + want: "DEADLINE_EXCEEDED", + }, + { + name: "rpc.system.name=grpc still accepted when legacy rpc.system disagrees", + attrs: map[string]any{ + "rpc.response.status_code": "OK", + "rpc.system.name": "grpc", + "rpc.system": "jsonrpc", + }, + want: "OK", + }, + { + name: "non-gRPC rpc.system.name rejects rpc.response.status_code", + attrs: map[string]any{ + "rpc.response.status_code": "-32602", + "rpc.system.name": "jsonrpc", + }, + }, + { + // New rpc.system.name takes precedence over legacy rpc.system: if the + // SDK set rpc.system.name explicitly, ignore a stale rpc.system=grpc. + name: "explicit non-grpc rpc.system.name overrides legacy rpc.system=grpc", + attrs: map[string]any{ + "rpc.response.status_code": "-32602", + "rpc.system.name": "jsonrpc", + "rpc.system": "grpc", + }, + }, + { + name: "int64 rpc.response.status_code is formatted as string", + attrs: map[string]any{ + "rpc.response.status_code": int64(7), + "rpc.system.name": "grpc", + }, + want: "7", + }, + } { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, LookupString(r, newTestAccessor(tt.attrs), ConceptGRPCStatusCode)) + }) + } + + t.Run("LookupInt64 returns typed value through conditional fallback", func(t *testing.T) { + accessor := newTestAccessor(map[string]any{ + "rpc.response.status_code": int64(7), + "rpc.system.name": "grpc", + }) + v, ok := LookupInt64(r, accessor, ConceptGRPCStatusCode) + assert.True(t, ok) + assert.Equal(t, int64(7), v) + }) +} + func TestStringMapAccessor(t *testing.T) { t.Run("returns value for key", func(t *testing.T) { accessor := NewStringMapAccessor(map[string]string{"key": "value", "empty": ""}) diff --git a/pkg/trace/semantics/mappings.json b/pkg/trace/semantics/mappings.json index 4d457d4d10df..90cf5dd0e78d 100644 --- a/pkg/trace/semantics/mappings.json +++ b/pkg/trace/semantics/mappings.json @@ -150,7 +150,11 @@ {"name": "rpc.grpc.status.code", "provider": "otel", "type": "string"}, {"name": "rpc.grpc.status.code", "provider": "otel", "type": "int64"}, {"name": "grpc.status.code", "provider": "datadog", "type": "string"}, - {"name": "grpc.status.code", "provider": "datadog", "type": "int64"} + {"name": "grpc.status.code", "provider": "datadog", "type": "int64"}, + {"name": "rpc.response.status_code", "provider": "otel", "version": "1.39.0", "type": "string", "when": [{"attribute": "rpc.system.name", "eq": "grpc"}]}, + {"name": "rpc.response.status_code", "provider": "otel", "version": "1.39.0", "type": "int64", "when": [{"attribute": "rpc.system.name", "eq": "grpc"}]}, + {"name": "rpc.response.status_code", "provider": "otel", "version": "1.39.0", "type": "string", "when": [{"attribute": "rpc.system", "eq": "grpc"}, {"attribute": "rpc.system.name", "present": false}]}, + {"name": "rpc.response.status_code", "provider": "otel", "version": "1.39.0", "type": "int64", "when": [{"attribute": "rpc.system", "eq": "grpc"}, {"attribute": "rpc.system.name", "present": false}]} ] }, "span.kind": { @@ -449,4 +453,4 @@ ] } } -} \ No newline at end of file +} diff --git a/pkg/trace/semantics/semantics.go b/pkg/trace/semantics/semantics.go index 42611205ce5e..608dc1b9aed3 100644 --- a/pkg/trace/semantics/semantics.go +++ b/pkg/trace/semantics/semantics.go @@ -9,7 +9,9 @@ // Future work (OTel semantic convention updates): // - rpc.service is deprecated; the replacement is to include it as part of rpc.method, // so the fallback system alone cannot extract the concept value. Needs different handling. -// - rpc.system is superseded by rpc.system.name; add rpc.system.name as fallback/canonical. +// - rpc.system is superseded by rpc.system.name (OTel semconv v1.39.0); add rpc.system.name +// as a fallback or promote it to canonical. Note: this affects getOTelOperationNameV2, +// so a release note is required. // - db.system is deprecated in favor of db.system.name; add db.system.name to mappings. package semantics @@ -128,12 +130,20 @@ const ( ConceptDDAPMMode Concept = "_dd.apm_mode" ) +// Condition describes a predicate that must match before a fallback tag can be used. +type Condition struct { + Attribute string `json:"attribute,omitempty"` + Present *bool `json:"present,omitempty"` + Eq *string `json:"eq,omitempty"` +} + // TagInfo contains metadata about a semantic attribute and its location. type TagInfo struct { - Name string `json:"name"` - Provider Provider `json:"provider"` - Version string `json:"version,omitempty"` - Type ValueType `json:"type,omitempty"` + Name string `json:"name"` + Provider Provider `json:"provider"` + Version string `json:"version,omitempty"` + Type ValueType `json:"type,omitempty"` + When []Condition `json:"when,omitempty"` } // ConceptMapping represents a semantic concept and its equivalent attributes. diff --git a/pkg/trace/transform/transform.go b/pkg/trace/transform/transform.go index 23cb2062b6b3..aba73d87c35c 100644 --- a/pkg/trace/transform/transform.go +++ b/pkg/trace/transform/transform.go @@ -91,7 +91,7 @@ func otelSpanToDDSpanMinimal( if code, ok := semantics.LookupInt64(reg, spanAccessor, semantics.ConceptHTTPStatusCode); ok && code >= 0 { ddspan.Metrics[traceutil.TagStatusCode] = float64(code) } - if grpcCode := getOTelGRPCStatusCode(reg, spanAccessor, sattr, rattr); grpcCode != "" { + if grpcCode := semantics.LookupString(reg, spanAccessor, semantics.ConceptGRPCStatusCode); grpcCode != "" { ddspan.Meta[string(semantics.ConceptGRPCStatusCode)] = grpcCode } if isTopLevel { @@ -113,23 +113,6 @@ func otelSpanToDDSpanMinimal( return ddspan } -func getOTelGRPCStatusCode(reg semantics.Registry, spanAccessor semantics.Accessor, sattr, rattr pcommon.Map) string { - if grpcCode := semantics.LookupString(reg, spanAccessor, semantics.ConceptGRPCStatusCode); grpcCode != "" { - return grpcCode - } - // reject non-gRPC systems so we don't write a JSON-RPC error code or Connect error. - // Check rpc.system.name (current semconv) first, then the legacy rpc.system as a - // fallback: SDKs may migrate rpc.response.status_code without simultaneously updating - // the system identifier, and the legacy attribute is still present in many fixtures. - if GetOTelAttrFromEitherMap(sattr, rattr, false, "rpc.system.name", "rpc.system") != "grpc" { - return "" - } - // Newer OTel gRPC SDKs emit status under rpc.response.status_code instead of - // rpc.grpc.status_code (the legacy key already covered by the registry above), - // so without this read we'd drop gRPC status from spans produced by those SDKs. - return GetOTelAttrFromEitherMap(sattr, rattr, false, "rpc.response.status_code") -} - // OtelSpanToDDSpanMinimal converts an OTel span to a DD span with only the minimal fields // needed for APM stats calculation. Only use with OTLPTracesToConcentratorInputs. func OtelSpanToDDSpanMinimal(