OpenTelemetry API

[PROFeNoM]

Description

Trace API

• Span Events and Span Links are missing

Metrics

• Not supported

Log (Appenders)

• Logs correlation is properly done when using Monolog log appender. Still, please review HandlerTest.php to confirm behavior with 128-bit trace identifiers (i.e., trace id: hex, span id: dec)

Context

• Spans created using DD's API are only converted to OTel spans when the OTel API is used and a context update is triggered (e.g., Context::getCurrent). Otherwise, they aren't converted (Reason: Creating 1 OTel span for each DD Span could become expensive for massive traces, considering OTel's interface carries many instances).
• Creating a span using DD's API assumes that its scope becomes activated, and activates ancestors from the oldest non-activated ancestor to the newly created span
• Converted spans use a custom InstrumentationScope (named datadog) and default resource information

Autoinstrumentation

• OTel & DD Auto instrumentation worked concurrently as far as my testing could tell. (TODO: Add some CI tests)
• Note to self: Look at web.request spans existence

Trace SDK

• Since span events and span links are missing, span events/links limits are missing
• Default span processors and exporters are supported as far as my testing could tell (e.g., Jaeger, see below)
  

Profiler

• Works with OTel-created spans (See screenshots below - Support Admin Link to Trace)
  
  

---

Misc:

• Datadog sources (src/ directory) will now be copied to the artifacts. It is necessary for the autoloading to be done when installing with datadog-setup.php (the problem doesn't exist with .apk or .deb)

Readiness checklist

• (only for Members) Changelog has been added to the release document.
• Tests added for this feature/bug.

Reviewer checklist

• Appropriate labels assigned.
• Milestone is set.
• Changelog has been added to the release document. For community contributors the reviewer is in charge of this task.

[bwoebi]

Note to self: This is really caused by bad handling of the trailing comma in distributed tracing parsing and has been fixed ... on the span links branch.

[bwoebi]

Is this intentionally just checking for hasEnded and not getDuration() != 0, which is only triggered when Context::current is called?

start_span();
$otelSpan = Context::current();
close_span();
var_dump($otelSpan->hasEnded()); // false?

[PROFeNoM]

That's a lovely counter-example 👍 Yes, this was intentional - I didn't want to use getDuration and rely on an instance variable, although this is an O(1) (only because I found it to be prettier). But, as you showed, it only works if the context is updated.

PS: I guess $this->endEpochNanos is also wrong (when calling $span->toSpanData()) in this example because of this

[PROFeNoM]

Speaking about toSpanData, that just made me realized that... the attributes may be wrong if you set/unset attributes from the DD-span...

[PROFeNoM]

Especially if you unset

[bwoebi]

Not sure about this:

• Given that endOTelSpan is only called on Context::current(), if there's no further access to Context::current() after terminating the span, the spanProcessor->onEnd() will not be called
• this only runs for datadog-only Spans which happen to be accessed at least once by a Context::current(), right?
• I'm not sure what the best behaviour is, maybe, for consistency exclusively run it on OTel managed spans?

[PROFeNoM]

Given that endOTelSpan is only called on Context::current(), if there's no further access to Context::current() after terminating the span, the spanProcessor->onEnd() will not be called

Yes. One alternative I also tried would have been to do an OpenTelemetryIntegration, which traces a span's termination... but faced limitations. To this day, I still think this would have been the cleanest way overall (i.e., have sort of an event when a DD span is created/closed without the OTel API so that we could simply instrument them and start/end OTel spans as needed).

this only runs for datadog-only Spans, which happen to be accessed at least once by a Context::current(), right?

Yes

I'm not sure what the best behaviour is, maybe, for consistency exclusively run it on OTel managed spans?

For consistency, it may indeed make sense

[bwoebi]

There are some minor tradeoffs in the code, like Span::endOTelSpan fetching getTraceState() - which incidentally is only called when the next Context::current() call happens.
There might be theoretical divergence, but as long as we acknowledge such edge cases, I think it's fine.

I've left one more point for consideration, but overall looks good I think :-)

[PROFeNoM]

@bwoebi

The changes made on the endEpochNanos/hasEnded 'flags' to simplify the code, although not impacting at all DD's end result, impact Jaeger traces.

I'll be running the span processors after the Datadog span termination. It may conflict with partial flushing in the future. Alternatively, I can just rollback the changes made and use some flags, but it fundamentally conflicts as well.

We can discuss this in our meeting later on today if you care about this.

Before commit:

After commit:

Datadog Trace

[bwoebi]

I think that's fine. Even if we implement partial flushing in future, we probably shouldn't immediately, irreversibly flush the span, but flush it slightly delayed, so that some data may still be set on the closed span, otherwise we risk breaking currently perfectly working code from other users, which nonchalantly add some meta data to the span after closing it.