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.

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

Add span links capabilities to the OTel API #2451

Merged
merged 7 commits into from
Jan 10, 2024
Merged

Conversation

PROFeNoM
Copy link
Contributor

@PROFeNoM PROFeNoM commented Jan 4, 2024

Description

Add the span links to the OpenTelemetry API.

Please note the following points about converting Datadog spans to OpenTelemetry (OTel) spans and creating span links:

  • When converting a Datadog span to an OTel one, its current span links are also converted.
  • When creating a span using the OTel API, the corresponding \DDTrace\SpanLink instances are also created.

It is important to note that span links can be added to a span at any point during its lifetime using the Datadog (DD) API, but such links can only be added using the OTel API during the build phase. This limitation is not a problem with the DD API but with the OTel API. To address this, DD span links are actualized onto the OTel span when creating an immutable span from it (toSpanData()). This is the only way to access the links associated with an OTel span. Span Links removal is supported and actualized during the same step.

Span Links limits aren't supported, as such a concept doesn't exist in the DD API and is irrelevant.

A WeakMap is used to store information about previously instantiated span links that are associated with an OpenTelemetry (OTel) span. When a span link is added to an OTel span, a one-to-one relationship is established between the SpanLink and Link instances to ensure consistency. This is necessary because duplicate span links are allowed. I'd advise to look at the added Interoperability Tests to understand better what I mean :)

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@PROFeNoM PROFeNoM self-assigned this Jan 4, 2024
@PROFeNoM PROFeNoM requested a review from a team as a code owner January 4, 2024 14:55
@PROFeNoM PROFeNoM marked this pull request as draft January 4, 2024 16:07
@PROFeNoM PROFeNoM force-pushed the alex/otel/span-links branch from 1dfd0db to ab750d8 Compare January 5, 2024 15:27
@PROFeNoM PROFeNoM marked this pull request as ready for review January 8, 2024 10:10
@PROFeNoM PROFeNoM force-pushed the alex/otel/span-links branch from d8390fc to 2908881 Compare January 8, 2024 14:22
Co-authored-by: Bob Weinand <bob.weinand@datadoghq.com>
Copy link
Collaborator

@bwoebi bwoebi left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me :-)

@pr-commenter
Copy link

pr-commenter bot commented Jan 9, 2024

Benchmarks

Benchmark execution time: 2024-01-10 12:38:41

Comparing candidate commit in PR branch `` with baseline commit 77781a4 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 36 metrics, 54 unstable metrics.

@PROFeNoM PROFeNoM merged commit 4568c41 into master Jan 10, 2024
522 of 525 checks passed
@PROFeNoM PROFeNoM deleted the alex/otel/span-links branch January 10, 2024 13:39
@github-actions github-actions bot added this to the 0.97.0 milestone Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants