envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.74k stars 4.75k forks source link

opentelemetry tracing provider doesn't respect client_sampling when both traceparent and x-client-trace-id header is present #35721

Open yuanguolang opened 1 month ago

yuanguolang commented 1 month ago

Title: opentelemetry tracing provider doesn't respect client_sampling when both traceparent and x-client-trace-id header is present

Description: Hi, we enabled opentelemtry tracing provider and we found out client_sampling in tracing is not applied if the incoming request has both traceparent and x-client-trace-id header. We also use x-request-id by default. Even if we set client_sampling to 0%, envoy still exports spans based on sample flag in parent context(traceparent) header. And envoy code seems align with the behavior. I wonder if this is the expected behavior, which means if envoy can extract the parent context from the incoming request, envoy will respect the sampling decision in the downstream, then no sampling decision will be applied.

in conn-manager, envoy startspan no matter what tracing decision made earlier:

https://github.com/envoyproxy/envoy/blob/6b9db09c69965d5bfb37bdd29693f8b7f9e9e9ec/source/common/http/conn_manager_impl.cc#L1397-L1398

in opentelemetry tracer, if envoy can extract parent context, which is a valid traceparent header, it will start a span from parent context, ignoring tracing decision made earlier. https://github.com/envoyproxy/envoy/blob/6b9db09c69965d5bfb37bdd29693f8b7f9e9e9ec/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc#L56-L68

thank you!

adisuissa commented 1 month ago

cc @alexanderellis @htuch as code owners. cc @wbpcode who has lots of tracing expertise.

yuanguolang commented 1 month ago

Because in the envoy documentation(link), client_samping is:

Target percentage of requests managed by this HTTP connection manager that will be force traced if the x-client-trace-id header is set.

So I thought if the x-client-trace-id is present, no matter if parent context can be extracted, client_sampling should be respected. Please correct me if I understand it wrong here.