add InstrumentationMode enum

[zeitlinger]

No description provided.

[trask]

I'm wondering if we even need these:

• otel.instrumentation.common.default-enabled
• otel.instrumentation.xyz.enabled

now that we have:

tracer_provider:
  tracer_configurator/development:
    default_config:
      disabled: true
    tracers:
      - name: io.opentelemetry.okhttp*
        config:
          disabled: false

[laurit]

I'm wondering if we even need these:

I think we still need to be able to disable individual instrumentations. For example imagine that instrumentation has a bug or does expensive class matching, then just disabling the tracer won't be enough. As for disabling all instrumentations with default-enabled I have never liked it much, but I'm sure we'd get a ton of requests for an option to let users selectively enable instrumentations if we'd remove this option.

[laurit]

I don't quite get why the default enabled couldn't be a simple boolean.

[zeitlinger]

I don't quite get why the default enabled couldn't be a simple boolean.

we wanted to allow for future values live "all", "minimal" or similar

[zeitlinger]

I think we still need to be able to disable individual instrumentations. For example imaging that instrumentation has a bug or does expensive class matching, then just disabling the tracer won't be enough.

I agree

[trask]

For example imaging that instrumentation has a bug or does expensive class matching, then just disabling the tracer won't be enough.

Should we scope these configuration options to the java agent then (under the agent distro node in declarative configuration)? We could restructure our general recommendations around using tracer config, while preserving this as an advanced java agent configuration.

[zeitlinger]

Should we scope these configuration options to the java agent then (under the agent distro node in declarative configuration)? We could restructure our general recommendations around using tracer config, while preserving this as an advanced java agent configuration.

The reason for the current design is that it allows native instrumentation to use the properties in a natural way.
Having tracer config in the SDK for a similar purpose is an additional argument to have this as a shared setting IMO.

[trask]

The reason for the current design is that it allows native instrumentation to use the properties in a natural way.

why does native instrumentation need to access these configuration options?

[zeitlinger]

why does native instrumentation need to access these configuration options?

for the same reason as in agent: disabling when it's broken

[trask]

I'm not sure that's an important use case outside of the Java agent (which needs it due to it being zero code, and due to the problems inherent in using bytecode instrumentation)

[zeitlinger]

I'm not sure that's an important use case outside of the Java agent (which needs it due to it being zero code, and due to the problems inherent in using bytecode instrumentation)

good point - I've created #15796 to continue the discussion.

This PR can be done first I think.

[trask]

This PR can be done first I think.

this PR seems unnecessary if we are going forward with #15796

also, I think we could reconsider whether we really need a multi-state "mode" or if boolean is good enough

[zeitlinger]

also, I think we could reconsider whether we really need a multi-state "mode" or if boolean is good enough

let's go back to boolean then.

I'll add a design proposal to #15796