open-telemetry / opentelemetry-java-instrumentation

OpenTelemetry auto-instrumentation and instrumentation libraries for Java
https://opentelemetry.io
Apache License 2.0
1.94k stars 850 forks source link

Change enable property on Instrumenter to a Predicate<Instrumenter> or a Supplier<Boolean> #11927

Open osvaldopina opened 2 months ago

osvaldopina commented 2 months ago

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

Allows a fine grain control over span creation for a Instrumenter.

Describe the solution you'd like

... private final Predicate<Instrumenter> enabled; ...

...

 public boolean shouldStart(Context parentContext, REQUEST request) {
     if (!enabled.test(this)) {
      return false;
    }

...

Or

... private final Supplier<Boolean> enabled; ...

...

 public boolean shouldStart(Context parentContext, REQUEST request) {
     if (!enabled.get()) {
      return false;
    }

...

Describe alternatives you've considered

I could not find any other way to enable or disable Instrumenter span creation on th fly. If there is such a mecanism this feature request would not be necessary.

Additional context

No response

laurit commented 2 months ago

Please describe why you would need this.

osvaldopina commented 2 months ago

In my use case I create the instrumenters, both the ones provided by otel instrumentation and the ones that we develped internally, and I would like to be able to enable or disable a specific Instrumenter at runtime.

trask commented 3 weeks ago

hi @osvaldopina, is this to support dynamic configuration? e.g. https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/12251#issuecomment-2352404592

osvaldopina commented 2 weeks ago

Hi @trask. Yes, that's the goal.

trask commented 2 weeks ago

there's a broader effort happening that I think may address your use case, check out https://github.com/open-telemetry/opentelemetry-java/pull/6687 and https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/12251#issuecomment-2352404592

osvaldopina commented 1 week ago

I took a look at the issues you pointed out. The 6687 has a broader effect of enabling or disabling all traces. I need something tied to each instrumenter allowing a much finer control. I saw 12251 and, as it is now, it is a list of intentions and my proposition could address one aspect of it. Looking at 6687 I realize that I can achieve what I need if the final clause is removed from the enabled attribute.

trask commented 1 week ago

The 6687 has a broader effect of enabling or disabling all traces

it is per tracer (i.e. per instrumentation)

trask commented 1 week ago

you can see more thoughts about 6687 in the spec work behind it: https://github.com/open-telemetry/opentelemetry-specification/issues/3867