Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

parametric: remove http_headers from otel start span creation #3478

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+47 −460
Open

parametric: remove http_headers from otel start span creation #3478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a8dae5d
parametric: remove http_headers arg from otel start span
mabdinur Nov 18, 2024
1bdbd9b
fix apps
mabdinur Nov 18, 2024
91b5690
Merge branch 'main' into munir/remove-htpp-args-parame
mabdinur Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 6 additions & 17 deletions tests/parametric/test_otel_api_interoperability.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,35 +227,24 @@ 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this code useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this span ("dd_root") is linked to the otel span below. In this test I removed the http_headers argument from the Links object. Instead I use on the parent_id to generate a span link. This better aligns with the Otel Start Span API.


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

root = find_root_span(trace)
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
Expand Down
122 changes: 9 additions & 113 deletions tests/parametric/test_otel_span_methods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
Expand Down
17 changes: 0 additions & 17 deletions tests/parametric/test_otel_span_with_w3c.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,3 @@ def test_otel_start_span_with_w3c(self, test_agent, test_library):
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):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you review the manifest files? For example there is an entry for "Test_Otel_Span_With_W3c" in python.yml

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
61 changes: 1 addition & 60 deletions utils/build/docker/dotnet/parametric/Endpoints/ApmTestApiOtel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,34 +47,6 @@ private static async Task<string> 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<string[][]>(),
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)
{
Expand Down Expand Up @@ -132,38 +104,7 @@ private static async Task<string> OtelStartSpan(HttpRequest request)
tags = ToActivityTagsCollection(((Newtonsoft.Json.Linq.JObject?)spanLink["attributes"])?.ToObject<Dictionary<string, object>>());
}

ActivityContext contextToLink = new ActivityContext();

if (parentSpanLink > 0)
{
contextToLink = FindActivity(parentSpanLink).Context;
}
else
{
var httpHeadersToken = (JArray)spanLink["http_headers"]!;

var manualExtractedContext = _spanContextExtractor.Extract(
httpHeadersToken.ToObject<string[][]>(),
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));
}
}
Expand Down
3 changes: 0 additions & 3 deletions utils/build/docker/golang/parametric/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand All @@ -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"`
}

Expand Down Expand Up @@ -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"`
}

Expand Down
54 changes: 1 addition & 53 deletions utils/build/docker/golang/parametric/otel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -59,61 +58,10 @@ 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(),
}))
}
otelOpts = append(otelOpts, otel_trace.WithLinks(otel_trace.Link{SpanContext: s.otelSpans[link.ParentId].span.SpanContext(), Attributes: link.Attributes.ConvertToAttributesStringified()}))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not care about these conditions anymore when adding span links?
if p := link.ParentId; p != 0
if _, ok := s.otelSpans[p]; ok

}
}

Expand Down
Loading