Instrumenter SpanOmitter

[LikeTheSalad]

An approach to allow suppressing Spans for an Instrumenter instance.

Example usage:

SpanOmitter omitter = ...

Instrumenter.<String, String>builder(OpenTelemetry, "test", request -> "test span")
  .addSpanOmitter(omitter)
  .buildInstrumenter();

The provided SpanOmitters allow to either continue with the Span creation or not, and their decision can be changed at any point at runtime. This is useful to allow for remote config instrumentation filtering for example.

By the way, I think SpanSuppressor could have been a better name but it was already taken.

[trask]

what is your specific use case? (because we have a couple different alternative solutions floating around, e.g. open-telemetry/opentelemetry-java#6197)

By the way, I think SpanSuppressor could have been a better name but it was already taken.

Would SpanSuppressor solve your use case if we made that class public?

[laurit]

I don't get this. SpanSuppressor is used to suppress nested spans for the same semconv/kind. So if you have a high level http client that uses a low level library, such as netty, the span created by the instrumentation for the high level client will suppress the one from the low level client and you don't end up with 2 spans for the same http call. In the PR description you say

The provided SpanOmitters allow to either continue with the Span creation or not, and their decision can be changed at any point at runtime. This is useful to allow for remote config instrumentation filtering for example.

Would replacing

opentelemetry-java-instrumentation/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java

Line 80 in 34e0eb6

private final boolean enabled;

with BooleanSupplier or Predicate satisfy this?
If you want to suppress span creation this way I'd imagine that toggling it per Instrumenter by adding a SpanOmitter isn't the most convenient way. Perhaps it would make sense to have a global predicate that is applied to all instrumenters that based on the instrumentation name, span kind, context etc. decides whether span should be created? This way you wouldn't need to separately modify instrumenters to have them participate in this. You could have rules that say drop all INTERNAL spans or drop all spans from okhttp instrumentation and modify these at runtime.

[LikeTheSalad]

Hi, thanks for the feedback!

what is your specific use case? (because we have a couple different alternative solutions floating around, e.g. open-telemetry/opentelemetry-java#6197)

I think if samplers had access to the scope, as mentioned in that issue, it could work because my use case is about being able to toggle on/off a single auto-instrumentation (although there might be some other details needed with that approach that I'm ignoring right now, but overall it looks like it should work). So I'm looking forward to those changes, although it seems like it would require involving changes in the spec, so I'm a little worried about how long it would take.

Would SpanSuppressor solve your use case if we made that class public?

I think so, but we would also need some setter in the InstrumenterBuilder to provide my own and evaluate it after the existing SpanSuppressor.

I don't get this. SpanSuppressor is used to suppress nested spans for the same semconv/kind. So if you have a high level http client that uses a low level library, such as netty, the span created by the instrumentation for the high level client will suppress the one from the low level client and you don't end up with 2 spans for the same http call.

I think a concrete example might help better illustrate the idea of what I'm looking for, so let's take this okhttp auto-instrumentation for example, the only way Android users have to configure its behavior is through the OkHttpInstrumentationConfig config object, which is later used internally to build the okhttp instrumenter here.

The SpanOmitter idea could be helpful to provide a setter for custom SpanOmitters inside the OkHttpInstrumentationConfig config object to later pass them to the creation of the okhttp instrumenter instance. This way there could be several filters set by either the end users or a vendor, such as Elastic, which has a remote config system that could use it to turn the okhttp instrumentation on/off having some SpanOmitter implementation that is aware of said remote config and checks it before deciding what to return in the shouldOmit method.

Would replacing ... with BooleanSupplier or Predicate satisfy this?

That sounds promising, I wasn't aware of that flag, though now that I think about it, it looks like it could be the simplest way to accomplish what I'm looking for. I'll take a deeper look at it today and run some tests to be sure, thanks!

Perhaps it would make sense to have a global predicate that is applied to all instrumenters that based on the instrumentation name, span kind, context etc.

I think this may be related to the issue that Trask shared above about making these decisions on samplers that are scope-aware. I guess the scope name could be enough for a global predicate to make a decision on what passes and what doesn't.

[LikeTheSalad]

Update: I think by using the enabled flag it should work, although we're still going to need to add some changes as you mentioned @laurit, to provide some sort of supplier for it when building the Instrumenter. I'll bring this up in today's SIG and, if no objections, I can create a PR for that approach.

[LikeTheSalad]

Closing this issue as these changes are no longer needed.