Introduce ConfigProvider API

[jack-berg]

This PR introduces a new category of API surface area called ConfigProvider, which is analog to TracerProvider, MeterProvider, LoggerProvider.

ConfigProvider (and its corresponding global accessor GlobalConfigProvider) provides a mechanism for instrumentation to read data from the new .instrumentation portion of declarative configuration.

Why does instrumentation need to participate in config?

To provide a good user experience, we should centralize all otel config a user might provide in a single place. Limiting declarative config to SDK configuration just kicks the problem of instrumentation config down the road when the problem domains are quite similar.

Why do we need config surface area in the API instead of just SDK?

Historically, we've limited configuration to the SDK and the related autoconfigure module. (Although long ago we debated whether certain classes from the opentelemetry-extension-autoconfigure-spi like ConfigProperties should be in the opentelemetry-api instead). This pattern breaks down with instrumentation config because we want native instrumentation to read from declarative config without taking a dependency on any SDK artifacts.

What's the user flow for interacting with ConfigProvider?

Let's use the otel java agent as an example. Supposing a user opts into declarartive configuration by setting OTEL_EXPERIMENTAL_CONFIG_FILE=/path/to/config.yaml, the agent will install an SDK configured according to the file contents using logic in opentelemetry-sdk-extension-autoconfigure / opentelemetry-sdk-extension-incubator, which are bundled with the otel java agent.

This PR extends the autoconfigure logic to initialize a ConfigProvider instance when OTEL_EXPERIMENTAL_CONFIG_FILE is specified, which is accessible in via:

• AutoConfigureUtil#getConfigProvider()
• GlobalConfigProvider#get()

ConfigProvider has just a single operation right now:

  @Nullable
  StructuredConfigProperties getInstrumentationConfig();

Where StructuredConfigProperties represents the YAML node at .instrumentation from the user's config file. I've added convenience methods in InstrumentationConfigUtil to read common properties:

public static Map<String, String> peerServiceMapping(ConfigProvider configProvider) {}
public static List<String> httpClientRequestCapturedHeaders(ConfigProvider configProvider) {}
public static List<String> httpClientResponseCapturedHeaders(ConfigProvider configProvider) {}
public static List<String> httpServerRequestCapturedHeaders(ConfigProvider configProvider) {}
public static List<String> httpSeverResponseCapturedHeaders(ConfigProvider configProvider) {}

And a convenience method for accessing properties specific to a particular instrumentation library:

public static StructuredConfigProperties javaInstrumentationConfig(ConfigProvider configProvider, String instrumentationName)

The agent would need to be updated to detect that file configuration was used, and to configure the various instrumentation libraries it installs according to the installed ConfigProvider.

Any native instrumentation can similarly read ConfigProvider on initialization and configure itself accordingly.

What does this PR do to make all this work?

There are lot of files changed to accomplish this, but they mostly shift existing code around to move the required libraries to opentelemetry-api-incubator module. Notably, the spec PR introducing the instrumentation configuration API rebrands "file configuration" to be "declarative configuration". Its best to do user facing class renames at the same time that those classes are moving packages to minimize churn.

Here's a summary of the changes:

• Introduce new io.opentelemetry.api.incubator.config package in opentelemetry-api-incubator to hold all classes / interfaces required for the config API
• Introduce new ConfigProvider, GlobalConfigProvider, InstrumentationConfigUtil representing the config API
• Move StructuredConfigProperties from opentelemetry-sdk-extension-incubator to opentelemetry-api-incubator, rename to DeclarativeConfigProperties
• Replace all file config usages of ConfigurationException with new DeclarativeConfigurationException which lives in opentelemetry-api-incubator and can be referenced in API

[brunobat]

From what I understand, if you provide an implementation for StructuredConfigProperties, then the getConfig():

opentelemetry-java/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java

Line 429 in d9cef81

ConfigProperties config = getConfig();

Will return your implementation, If you provide no implementation, then the existing propertiesSupplier and propertiesCustomizerswill kick in.
Right?

[jack-berg]

Will return your implementation, If you provide no implementation, then the existing propertiesSupplier and propertiesCustomizerswill kick in. Right?

No. This API only provides StructuredConfigProperties, and alternative to ConfigProperties which is akin to a YAML mapping node. Its possible for ConfigProvider to return something useful when file config is not used, but we'd need to design that mechanism.

[brunobat]

No. This API only provides StructuredConfigProperties, and alternative to ConfigProperties which is akin to a YAML mapping node. Its possible for ConfigProvider to return something useful when file config is not used, but we'd need to design that mechanism.

Sorry, I'm confused.
From this code:

 private ConfigProperties getConfig() {
    ConfigProperties config = this.config;
    if (config == null) {
      config = computeConfigProperties();
    }
    return config;
  }

If no structured properties are present, properties will be computed from current propertiesSupplier and propertiesCustomizers... If this is not the case, then it's a breaking change...

[jack-berg]

If no structured properties are present, properties will be computed from current propertiesSupplier and propertiesCustomizers... If this is not the case, then it's a breaking change...

Sorry I must have misunderstood your question. This PR introduces net new (experimental) API surface area in the opentelemetry-api-incubator module. None of the APIs in the stable opentelemetry-sdk-extension-autoconfigure module are changed / impacted.

[brunobat]

thanks

[codefromthecrypt]

handy. hope the review helps despite being not authoritative @jack-berg!

[codefromthecrypt]

recommend tests for below cases possibly something that can be refactored to a common fixture for all, but even if explicit 1. path is incomplete 2. middle of the path is the wrong type 3. end of the path is the wrong type 4. end of the path is empty

[codefromthecrypt]

if we care, we run this again to test that the value returned is a constant

[jack-berg]

handy. hope the review helps despite being not authoritative @jack-berg!

Thanks for your time - lots of good advice in here 🙂

[codecov]

Codecov Report

Attention: Patch coverage is 77.30496% with 32 lines in your changes missing coverage. Please review.

Project coverage is 90.04%. Comparing base (a6a9acb) to head (81f601c).

Files with missing lines	Patch %	Lines
...pi/incubator/config/InstrumentationConfigUtil.java	86.66%	1 Missing and 3 partials ⚠️
...incubator/fileconfig/DeclarativeConfiguration.java	71.42%	2 Missing and 2 partials ⚠️
...or/fileconfig/YamlDeclarativeConfigProperties.java	77.77%	4 Missing ⚠️
...elemetry/sdk/autoconfigure/internal/SpiHelper.java	33.33%	2 Missing ⚠️
...nsion/incubator/fileconfig/AggregationFactory.java	0.00%	2 Missing ⚠️
...incubator/fileconfig/LogRecordExporterFactory.java	0.00%	2 Missing ⚠️
...ncubator/fileconfig/LogRecordProcessorFactory.java	0.00%	2 Missing ⚠️
...on/incubator/fileconfig/MetricExporterFactory.java	0.00%	2 Missing ⚠️
...extension/incubator/fileconfig/SamplerFactory.java	0.00%	2 Missing ⚠️
...sion/incubator/fileconfig/SpanExporterFactory.java	0.00%	2 Missing ⚠️
... and 5 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #6549      +/-   ##
============================================
+ Coverage     90.02%   90.04%   +0.01%     
- Complexity     6488     6516      +28     
============================================
  Files           719      724       +5     
  Lines         19587    19643      +56     
  Branches       1931     1938       +7     
============================================
+ Hits          17634    17687      +53     
  Misses         1354     1354              
- Partials        599      602       +3     

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

[jack-berg]

The spec PR introducing the configuration API is merged and I'm now seeking reviews for this with the intent to merge.

[codefromthecrypt]

@geoand @ThomasVitale I know both of you work in different frameworks, so that's a good thing ;) Imagine in LLM a setting for disabling prompt logging, and this would be obtained by a config provider, using config semantics that also apply to other languages like python or Go.

In understand that in your frameworks, config loading compat might not be a specific goal, but if you have any commentary around this impl, it would be much obliged. conventional and standardized loading is very helpful when agents are in use.

[geoand]

Thanks for the ping!

I'll have a look next week when I'm back from PTO