From d1e67f6d53ec96e50b3d5468597792e3dc5b1120 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Wed, 20 Nov 2024 10:09:21 -0500 Subject: [PATCH] parametric: remove http_headers from otel start span creation (#3478) --- manifests/dotnet.yml | 2 - manifests/php.yml | 2 - manifests/python.yml | 2 - manifests/ruby.yml | 2 - .../test_otel_api_interoperability.py | 23 +--- tests/parametric/test_otel_span_methods.py | 122 ++---------------- tests/parametric/test_otel_span_with_w3c.py | 62 --------- .../parametric/Endpoints/ApmTestApiOtel.cs | 61 +-------- .../build/docker/golang/parametric/helpers.go | 3 - utils/build/docker/golang/parametric/otel.go | 56 +------- .../controller/OpenTelemetryController.java | 50 +------ .../trace/opentelemetry/dto/SpanLink.java | 3 +- .../opentelemetry/dto/StartSpanArgs.java | 1 - .../build/docker/nodejs/parametric/server.js | 23 +--- utils/build/docker/php/parametric/server.php | 55 +------- .../parametric/apm_test_client/server.py | 42 +----- utils/build/docker/ruby/parametric/server.rb | 29 ++--- utils/parametric/_library_client.py | 24 +--- 18 files changed, 50 insertions(+), 512 deletions(-) delete mode 100644 tests/parametric/test_otel_span_with_w3c.py diff --git a/manifests/dotnet.yml b/manifests/dotnet.yml index 13e8d85c37..c9c07d622b 100644 --- a/manifests/dotnet.yml +++ b/manifests/dotnet.yml @@ -387,8 +387,6 @@ tests/: Test_Otel_Env_Vars: v2.53.0 test_otel_span_with_baggage.py: Test_Otel_Span_With_Baggage: missing_feature - test_otel_span_with_w3c.py: - Test_Otel_Span_With_W3c: v2.42.0 test_otel_tracer.py: Test_Otel_Tracer: v2.8.0 test_parametric_endpoints.py: diff --git a/manifests/php.yml b/manifests/php.yml index c20c886d37..7e3b9670c3 100644 --- a/manifests/php.yml +++ b/manifests/php.yml @@ -325,8 +325,6 @@ tests/: Test_Otel_Span_Methods: v0.94.0 test_otel_span_with_baggage.py: Test_Otel_Span_With_Baggage: missing_feature - test_otel_span_with_w3c.py: - Test_Otel_Span_With_W3c: v0.94.0 test_otel_tracer.py: Test_Otel_Tracer: v0.94.0 test_parametric_endpoints.py: diff --git a/manifests/python.yml b/manifests/python.yml index 864f75dc8e..473e64f1fd 100644 --- a/manifests/python.yml +++ b/manifests/python.yml @@ -771,8 +771,6 @@ tests/: Test_Otel_Span_Methods: v2.8.0 test_otel_span_with_baggage.py: Test_Otel_Span_With_Baggage: missing_feature - test_otel_span_with_w3c.py: - Test_Otel_Span_With_W3c: v2.8.0 test_otel_tracer.py: Test_Otel_Tracer: v2.8.0 test_parametric_endpoints.py: diff --git a/manifests/ruby.yml b/manifests/ruby.yml index 3a89825a9a..26b7e80899 100644 --- a/manifests/ruby.yml +++ b/manifests/ruby.yml @@ -393,8 +393,6 @@ tests/: Test_Otel_Span_Methods: v1.17.0 test_otel_span_with_baggage.py: Test_Otel_Span_With_Baggage: missing_feature - test_otel_span_with_w3c.py: - Test_Otel_Span_With_W3c: v1.17.0 test_parametric_endpoints.py: Test_Parametric_DDSpan_Set_Resource: missing_feature (set_resource endpoint is not supported) Test_Parametric_DDTrace_Baggage: missing_feature (baggage is not supported) diff --git a/tests/parametric/test_otel_api_interoperability.py b/tests/parametric/test_otel_api_interoperability.py index 75d65614f6..8211e89f1a 100644 --- a/tests/parametric/test_otel_api_interoperability.py +++ b/tests/parametric/test_otel_api_interoperability.py @@ -227,21 +227,17 @@ def test_span_links_add(self, test_agent, test_library): - Test that links can be added with the Datadog API on a span created with the OTel API """ with test_library: - with test_library.otel_start_span("otel.span") as otel_span: - current_span = test_library.current_span() + with test_library.start_span("dd_root") as dd_span: + pass + with test_library.otel_start_span("otel_root") as otel_span: + current_span = test_library.current_span() current_span.add_link( - parent_id=0, - attributes=TEST_ATTRIBUTES, - http_headers=[ - ("traceparent", f"00-{TEST_TRACE_ID}-{TEST_SPAN_ID}-01"), - ("tracestate", TEST_TRACESTATE), - ], + parent_id=dd_span.span_id, attributes=TEST_ATTRIBUTES, ) - otel_span.end_span() - traces = test_agent.wait_for_num_traces(1, sort_by_start=False) + traces = test_agent.wait_for_num_traces(2, sort_by_start=False) trace = find_trace(traces, otel_span.trace_id) assert len(trace) == 1 @@ -249,13 +245,6 @@ def test_span_links_add(self, test_agent, test_library): span_links = retrieve_span_links(root) assert len(span_links) == 1 - link = span_links[0] - assert link["trace_id"] == TEST_TRACE_ID_LOW - assert link["trace_id_high"] == TEST_TRACE_ID_HIGH - assert link["span_id"] == TEST_SPAN_ID_INT - assert "t.dm:-0" in link["tracestate"] - assert link["attributes"]["arg1"] == "val1" - def test_concurrent_traces_in_order(self, test_agent, test_library): """ - Basic concurrent traces and spans diff --git a/tests/parametric/test_otel_span_methods.py b/tests/parametric/test_otel_span_methods.py index 7792500a36..561982c661 100644 --- a/tests/parametric/test_otel_span_methods.py +++ b/tests/parametric/test_otel_span_methods.py @@ -512,107 +512,6 @@ def test_otel_span_started_with_link_from_another_span(self, test_agent, test_li root_tid = root["meta"].get("_dd.p.tid", "0") assert link.get("trace_id_high") == int(root_tid, 16) - @missing_feature(context.library < "dotnet@2.53.0", reason="Will be released in 2.53.0") - @missing_feature(context.library < "java@1.26.0", reason="Implemented in 1.26.0") - @missing_feature(context.library < "nodejs@5.3.0", reason="Implemented in 3.48.0, 4.27.0, and 5.3.0") - @missing_feature(context.library < "golang@1.61.0", reason="Implemented in 1.61.0") - @missing_feature(context.library < "ruby@2.0.0", reason="Not implemented") - @missing_feature(context.library == "php", reason="Implemented in 0.97.0 but link.flags are not natively supported") - def test_otel_span_started_with_link_from_datadog_headers(self, test_agent, test_library): - """Properly inject datadog distributed tracing information into span links. - """ - with test_library: - with test_library.otel_start_span( - "root", - links=[ - Link( - http_headers=[ - ["x-datadog-trace-id", "1234567890"], - ["x-datadog-parent-id", "9876543210"], - ["x-datadog-sampling-priority", "2"], - ["x-datadog-origin", "synthetics"], - ["x-datadog-tags", "_dd.p.dm=-4,_dd.p.tid=0000000000000010"], - ], - attributes={"foo": "bar"}, - ) - ], - ) as span: - span.end_span() - - traces = test_agent.wait_for_num_traces(1) - trace = find_trace(traces, span.trace_id) - span = find_span(trace, span.span_id) - span_links = retrieve_span_links(span) - assert span_links is not None - assert len(span_links) == 1 - - link = span_links[0] - assert link.get("span_id") == 9876543210 - assert link.get("trace_id") == 1234567890 - assert link.get("trace_id_high") == 16 - - # Tracestate is not required, but if it is present, it must be valid - if link.get("tracestate"): - tracestateArr = link["tracestate"].split(",") - assert len(tracestateArr) == 1 and tracestateArr[0].startswith("dd=") - tracestateDD = tracestateArr[0][3:].split(";") - assert "o:synthetics" in tracestateDD - assert "s:2" in tracestateDD - assert "t.dm:-4" in tracestateDD - # Sampled flag should be set to match the existing tracestate - assert link.get("flags") == 1 | TRACECONTEXT_FLAGS_SET - - @missing_feature(context.library < "dotnet@2.53.0", reason="Will be released in 2.53.0") - @missing_feature(context.library < "java@1.28.0", reason="Implemented in 1.28.0") - @missing_feature(context.library < "nodejs@5.3.0", reason="Implemented in 3.48.0, 4.27.0, and 5.3.0") - @missing_feature(context.library < "golang@1.61.0", reason="Implemented in 1.61.0") - @bug(context.library < "ruby@2.3.1-dev", reason="APMRP-360") - @missing_feature(context.library == "php", reason="Implemented in 0.97.0 but link.flags are not natively supported") - def test_otel_span_started_with_link_from_w3c_headers(self, test_agent, test_library): - """Properly inject w3c distributed tracing information into span links. - This mostly tests that the injected tracestate and flags are accurate. - """ - with test_library: - with test_library.otel_start_span( - "root", - links=[ - Link( - http_headers=[ - ["traceparent", "00-12345678901234567890123456789012-1234567890123456-01"], - ["tracestate", "foo=1,dd=t.dm:-4;s:2,bar=baz"], - ] - ) - ], - ) as span: - span.end_span() - - traces = test_agent.wait_for_num_traces(1) - trace = find_trace(traces, span.trace_id) - span = find_span(trace, span.span_id) - span_links = retrieve_span_links(span) - assert span_links is not None - assert len(span_links) == 1 - - link = span_links[0] - assert link.get("span_id") == 1311768467284833366 - assert link.get("trace_id") == 8687463697196027922 - assert link.get("trace_id_high") == 1311768467284833366 - - assert link.get("tracestate") is not None - tracestateArr = link["tracestate"].split(",") - dd_member = next(iter([x for x in tracestateArr if x.startswith("dd=")]), None) - foo_member = next(iter([x for x in tracestateArr if x.startswith("foo=")]), None) - bar_member = next(iter([x for x in tracestateArr if x.startswith("bar=")]), None) - # ruby removes the dd member from the tracestate while python does not - if dd_member: - assert "s:2" in dd_member - assert "t.dm:-4" in dd_member - assert foo_member == "foo=1" - assert bar_member == "bar=baz" - - assert (link.get("flags") == 1 | TRACECONTEXT_FLAGS_SET) or (link.get("flags") == TRACECONTEXT_FLAGS_SET) - assert link.get("attributes") is None or len(link.get("attributes")) == 0 - @missing_feature(context.library < "dotnet@2.53.0", reason="Will be released in 2.53.0") @missing_feature(context.library < "java@1.26.0", reason="Implemented in 1.26.0") @missing_feature(context.library == "golang", reason="Not implemented") @@ -623,26 +522,23 @@ def test_otel_span_link_attribute_handling(self, test_agent, test_library): """Test that span links implementations correctly handle attributes according to spec. """ with test_library: + with test_library.otel_start_span("span1") as s1: + s1.end_span() + with test_library.otel_start_span( "root", links=[ Link( - http_headers=[ - ["x-datadog-trace-id", "1234567890"], - ["x-datadog-parent-id", "9876543210"], - ["x-datadog-sampling-priority", "2"], - ["x-datadog-origin", "synthetics"], - ["x-datadog-tags", "_dd.p.dm=-4,_dd.p.tid=0000000000000010"], - ], + parent_id=s1.span_id, attributes={"foo": "bar", "array": ["a", "b", "c"], "bools": [True, False], "nested": [1, 2]}, ) ], - ) as span: - span.end_span() + ) as s2: + s2.end_span() - traces = test_agent.wait_for_num_traces(1) - trace = find_trace(traces, span.trace_id) - span = find_span(trace, span.span_id) + traces = test_agent.wait_for_num_traces(2) + trace = find_trace(traces, s2.trace_id) + span = find_span(trace, s2.span_id) span_links = retrieve_span_links(span) assert span_links is not None assert len(span_links) == 1 diff --git a/tests/parametric/test_otel_span_with_w3c.py b/tests/parametric/test_otel_span_with_w3c.py deleted file mode 100644 index b81d9adcbe..0000000000 --- a/tests/parametric/test_otel_span_with_w3c.py +++ /dev/null @@ -1,62 +0,0 @@ -import time - -import pytest - -from utils.dd_constants import SpanKind -from utils.parametric.spec.trace import SAMPLING_PRIORITY_KEY, ORIGIN -from utils.parametric.spec.trace import find_only_span -from utils import missing_feature, irrelevant, context, scenarios, features - -# this global mark applies to all tests in this file. -# DD_TRACE_OTEL_ENABLED=true is required in some tracers (.NET, Python?) -# CORECLR_ENABLE_PROFILING=1 is required in .NET to enable auto-instrumentation -pytestmark = pytest.mark.parametrize( - "library_env", [{"DD_TRACE_OTEL_ENABLED": "true", "CORECLR_ENABLE_PROFILING": "1"}], -) - - -@scenarios.parametric -@features.open_tracing_api -class Test_Otel_Span_With_W3c: - @irrelevant(context.library == "cpp", reason="library does not implement OpenTelemetry") - @missing_feature(context.library == "python", reason="Not implemented") - @missing_feature(context.library <= "java@1.23.0", reason="OTel resource naming implemented in 1.24.0") - @missing_feature(context.library == "nodejs", reason="Not implemented") - def test_otel_start_span_with_w3c(self, test_agent, test_library): - """ - - Start/end a span with start and end options - """ - with test_library: - duration_us = int(2 * 1_000_000) - start_time = int(time.time()) - with test_library.otel_start_span( - "operation", - span_kind=SpanKind.PRODUCER, - timestamp=start_time, - attributes={"start_attr_key": "start_attr_val"}, - ) as parent: - parent.end_span(timestamp=start_time + duration_us) - duration_ns = int(duration_us * 1_000) # OTEL durations are microseconds, must convert to ns for dd - - root_span = find_only_span(test_agent.wait_for_num_traces(1)) - assert root_span["name"] == "producer" - assert root_span["resource"] == "operation" - assert root_span["meta"]["start_attr_key"] == "start_attr_val" - assert root_span["duration"] == duration_ns - - @irrelevant(context.library == "cpp", reason="library does not implement OpenTelemetry") - def test_otel_span_with_w3c_headers(self, test_agent, test_library): - with test_library: - with test_library.otel_start_span( - name="name", http_headers=[["traceparent", "00-00000000000000001111111111111111-2222222222222222-01"]], - ) as span: - context = span.span_context() - assert context.get("trace_flags") == "01" - assert context.get("trace_id") == "00000000000000001111111111111111" - span.end_span() - - span = find_only_span(test_agent.wait_for_num_traces(1)) - assert span.get("trace_id") == 1229782938247303441 - assert span.get("parent_id") == 2459565876494606882 - assert span["metrics"].get(SAMPLING_PRIORITY_KEY) == 1 - assert span["meta"].get(ORIGIN) is None diff --git a/utils/build/docker/dotnet/parametric/Endpoints/ApmTestApiOtel.cs b/utils/build/docker/dotnet/parametric/Endpoints/ApmTestApiOtel.cs index c7b3a033bc..4ffaf6ab90 100644 --- a/utils/build/docker/dotnet/parametric/Endpoints/ApmTestApiOtel.cs +++ b/utils/build/docker/dotnet/parametric/Endpoints/ApmTestApiOtel.cs @@ -47,34 +47,6 @@ private static async Task OtelStartSpan(HttpRequest request) } } - // try extracting parent context from headers (remote parent) - if (requestBodyObject.TryGetValue("http_headers", out var headersList)) - { - var manualExtractedContext = _spanContextExtractor.Extract( - ((Newtonsoft.Json.Linq.JArray)headersList).ToObject(), - getter: GetHeaderValues!); - - _logger?.LogInformation("Extracted SpanContext: {ExtractedContext}", manualExtractedContext); - - if (manualExtractedContext is not null) - { - // This implementation is .NET v3 specific, and assumes that the span returned by StartActive is a DuckType - var extractedContext = manualExtractedContext.GetType() - .GetProperty("Instance", BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public) - ?.GetValue(manualExtractedContext); - var parentTraceId = ActivityTraceId.CreateFromString(RawTraceId.GetValue(extractedContext) as string); - var parentSpanId = ActivitySpanId.CreateFromString(RawSpanId.GetValue(extractedContext) as string); - var flags = (SamplingPriority.GetValue(extractedContext) as int?) > 0 ? ActivityTraceFlags.Recorded : ActivityTraceFlags.None; - - remoteParentContext = new ActivityContext( - parentTraceId, - parentSpanId, - flags, - AdditionalW3CTraceState.GetValue(extractedContext) as string, - isRemote: true); - } - } - // sanity check that we didn't receive both a local and remote parent if (localParentContext != null && remoteParentContext != null) { @@ -132,38 +104,7 @@ private static async Task OtelStartSpan(HttpRequest request) tags = ToActivityTagsCollection(((Newtonsoft.Json.Linq.JObject?)spanLink["attributes"])?.ToObject>()); } - ActivityContext contextToLink = new ActivityContext(); - - if (parentSpanLink > 0) - { - contextToLink = FindActivity(parentSpanLink).Context; - } - else - { - var httpHeadersToken = (JArray)spanLink["http_headers"]!; - - var manualExtractedContext = _spanContextExtractor.Extract( - httpHeadersToken.ToObject(), - getter: GetHeaderValues!); - - // This implementation is .NET v3 specific, and assumes that the span returned by StartActive is a DuckType - var extractedContext = manualExtractedContext?.GetType() - .GetProperty("Instance", BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public) - ?.GetValue(manualExtractedContext); - var parentTraceId = ActivityTraceId.CreateFromString(RawTraceId.GetValue(extractedContext) as string); - var parentSpanId = ActivitySpanId.CreateFromString(RawSpanId.GetValue(extractedContext) as string); - var flags = (SamplingPriority.GetValue(extractedContext) as int?) > 0 ? ActivityTraceFlags.Recorded : ActivityTraceFlags.None; - var datadogHeadersTracestate = W3CTraceContextCreateTraceStateHeader.Invoke(null, new object[] { extractedContext! }); - var tracestate = (string?)httpHeadersToken[1][0] == "tracestate" ? (string?)httpHeadersToken[1][1] : datadogHeadersTracestate; - - contextToLink = new ActivityContext( - parentTraceId, - parentSpanId, - flags, - (string?)tracestate, - isRemote: true); - } - + ActivityContext contextToLink = FindActivity(parentSpanLink).Context; linksList.Add(new ActivityLink(contextToLink, tags)); } } diff --git a/utils/build/docker/golang/parametric/helpers.go b/utils/build/docker/golang/parametric/helpers.go index 05f110b3d7..fbcfc0884e 100644 --- a/utils/build/docker/golang/parametric/helpers.go +++ b/utils/build/docker/golang/parametric/helpers.go @@ -19,7 +19,6 @@ type StartSpanArgs struct { Resource string `json:"resource,omitempty"` Type string `json:"type,omitempty"` Origin string `json:"origin,omitempty"` - HttpHeaders []Tuple `json:"http_headers,omitempty"` SpanTags []Tuple `json:"span_tags,omitempty"` SpanLinks []SpanLink `json:"span_links,omitempty"` } @@ -28,7 +27,6 @@ type Tuple []string type SpanLink struct { ParentId uint64 `json:"parent_id"` - HttpHeaders []Tuple `json:"http_headers"` Attributes AttributeKeyVals `json:"attributes,omitempty"` } @@ -98,7 +96,6 @@ type OtelStartSpanArgs struct { Type string `json:"type"` Timestamp int64 `json:"timestamp"` SpanLinks []SpanLink `json:"links"` - HttpHeaders []Tuple `json:"http_headers"` Attributes AttributeKeyVals `json:"attributes"` } diff --git a/utils/build/docker/golang/parametric/otel.go b/utils/build/docker/golang/parametric/otel.go index 6fed45fa9a..a87f714e37 100644 --- a/utils/build/docker/golang/parametric/otel.go +++ b/utils/build/docker/golang/parametric/otel.go @@ -12,7 +12,6 @@ import ( "go.opentelemetry.io/otel/codes" otel_trace "go.opentelemetry.io/otel/trace" - "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" ddotel "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/opentelemetry" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" ) @@ -59,60 +58,13 @@ func (s *apmClientServer) OtelStartSpan(args OtelStartSpanArgs) (OtelStartSpanRe if a := args.Attributes; len(a) > 0 { otelOpts = append(otelOpts, otel_trace.WithAttributes(a.ConvertToAttributes()...)) } - if h := args.HttpHeaders; len(h) > 0 { - headers := map[string]string{} - for _, headerTuple := range h { - k := headerTuple.Key() - v := headerTuple.Value() - if k != "" && v != "" { - headers[k] = v - } - } - sctx, err := tracer.NewPropagator(nil).Extract(tracer.TextMapCarrier(headers)) - if err != nil { - fmt.Println("failed to extract span context from headers:", err, args.HttpHeaders) - } else { - ddOpts = append(ddOpts, tracer.ChildOf(sctx)) - } - } if links := args.SpanLinks; links != nil { for _, link := range links { - if p := link.ParentId; p != 0 { - if _, ok := s.otelSpans[p]; ok { - otelOpts = append(otelOpts, otel_trace.WithLinks(otel_trace.Link{SpanContext: s.otelSpans[p].span.SpanContext(), Attributes: link.Attributes.ConvertToAttributesStringified()})) - } - } else if h := link.HttpHeaders; h != nil { - headers := map[string]string{} - for _, headerTuple := range h { - k := headerTuple.Key() - v := headerTuple.Value() - if k != "" && v != "" { - headers[k] = v - } - } - extractedContext, _ := tracer.NewPropagator(nil).Extract(tracer.TextMapCarrier(headers)) - state, _ := otel_trace.ParseTraceState(headers["tracestate"]) - - var traceID otel_trace.TraceID - var spanID otel_trace.SpanID - if w3cCtx, ok := extractedContext.(ddtrace.SpanContextW3C); ok { - traceID = w3cCtx.TraceID128Bytes() - } else { - fmt.Printf("Non-W3C context found in span, unable to get full 128 bit trace id") - uint64ToByte(extractedContext.TraceID(), traceID[:]) - } - uint64ToByte(extractedContext.SpanID(), spanID[:]) - config := otel_trace.SpanContextConfig{ - TraceID: traceID, - SpanID: spanID, - TraceState: state, - } - var newCtx = otel_trace.NewSpanContext(config) - otelOpts = append(otelOpts, otel_trace.WithLinks(otel_trace.Link{ - SpanContext: newCtx, - Attributes: link.Attributes.ConvertToAttributesStringified(), - })) + if pSpan, ok := s.otelSpans[link.ParentId]; ok { + otelOpts = append(otelOpts, otel_trace.WithLinks(otel_trace.Link{SpanContext: pSpan.span.SpanContext(), Attributes: link.Attributes.ConvertToAttributesStringified()})) + } else { + return OtelStartSpanReturn{}, fmt.Errorf("OtelStartSpan call failed. Failed to generate a link to span with id=%d", link.ParentId) } } } diff --git a/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/controller/OpenTelemetryController.java b/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/controller/OpenTelemetryController.java index 23390586c8..a69a0c1369 100644 --- a/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/controller/OpenTelemetryController.java +++ b/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/controller/OpenTelemetryController.java @@ -171,15 +171,6 @@ public StartSpanResult startSpan(@RequestBody StartSpanArgs args) { builder.setParent(contextWithParentSpan); } } - // Check HTTP headers to extract propagated context from - if (args.httpHeaders() != null && !args.httpHeaders().isEmpty()) { - Context extractedContext = this.propagator.extract( - Context.root(), - args.httpHeaders(), - HeadersTextMapGetter.INSTANCE - ); - builder.setParent(extractedContext); - } // Add other span information builder.setSpanKind(parseSpanKindNumber(args.spanKind())); if (args.timestamp() > 0) { @@ -187,22 +178,12 @@ public StartSpanResult startSpan(@RequestBody StartSpanArgs args) { } if (args.links() != null && !args.links().isEmpty()) { for (SpanLink spanLink : args.links()) { - SpanContext spanContext = null; LOGGER.debug("Span link: {}", spanLink); - if (spanLink.parentId() > 0) { - Span span = getSpan(spanLink.parentId()); - if (span == null) { - return StartSpanResult.error(); - } - spanContext = span.getSpanContext(); - } else if (spanLink.httpHeaders() != null && !spanLink.httpHeaders().isEmpty()) { - Context extractedContext = this.propagator.extract( - Context.root(), - spanLink.httpHeaders(), - HeadersTextMapGetter.INSTANCE - ); - spanContext = Span.fromContext(extractedContext).getSpanContext(); + Span span = getSpan(spanLink.parentId()); + if (span == null) { + return StartSpanResult.error(); } + SpanContext spanContext = span.getSpanContext(); if (spanContext != null && spanContext.isValid()) { LOGGER.debug("Adding links from context {}", spanContext); builder.addLink(spanContext, parseAttributes(spanLink.attributes())); @@ -342,27 +323,4 @@ private Span getSpan(long spanId) { } return span; } - - private static class HeadersTextMapGetter implements TextMapGetter> { - private static final HeadersTextMapGetter INSTANCE = new HeadersTextMapGetter(); - - @Override - public Iterable keys(List headers) { - return headers.stream() - .map(KeyValue::key) - .toList(); - } - - @Override - public String get(List headers, String key) { - if (headers == null || headers.isEmpty()) { - return null; - } - return headers.stream() - .filter(kv -> Objects.equals(key, kv.key())) - .map(KeyValue::value) - .findFirst() - .orElse(null); - } - } } diff --git a/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/dto/SpanLink.java b/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/dto/SpanLink.java index 501e959ea0..281434fcf2 100644 --- a/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/dto/SpanLink.java +++ b/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/dto/SpanLink.java @@ -8,6 +8,5 @@ public record SpanLink( @JsonProperty("parent_id") long parentId, - Map attributes, - @JsonProperty("http_headers") @JsonDeserialize(using = KeyValueListDeserializer.class) List httpHeaders) { + Map attributes) { } diff --git a/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/dto/StartSpanArgs.java b/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/dto/StartSpanArgs.java index fbeb0282b5..64696cf837 100644 --- a/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/dto/StartSpanArgs.java +++ b/utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentelemetry/dto/StartSpanArgs.java @@ -11,7 +11,6 @@ public record StartSpanArgs( String name, @JsonProperty("span_kind") int spanKind, long timestamp, - @JsonProperty("http_headers") @JsonDeserialize(using = KeyValueListDeserializer.class) List httpHeaders, List links, Map attributes) { } diff --git a/utils/build/docker/nodejs/parametric/server.js b/utils/build/docker/nodejs/parametric/server.js index c461de5d44..4af103152a 100644 --- a/utils/build/docker/nodejs/parametric/server.js +++ b/utils/build/docker/nodejs/parametric/server.js @@ -159,14 +159,7 @@ app.post('/trace/otel/start_span', (req, res) => { const makeSpan = (parentContext) => { const links = (request.links || []).map(link => { - let spanContext; - if (link.parent_id && link.parent_id !== 0) { - spanContext = otelSpans[link.parent_id].spanContext(); - } else { - const linkHeaders = Object.fromEntries(link.http_headers.map(([k, v]) => [k.toLowerCase(), v])); - const extractedContext = tracer.extract('http_headers', linkHeaders) - spanContext = new OtelSpanContext(extractedContext) - } + let spanContext = otelSpans[link.parent_id].spanContext() return {context: spanContext, attributes: link.attributes} }); @@ -189,20 +182,6 @@ app.post('/trace/otel/start_span', (req, res) => { const parentContext = trace.setSpan(ROOT_CONTEXT, parentSpan) return makeSpan(parentContext) } - if (request.http_headers) { - const http_headers = request.http_headers || [] - // Node.js HTTP headers are automatically lower-cased, simulate that here. - const convertedHeaders = {} - for (const [ key, value ] of http_headers) { - convertedHeaders[key.toLowerCase()] = value - } - const extracted = tracer.extract('http_headers', convertedHeaders) - if (extracted) { - const parentSpan = trace.wrapSpanContext(new OtelSpanContext(extracted)) - const parentContext = trace.setSpan(ROOT_CONTEXT, parentSpan) - return makeSpan(parentContext) - } - } makeSpan() }); diff --git a/utils/build/docker/php/parametric/server.php b/utils/build/docker/php/parametric/server.php index b016b3dc3d..068f184f4c 100644 --- a/utils/build/docker/php/parametric/server.php +++ b/utils/build/docker/php/parametric/server.php @@ -215,23 +215,8 @@ function remappedSpanKind($spanKind) { $router->addRoute('POST', '/trace/span/add_link', new ClosureRequestHandler(function (Request $req) use (&$spans, &$closed_spans) { $span = $spans[arg($req, 'span_id')]; $parent_id = arg($req, 'parent_id'); - $httpHeaders = arg($req, 'http_headers'); - if (isset($spans[$parent_id]) || isset($closed_spans[$parent_id])) { - $link = ($spans[$parent_id] ?? $closed_spans[$parent_id])->getLink(); - $link->attributes += arg($req, "attributes") ?? []; - } elseif ($httpHeaders) { - $httpHeaders = array_merge(...array_map(fn($h) => [strtolower($h[0]) => $h[1]], $httpHeaders)); - $callback = function ($headername) use ($httpHeaders) { - return $httpHeaders[$headername] ?? null; - }; - $link = \DDTrace\SpanLink::fromHeaders($callback); - $link->attributes += arg($req, "attributes") ?? []; - } else { - $link = new \DDTrace\SpanLink(); - $link->spanId = arg($req, 'parent_id'); - $link->attributes = arg($req, 'attributes') ?? []; - } - + $link = ($spans[$parent_id] ?? $closed_spans[$parent_id])->getLink(); + $link->attributes = ($link->attributes ?? []) + (arg($req, "attributes") ?? []); $span->links[] = $link; return jsonResponse([]); })); @@ -286,7 +271,6 @@ function remappedSpanKind($spanKind) { $timestamp = arg($req, 'timestamp'); $spanKind = arg($req, 'span_kind'); $parentId = arg($req, 'parent_id'); - $httpHeaders = arg($req, 'http_headers'); $attributes = arg($req, 'attributes'); $tracer = (new TracerProvider())->getTracer('OpenTelemetry.PHPTestTracer'); @@ -324,39 +308,10 @@ function remappedSpanKind($spanKind) { if ($span_links = arg($req, 'links')) { foreach ($span_links as $span_link) { $span_link_attributes = isset($span_link["attributes"]) ? $span_link["attributes"] : []; - if ($span_link_parent_id = $span_link["parent_id"]) { - $span_context = $otelSpans[$span_link_parent_id]->getContext(); - $spanBuilder->addLink($span_context, $span_link_attributes); - } else if ($span_link_http_headers = $span_link["http_headers"]) { - $carrier = []; - foreach ($span_link_http_headers as $span_link_http_header) { - $carrier[$span_link_http_header[0]] = $span_link_http_header[1]; - } - - $callback = function ($headername) use ($carrier) { - return $carrier[$headername] ?? null; - }; - $headers_link = \DDTrace\SpanLink::fromHeaders($callback); - - $linkSpanContext = \OpenTelemetry\API\Trace\SpanContext::create( - $headers_link->traceId, - $headers_link->spanId, - \OpenTelemetry\API\Trace\TraceFlags::DEFAULT, // trace flags are not currently embedded into the native span link - new \OpenTelemetry\API\Trace\TraceState($headers_link->traceState ?? null), - ); - - $spanBuilder->addLink($linkSpanContext, $span_link_attributes); - } - } - } - - if ($httpHeaders) { - $carrier = []; - foreach ($httpHeaders as $headers) { - $carrier[$headers[0]] = $headers[1]; + $span_link_parent_id = $span_link["parent_id"]; + $span_context = $otelSpans[$span_link_parent_id]->getContext(); + $spanBuilder->addLink($span_context, $span_link_attributes); } - $remoteContext = TraceContextPropagator::getInstance()->extract($carrier); - $spanBuilder->setParent($remoteContext); } if ($attributes) { diff --git a/utils/build/docker/python/parametric/apm_test_client/server.py b/utils/build/docker/python/parametric/apm_test_client/server.py index 69ad62a139..a305689d0d 100644 --- a/utils/build/docker/python/parametric/apm_test_client/server.py +++ b/utils/build/docker/python/parametric/apm_test_client/server.py @@ -401,7 +401,6 @@ class OtelStartSpanArgs(BaseModel): type: str = "" links: List[Dict] = [] timestamp: int - http_headers: List[Tuple[str, str]] attributes: dict @@ -414,46 +413,11 @@ class OtelStartSpanReturn(BaseModel): def otel_start_span(args: OtelStartSpanArgs): otel_tracer = opentelemetry.trace.get_tracer(__name__) - if args.parent_id: - parent_span = otel_spans[args.parent_id] - elif args.http_headers: - headers = {k: v for k, v in args.http_headers} - ddcontext = HTTPPropagator.extract(headers) - parent_span = OtelNonRecordingSpan( - OtelSpanContext( - ddcontext.trace_id, - ddcontext.span_id, - True, - ( - TraceFlags.SAMPLED - if ddcontext.sampling_priority and ddcontext.sampling_priority > 0 - else TraceFlags.DEFAULT - ), - TraceState.from_header([ddcontext._tracestate]), - ) - ) - else: - parent_span = None - + parent_span = otel_spans.get(args.parent_id) links = [] for link in args.links: - parent_id = link.get("parent_id", 0) - if parent_id > 0: - span_context = otel_spans[parent_id].get_span_context() - else: - headers = {k: v for k, v in link["http_headers"]} - ddcontext = HTTPPropagator.extract(headers) - span_context = OtelSpanContext( - ddcontext.trace_id, - ddcontext.span_id, - True, - ( - TraceFlags.SAMPLED - if ddcontext.sampling_priority and ddcontext.sampling_priority > 0 - else TraceFlags.DEFAULT - ), - TraceState.from_header([ddcontext._tracestate]), - ) + parent_id = link["parent_id"] + span_context = otel_spans[parent_id].get_span_context() links.append(opentelemetry.trace.Link(span_context, link.get("attributes"))) # parametric tests expect span kind to be 0 for internal, 1 for server, 2 for client, .... diff --git a/utils/build/docker/ruby/parametric/server.rb b/utils/build/docker/ruby/parametric/server.rb index de54fb342e..eb75b15566 100644 --- a/utils/build/docker/ruby/parametric/server.rb +++ b/utils/build/docker/ruby/parametric/server.rb @@ -266,7 +266,7 @@ def to_json(*_args) end class OtelStartSpanArgs - attr_accessor :name, :parent_id, :span_kind, :service, :resource, :type, :links, :timestamp, :http_headers, + attr_accessor :name, :parent_id, :span_kind, :service, :resource, :type, :links, :timestamp, :attributes def initialize(params) @@ -278,7 +278,6 @@ def initialize(params) @type = params['type'] @links = params['links'] @timestamp = params['timestamp'] - @http_headers = params['http_headers'] @attributes = params['attributes'] end end @@ -509,18 +508,15 @@ def get_digest(span_id) end def parse_otel_link(link) - link_context = if !link['http_headers'].nil? && !link['http_headers'].size.nil? - digest = extract_http_headers(link['http_headers']) - digest_to_spancontext(digest) - elsif OTEL_SPANS.key?(link['parent_id']) - OTEL_SPANS[link['parent_id']].context - else - raise "Span id in #{link} not found in span list: #{OTEL_SPANS}" - end - OpenTelemetry::Trace::Link.new( - link_context, - link['attributes'] - ) + if OTEL_SPANS.key?(link['parent_id']) + link_context = OTEL_SPANS[link['parent_id']].context + OpenTelemetry::Trace::Link.new( + link_context, + link['attributes'] + ) + else + raise "Parent id in #{link} not found in span list: #{OTEL_SPANS}" + end end def digest_to_spancontext(digest) @@ -724,10 +720,7 @@ def handle_trace_otel_start_span(req, res) js = JSON.parse(req.body.read) args = OtelStartSpanArgs.new(js) - headers = args.http_headers.to_h - if !headers.empty? - parent_context = OpenTelemetry.propagation.extract(headers) - elsif args.parent_id != 0 + if args.parent_id != 0 parent_span = OTEL_SPANS[args.parent_id] parent_context = OpenTelemetry::Trace.context_with_span(parent_span) end diff --git a/utils/parametric/_library_client.py b/utils/parametric/_library_client.py index 5fbdff47fe..b8d9df4f8b 100644 --- a/utils/parametric/_library_client.py +++ b/utils/parametric/_library_client.py @@ -32,9 +32,8 @@ class SpanResponse(TypedDict): class Link(TypedDict): - parent_id: int # 0 to extract from headers + parent_id: int attributes: dict - http_headers: List[Tuple[str, str]] class APMLibraryClient: @@ -188,19 +187,10 @@ def span_set_error(self, span_id: int, typestr: str, message: str, stack: str) - json={"span_id": span_id, "type": typestr, "message": message, "stack": stack}, ) - def span_add_link( - self, span_id: int, parent_id: int, attributes: dict = None, http_headers: List[Tuple[str, str]] = None - ): - # Avoid using http_headers when creating a span link in the parametric apps - # Alternative endpoints will be provided to set these values. This will be documented in a future PR. + def span_add_link(self, span_id: int, parent_id: int, attributes: dict = None): self._session.post( self._url("/trace/span/add_link"), - json={ - "span_id": span_id, - "parent_id": parent_id, - "attributes": attributes or {}, - "http_headers": http_headers or [], - }, + json={"span_id": span_id, "parent_id": parent_id, "attributes": attributes or {},}, ) def span_get_baggage(self, span_id: int, key: str) -> str: @@ -236,7 +226,6 @@ def otel_trace_start_span( span_kind: SpanKind, parent_id: int, links: List[Link], - http_headers: List[Tuple[str, str]], attributes: dict = None, ) -> StartSpanResponse: resp = self._session.post( @@ -247,7 +236,6 @@ def otel_trace_start_span( "span_kind": span_kind.value, "parent_id": parent_id, "links": links, - "http_headers": http_headers, "attributes": attributes or {}, }, ).json() @@ -369,8 +357,8 @@ def remove_all_baggage(self): def set_error(self, typestr: str = "", message: str = "", stack: str = ""): self._client.span_set_error(self.span_id, typestr, message, stack) - def add_link(self, parent_id: int, attributes: dict = None, http_headers: List[Tuple[str, str]] = None): - self._client.span_add_link(self.span_id, parent_id, attributes, http_headers) + def add_link(self, parent_id: int, attributes: dict = None): + self._client.span_add_link(self.span_id, parent_id, attributes) def finish(self): self._client.finish_span(self.span_id) @@ -464,7 +452,6 @@ def otel_start_span( parent_id: int = 0, links: Optional[List[Link]] = None, attributes: dict = None, - http_headers: Optional[List[Tuple[str, str]]] = None, ) -> Generator[_TestOtelSpan, None, None]: resp = self._client.otel_trace_start_span( name=name, @@ -473,7 +460,6 @@ def otel_start_span( parent_id=parent_id, links=links if links is not None else [], attributes=attributes, - http_headers=http_headers if http_headers is not None else [], ) span = _TestOtelSpan(self._client, resp["span_id"], resp["trace_id"]) yield span