open-telemetry / opentelemetry-collector-contrib

Contrib repository for the OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
3.1k stars 2.38k forks source link

Support virtual node with zipkin shared span ID #35358

Open EOjeah opened 1 month ago

EOjeah commented 1 month ago

Component(s)

connector/servicegraph

Is your feature request related to a problem? Please describe.

A way to configure the connector to form a virtual node from zipkin model with shared span ID.

Describe the solution you'd like

A way to configure the connector to work with systems that use zipkin context propagation where both client and server spans share the same span ID

Describe alternatives you've considered

No response

Additional context

Currently, service graph connector supports otel context proration for client, server architecture. This is when the server is linked to the client by the server having the parentSpanID set to the client span ID. https://opentelemetry.io/docs/concepts/context-propagation/#context

Can this connector also support RPC with zipkin model where client and server share the same span ID?

Fully aware of this statement but Zipkin has the concept of shared IDs https://opentelemetry.io/docs/specs/otel/context/api-propagators/#b3-extract

MUST NOT reuse X-B3-SpanId as the id for the server-side span.

github-actions[bot] commented 1 month ago

Pinging code owners:

crobert-1 commented 1 month ago

Removing needs triage as a code owner has responded 👍 to the enhnancement proposal.

EOjeah commented 1 month ago

thanks @mapno and @crobert-1 for looking into this. I might need some further clarification.

The idea behind this is to have a new config key/value with something like

servicegraph:
  propagator: b3 # default w3c

Which will adhere to B3 Specification for shared span ID between client and server But according to the OpenTelemetry API guide B3-Requirements, implementors of libraries should follow this in order to maximize compatibility between OpenTelemetry and Zipkin.

B3 Extract

When extracting B3, propagators: MUST NOT reuse X-B3-SpanId as the id for the server-side span.

B3 Inject

When injecting B3, propagators: MUST NOT propagate X-B3-ParentSpanId as OpenTelemetry does not support reusing the same id for both sides of a request.

Which means that this config key/value will not work as expected for those using the opentelemetry-sdk with b3-propagator but will work for zipkin instrumentation that follows the actual B3 specification.

I guess the other method is to use something like below and ignore the b3 stuff

servicegraph:
  shared_span_id: true

What do you think?