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.3
tracer_provider: ...
logger_provider: ...
meter_provider: ...
instrumentation/development:
  java:
    otel_sdk:
      internal_telemetry_version: latest # or legacy

Also adjusts the default internal telemetry when declarative config is used. By default, internal telemetry is disabled. This is in contrast with system properties / env vars, which defaults to InternalTelemetryVersion.LEGACY, to maintain compatibility. The idea of default to disabled is to allow us to enable by default once semantic conventions is stable, without it being considered a breaking change.

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]

No worries.

Was thinking about it more overnight and I still wonder if the approach is worth exploring despite its complexity. One thing we've talked about is that there should be better error reporting / logging when something goes wrong. For example, right now if a property has the wrong type (i.e. string when an int is expected), we log Ignoring value for key [foo] because it is string instead of int: bar. These logs would be much more useful if they told the user where in the config file the error took place #7949. But for this to work, each DeclarativeConfigProperties needs to embed path information in it, which means that the idea of a singleton DeclarativeConfigProperties.empty() isn't possible since each instance is unique to its context.

So half the the issues I mention above go away when #7949 is solved. Maybe the other half of the issue doesn't look as bad at that point..

[codecov]

Codecov Report

❌ Patch coverage is 90.76923% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.19%. Comparing base (9a089b6) to head (ef41c82).

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

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #8045   +/-   ##
=========================================
  Coverage     90.19%   90.19%           
- Complexity     7588     7607   +19     
=========================================
  Files           841      842    +1     
  Lines         22906    22948   +42     
  Branches       2289     2294    +5     
=========================================
+ Hits          20659    20698   +39     
- Misses         1531     1534    +3     
  Partials        716      716           

☔ 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.

[jack-berg]

Updated this PR to disable internal telemetry by default, per this conversation. Updated PR description with explanation:

Also adjusts the default internal telemetry when declarative config is used. By default, internal telemetry is disabled. This is in contrast with system properties / env vars, which defaults to InternalTelemetryVersion.LEGACY, to maintain compatibility. The idea of default to disabled is to allow us to enable by default once semantic conventions is stable, without it being considered a breaking change.