Allow restricting propagation of OpenTelemetry trace headers

[hacst]

Description

As described in the w3c trace-context and baggage specifications tracing headers might carry sensitive information. Currently once a propagator is configured it unconditionally propagates regardless of who the service is talking to. If a service calls out to an untrusted system/third party this would expose the information.

As the headers being propagated might come from another applications call, it is very hard to ensure no sensitive data is present in the application actually making the request. The only safe solution is to prevent the information from being propagated to untrusted systems in the first place.

Currently there does not seem to be any way to restrict or customize this propagation behavior while using the quarkus-opentelemetry extension (zulip).

As a developer it would be great to be able to whitelist URL patterns (and/or IP ranges) for propagation in the quarkus-opentelemetry configuration. This would allow safe configuration of who the information is transmitted to without much effort. The default would whitelist everything to retain compatibility.

Another, less safe, option would be some option to explicitly block propagation for certain URLs or rest clients through configuration/annotation.

Implementation ideas

No response

[quarkus-bot]

/cc @brunobat (opentelemetry,tracing), @radcortez (opentelemetry,tracing)

[quarkus-bot]

You added a link to a Zulip discussion, please make sure the description of the issue is comprehensive and doesn't require accessing Zulip

This message is automatically generated by a bot.

[brunobat]

@hacst can you please provide an example of how this should work?
A set of headers, how to configure the deny list and the expected output?

[luneo7]

Currently there is a way of creating your own propagator, and you can set your own rules, you do something like this:
1 - Create a propagator that implements io.opentelemetry.context.propagation.TextMapPropagator
2 - Create a configurable propagator provider that implements io.opentelemetry.sdk.autoconfigure.spi.ConfigurablePropagatorProvider
3 - Create a META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.ConfigurablePropagatorProvider that will have its content pointing to the class that you created in step 2
4 - Set the quarkus.opentelemetry.propagators=nameOfConfigurablePropagatorProvider in the application.properties file

With that ^, you will be able to create a custom propagator and load it in Quarkus, as Quarkus uses the SDK autoconfigure SPI to load the propagator =]

Regarding the spec, maybe it should be better to open an upstream issue (https://github.com/open-telemetry/opentelemetry-java) so OTEL supports the spec, as the trace-context and baggage propagators comes from the OTEL lib and not from Quarkus.

[brunobat]

I agree with @luneo7 that this should be better supported by OTel and an issue should by filed on them. However, given that is currently quite cumbersome to filter out those sensible headers I think we should keep this issue in our backlog.

[hacst]

Thanks for the responses. I'll report the issue upstream as suggested and link it here once I've done so.

[brunobat]

I think there will be some time until OTel provides a solution to this... Added a comment there.
It's worthwhile to explore a Quarkus solution for this, in the meantime.

[geoand]

Set the quarkus.opentelemetry.propagators=nameOfConfigurablePropagatorProvider in the application.properties file

Where in our code is quarkus.opentelemetry.propagators taken into account?

In any case, I think that a perfectly usable solution (and in line with what we do in other extensions) would be add:

@Target({ TYPE })
@Retention(RetentionPolicy.RUNTIME)
public @interface TenantFeature {
  /**
   * Returns the name of this propagator, which can be specified with the {@code otel.propagators}
   * property to enable it. If the name is the same as any other defined propagator name, it is
   * undefined which will be used.
   */
    String name();
}

which users would add to their implementations of TextMapPropagator.
Under the covers, we make turn these implementations into beans, and have our own ConfigurablePropagatorProvider that obtains the aforementioned TextMapPropagator implementations from CDI.

WDYT?

[brunobat]

I agree we should add a Quarkus feature for this but I think we should base our implementation on a propagation customiser.
see: https://github.com/open-telemetry/opentelemetry-java/blob/2d6b26b10d47f66290ed76ad7f9e4b5ec22dc161/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java#L381

In the OpenTelemetryProducer we could just do something like:

final AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk = AutoConfiguredOpenTelemetrySdk.builder()
...
                .addPropagatorCustomizer((textMapPropagator, configProperties) -> {
                    Collection<String> fields = textMapPropagator.extract()fields();
...
                })
...

We could include a Quarkus property to filter out certain headers.

[geoand]

Hm... The customizer takes an already created TextMapPropagator and changes it. Is that what we want or do we want to allow the give users the ability to create new ones?

[brunobat]

There are a bunch of propagators in the wild:

public enum PropagatorType {
    TRACE_CONTEXT(Constants.TRACE_CONTEXT),
    BAGGAGE(Constants.BAGGAGE),
    B3(Constants.B3),
    B3MULTI(Constants.B3MULTI),
    JAEGER(Constants.JAEGER),
    XRAY(Constants.XRAY),
    OT_TRACE(Constants.OT_TRACE);

The default is W3C, the one using TRACE_CONTEXT and is implemented out of the box in the OTel SDK.
There are libraries for the others. I don't see much ppl creating their own propagation schemas.

If we create a custom propagator we will be an island. If we use the customiser we can affect any content on all the already existing propagators.

[geoand]

Gotcha, thanks. So in that case all we need is to define some kind interface that users will implement and mark as a bean.

[brunobat]

Yes, a bean that would expose the customisation for the user to override, would be optimal.
FYI, If we want all or nothing, there is already an SDK config at runtime: quarkus.opentelemetry.propagators=none Will not propagate anything.

[geoand]

#34920 is what I have in mind. WDYT? If that seems reasonable, I'll go ahead and add a test.

[brunobat]

@geoand I think that will work great.
@hacst would this satisfy your use case?

[hacst]

If I understand correctly I would use this to create a wrapper around the propagator. On inject I would then introspect the carrier to see if it is a known type and whether it is OK to propagate according to my configuration and only then do the actual inject?

If that is possible then it should allow the kind of filtering I was trying to do. It is a bit unfortunate that this will have to rely on knowledge of the type and internals of the carriers, but as long as it allows handling normal framework http requests that will cover a lot already.

[geoand]

@brunobat I don't suppose OTel 1.28 changes anything in this APO does it?

[brunobat]

No, there are no changes in the autocunfigure API or on the propagation side.