feat: always return ExtendedOpenTelemetry when incubator is available

[zeitlinger]

this is the behavior we have for other extended classes and a first step towards #7961

[codecov]

Codecov Report

❌ Patch coverage is 95.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.13%. Comparing base (77b2f02) to head (808e482).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...etry/sdk/internal/OpenTelemetrySdkBuilderUtil.java	75.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7991      +/-   ##
============================================
- Coverage     90.14%   90.13%   -0.01%     
- Complexity     7476     7481       +5     
============================================
  Files           834      836       +2     
  Lines         22540    22562      +22     
  Branches       2236     2237       +1     
============================================
+ Hits          20319    20337      +18     
- Misses         1516     1520       +4     
  Partials        705      705              

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

Unused and a duplicate of ConfigProvider#noop()

[zeitlinger]

Called from https://github.com/zeitlinger/opentelemetry-java/blob/0db77eb839b0af1e7f32b0f7ced29e60edcce005/sdk/all/src/main/java/io/opentelemetry/sdk/IncubatingUtil.java#L26 to get an instance for ExtendedOpenTelemetrySdk.

I tried to make it return SdkConfigProvider - but then a design test failed.

[zeitlinger]

Called from https://github.com/zeitlinger/opentelemetry-java/blob/0db77eb839b0af1e7f32b0f7ced29e60edcce005/sdk/all/src/main/java/io/opentelemetry/sdk/IncubatingUtil.java#L26 to get an instance for ExtendedOpenTelemetrySdk.

I tried to make it return SdkConfigProvider - but then a design test failed.

[jack-berg]

This change isn't strictly necessary is it since the test code could still call ExtendedOpenTelemetrySdk.create(..)? Not suggesting reverting - just trying to confirm my understanding.

[zeitlinger]

see #7991 (comment)

[jack-berg]

IIUC, the benefit for this PR for callers is that now whenever opentelemetry-api-incubator is on the classpath, OpenTelemetrySdk.builder().build() will return an instance of ExtendedOpenTelemetrySdk.

[zeitlinger]

IIUC, the benefit for this PR for callers is that now whenever opentelemetry-api-incubator is on the classpath, OpenTelemetrySdk.builder().build() will return an instance of ExtendedOpenTelemetrySdk.

Yes that has benefits

• it's a consistent pattern towards stability
• you can't abuse instanceof ExtendedOpenTelemetry to check for declarative config (I did this mistake in the agent and that broke when we also used ExtendedOpenTelemetrySdk for non-declarative config)

[jack-berg]

I think we can go even further to align ExtendedOpenTelemetrySdk, SdkConfigProvider with the patterns we use in this repo for experimental features. Here's a PR to the source branch of this PR which shows how we might accomplish this by moving ExtendedOpenTelemetrySdk, SdkConfigProvidert to the opentelemetry-sdk package.

I think long term, we end up with a opentelemetry-sdk-config package analog to opentelemetry-sdk-metrics, etc. And this is a step in that direction. But there is still an open question of how that would work when the declarative config code depends on jackson, and of course we don't want to require all SDK users to require a jackson dependency. Maybe we structure the module / API in such a way where its a "bring your own jackson" situation, and you unlock additional features by opting into jackson, but basic functionality works without it. That's a different conversation though.

[zeitlinger]

Maybe we structure the module / API in such a way where its a "bring your own jackson" situation, and you unlock additional features by opting into jackson, but basic functionality works without it. That's a different conversation though.

sounds complicated

maybe we can have (Sdk)ConfigProvider be the API that is implemented in opentelemetry-sdk-config using jackson - or you provide your own

[jack-berg]

maybe we can have (Sdk)ConfigProvider be the API that is implemented in opentelemetry-sdk-config using jackson - or you provide your own

Something like this: (ignore the cringy names)

public interface SdkConfigProvider extends ConfigProvider {}
public class JacksonYamlSdkConfigProvider implements SdkConfigProvider {}
public class MyCustomSdkConfigProvider implements SdkConfigProvider {}

Another way we could go is we could define an SPI responsible for doing the YAML/json to POJO mapping we rely on jackson for. Then what class or method is responsible for parsing YAML/json could resolve the SPI. We provide a default implementation for jackson but allow the user to provide their own SPI implementation.