Add support for setting internal telemetry version w/ declarative config

[jack-berg]

Resolves #7928.

Declarative config counterpart to #8037.

Allows you to specify the version of internal telemetry w/ declarative config. For example:

file_version: 1.0-rc.2
tracer_provider: ...
logger_provider: ...
meter_provider: ...
instrumentation/development:
  java:
    otel_sdk:
      internal_telemetry_version: latest # or legacy

cc @anuraaga

[jack-berg]

For the OTLP epxorters to be able to access instrumentation/development.java.otel_sdk, I needed to figure out a strategy to provide access to ConfigProvider. Considered adding an accessor to DeclarativeConfigProperties but couldn't make it work.

Also considered a breaking change to ComponentProvider to add ConfigProvider param to create. But breaking change means churn and only a select few ComponentProvider implementations will need access ConfigProvider.

This pattern strikes a balance.

[anuraaga]

I'm not deep on the general design of the types, but an idea that came to mind is just adding a getter to DeclarativeConfigProperties. While purists might want it to be "only the config for that component", we already have ComponentLoader there which seems to break from that, an escape hatch into the global config doesn't seem that bad and has no churn (I didn't verify it's possible but couldn't think of a reason it wouldn't be)

opentelemetry-java/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/DeclarativeConfigProperties.java

Line 250 in d31d468

ComponentLoader getComponentLoader();

[anuraaga]

Considered adding an accessor to DeclarativeConfigProperties but couldn't make it work.

Oops! What was the problem with that?

[jack-berg]

It was a couple things:

• chicken and egg problem, where SdkConfigProvider is initialized from DeclarativeConfigProperites, and DeclarativeConfigProperites needs a ref to ConfigProvider during initialization. Can solve with something like an Atomic reference, but gets sloppy.
• we make liberal use of a stateless DeclarativeConfigProperites.empty() instance, which would need to now hold a ref to a specific ConfigProvider instance

I could probably make it all work, but the amount of impacted code was growing quickly so I looked for other options before learning the full extent.

[anuraaga]

Thanks - sorry for the noise that indeed looks worse than this approach which I could check by looking closer at the code

[jack-berg]

I followed the implementation of #8037 and don't set the internal telemetry version if instrumentation/development.otel_sdk.internal_telemetry_version isn't set.

But what I should probably do is default to latest:

• Since declarative config is new, we can still change the default internal telemetry verison.
• Not setting the internal telemetry version doesn't mean that its disabled. It means that it uses whatever the default is, which varies by component. So not choosing a default means more users exposed to legacy.

[jack-berg]

Thinking about this more, the thing that got us in trouble with the internal telemetry initially was that we went ahead and enabled it by default long before the semantic conventions existed or were stable.

To avoid this with declarative config, we might consider disabling all internal telemetry by default, and only enabling according to the level specified with instrumentation/development.otel_sdk.internal_telemetry_version.

[anuraaga]

This is a good point - there is no mention of opt-in in https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics/ so I have been assuming they are enabled by default in principle, while SDKs with legacy concerns like Java may have to work around that, as we've done by sticking to LEGACY as the default. So not sure if disabling by default would be a correct interpretation of that spec. Note, internally I've had discussions on how to disable these if desired and assumed metric views are the tool for that.

The above may be sidestepped though by my personal opinion that it's probably best to keep defaults the same for programmatic usage of SDK and declarative config (and non-declarative config). Otherwise, it just seems like the cognitive load of knowing which default is which could get quite high for both users and maintainers. I don't think anyone expects declarative config to have better defaults than the norm.

While I'm sure it must have been proposed by now, I'd also put in my hat that even for 1.x software, a long period of warnings followed by a breaking behavior change with a very easy knob to switch back is acceptable. If LEGACY triggered a logOnce warning, in August or so the default can become semconv. All Java operators that actually update their dependencies should have enough control to set the sys prop / env var in #8037 by then.

[codecov]

Codecov Report

❌ Patch coverage is 89.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.18%. Comparing base (d31d468) to head (208a3ec).

Files with missing lines	Patch %	Lines
...orter/otlp/internal/OtlpDeclarativeConfigUtil.java	75.00%	2 Missing and 1 partial ⚠️
...incubator/fileconfig/DeclarativeConfigContext.java	83.33%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #8045   +/-   ##
=========================================
  Coverage     90.17%   90.18%           
- Complexity     7479     7498   +19     
=========================================
  Files           834      835    +1     
  Lines         22559    22607   +48     
  Branches       2239     2250   +11     
=========================================
+ Hits          20342    20387   +45     
- Misses         1515     1518    +3     
  Partials        702      702           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.