search

DataDog / dd-trace-js

JavaScript APM Tracer
https://docs.datadoghq.com/tracing/
Other
640 stars 303 forks source link

Open Telemetry Integration Does not Adhere to Specification #4380

Open chrisguitarguy opened 4 months ago

chrisguitarguy commented 4 months ago

Specifically this bit https://github.com/DataDog/dd-trace-js/blob/c831ffa83f26365df1ccd829d8d775010bacd3a7/packages/dd-trace/src/opentelemetry/context_manager.js#L25-L33

Which causes something like this:

import { trace } from '@opentelemetry/api';

const span = trace.getActiveSpan();

To return a span that doesn't actually implement Span (see https://github.com/open-telemetry/opentelemetry-js/blob/957fa3b5e44e43f0cf07adeca8e118f27334dfc3/api/src/trace/span.ts#L33)

Instead it's just a plain object with spanContext and one cannot do something like span.setAttributes(...) that works just fine without datadog's tracer provider.

Would the DD Span implementation alredy take care of this? https://github.com/DataDog/dd-trace-js/blob/c831ffa83f26365df1ccd829d8d775010bacd3a7/packages/dd-trace/src/opentelemetry/span.js#L177-L179

dpnova commented 3 months ago

Agreed, there's places where the otel tracer assumes a parent trace is a DD trace too: https://github.com/DataDog/dd-trace-js/blob/master/packages/dd-trace/src/opentelemetry/tracer.js#L36

With DD_TRACE_OTEL_ENABLED this completely breaks

andrewpuch commented 2 months ago

I'm running into a similar issue as @dpnova where parent is null and parent._traceId throws an error.

dpnova commented 2 months ago

I'm running into a similar issue as @dpnova where parent is null and parent._traceId throws an error.

FWIW I left DD_TRACE_OTEL_ENABLED off and manually created the connection between the parent and child span on the boundary of my DD/OTEL traced code. Hope you get it sorted @andrewpuch