Add Distribution section for vendor-specific settings

[ysolomchenko]

Fixes #336

What

This PR introduces a new top-level distribution section.

The distribution section provides a standardized and explicit location for distribution-specific configuration parameters that are not part of the OpenTelemetry model. It enables OpenTelemetry distributions to define and expose their own extensions and general configuration options in a structured and interoperable way.

[marcalff]

Please describe what the yaml parser implemented in each SDK is supposed to do with this.

[ysolomchenko]

Please describe what the yaml parser implemented in each SDK is supposed to do with this.

It’s related to another repository: opentelemetry-specification

The plan is to create a separate PR in open-telemetry/opentelemetry-specification that extends the config provider

Currently, the config provider represents only the .instrumentation node, but it should also provide the .distribution node.
Alternatively, another approach would be to create a separate distribution provider instead of extending the existing config provider. New provider would represent the .distribution node.

Now, to directly answer to your question about what the parser should do:
The parser should simply parse the YAML node into an object containing all its fields.
That means it must adapt dynamically depending on the contents of the node, since each vendor may define its own specific configuration structure.

I understand your concern might be about implementing this in C++, since not knowing the exact structure of the node in advance can be problematic for static typing.
A possible solution would be to parse the YAML node into a generic representation for example, using a variant type such as std::any, or a map-like structure.
This allows the SDK to store and traverse arbitrary configuration data without needing to define all fields upfront.

[marcalff]

Thanks for the details.

See my comments / questions on the related issue.

[robsunday]

I think it would be good to update also kitchen-sink.yaml example

Clarifications provided

[robsunday]

LGTM

[zeitlinger]

Java agent and spring starter currently have some settings under agent and spring_starter, e.g. debug.

I think it should stay that way to make it clear that a vendor can have a distribution of the agent or starter.

For example:

• spring starter has spring_starter / debug
• parts unlimited has parts_unlimited / part_details
  • you can still use spring_starter / debug - and don't have to move it to parts_unlimited / debug

@trask since we talked about it

[zeitlinger]

Suggested change

	If omitted, noop distribution settings are used.
	If omitted, noop distribution settings are used.
	The OpenTelemetry Java Agent and Spring Boot Starter should not use this field.

[ysolomchenko]

I don’t think it’s worth adding Java-specific comments to the general documentation. The best place for that would be the documentation for your implementation.

[zeitlinger]

we can probably find a more generic wording to explain that this is only for vendor distributions - which might again be a reason to go back to vendor instead of distribution

[MrAlias]

Agreed that we should not be adding Java specific language to the configuration file.

[zeitlinger]

If you feel that distribution is still the right term, then I'm fine.

[MrAlias]

Overall, this looks good to me outside of the "noop distribution" issue. Once that is resolved this looks ready.

[jack-berg]

Should this be distribution/development, and reference #/$defs/ExperimentalDistribution?

With stability coming soon, we had better be sure about anything which isn't marked experimental. Makes me nervous to have new concepts which will be stable out of the gate.

[ysolomchenko]

I'm not sure why this need to be marked as unstable, since there are no configurations for OpenTelemetry inside the node. Everything used there relies on vendor-specific configurations. The only thing that might be unstable on the OpenTelemetry side is the name of this node. Should we really mark it as development just because of that?

[jack-berg]

I'm not sure why this need to be marked as unstable

The argument is that its a new concept that has not been leveraged anywhere. If we mark it stable and get it wrong, we're stuck with it forever. OpenTelemetry tends to be conservative.

With that said, distributions are not a new concept in otel and we've been contending where to put distribution specific configuration for a while. I don't think we'll come with a different answer.

[trask]

is example the name of a distribution here? if not, should it be? e.g. in Java Instrumentation we have at least two (upstream) distributions agent and spring_starter, I could see it being helpful to know what kind of config yaml we're looking at

or if not, maybe distribution could have a (mandatory?) name:?

[ysolomchenko]

example is the name of a vendor. For example splunk, grafana, new_relic

[ysolomchenko]

I agree that a distribution cannot be empty and should have at least one vendor name inside it.

[jack-berg]

I pushed a commit that ensures this. It also ensures that each entry under distribution is an object, disallowing something like:

distribution:
  grafana: bar

Has to be modeled like:

distribution:
  grafana:
    property: bar

[jack-berg]

Looks good to me. @marcalff any thoughts before merging?

[jack-berg]

The plan is to create a separate PR in open-telemetry/opentelemetry-specification that extends the config provider

Currently, the config provider represents only the .instrumentation node, but it should also provide the .distribution node.

I think this isn't even necessary. Consider that a distribution normally controls the SDK initialization, and therefore has code that will call parse and create. So it would be perfectly reasonable for a distribution to get access to distribution from after parse and before create. In java, this would look something like:

# parse config file contents
OpenTelemetryConfigurationModel otelConfigModel = parse(configFileContents);

# grab the distribution section of config and hold a ref
DistributionModel distributionModel = otelConfigModel.getDistribution();

# initialize SDK components from the model
OpenTelemetrySdk sdk = create(otelConfigModel);

[robsunday]

I think this isn't even necessary.

So how about distributions that are based on OTel Java Agent, that just extend it with some custom component providers and AgentListeners? If AgentListener needs access to distribution config then it must get it from ConfigProvider. I'm not aware of any other reasonable way to expose distribution config to AgentListener.
Do you have other ideas?

I already created issue in opentelemtry-specification for planned ConfigProvider changes and I plan to create associated PR there soon.
We may move conversation to this issue now.

[jack-berg]

So how about distributions that are based on OTel Java Agent, that just extend it with some custom component providers and AgentListeners? For this scenario I'm not aware of any reasonable way to execute custom code between parsing and SDK setup (could be done in customizer, but it would be ugly).

The java agent would evolve to provide access to this. Nothing about the java agent's startup flow and callbacks is part of the spec - why would this be any different?

I already created issue in open-telemetry/opentelemetry-specification#4770 for planned ConfigProvider changes and I plan to create associated PR there soon.

To be clear, I'm not opposed to a spec for this, but I would be amazed if the java SIG didn't evolve its distribution callback APIs to accommodate this, with or without spec changes.

[zeitlinger]

The java agent would evolve to provide access to this. Nothing about the java agent's startup flow and callbacks is part of the spec - why would this be any different?

agree - we could have ExtendedAgentListener that has an additional parameter.
I'll add it to the notes of open-telemetry/opentelemetry-java-instrumentation#14080

Example: https://github.com/grafana/grafana-opentelemetry-java/blob/e01f090c6338df900b19eaa98c250b093a34fa03/custom/src/main/java/com/grafana/extensions/servertiming/ServerTimingHeaderActivator.java#L14

The most common extension interface is already good to go: DeclarativeConfigurationCustomizer.

Example (not merged): https://github.com/grafana/grafana-opentelemetry-java/blob/support-for-file-config-poc/custom/src/main/java/com/grafana/extensions/GrafanaDeclarativeConfigurationCustomizerProvider.java

[robsunday]

agree - we could have ExtendedAgentListener that has an additional parameter.

I think we need to solve this issue for other languages as well, not for Java only. ExtendedAgentListener is Java only solution.

Example: https://github.com/grafana/grafana-opentelemetry-java/blob/e01f090c6338df900b19eaa98c250b093a34fa03/custom/src/main/java/com/grafana/extensions/servertiming/ServerTimingHeaderActivator.java#L14

This is applicable only if distribution specific config is located under .instrumentation/development node. This is good for instrumentation related settings, but there may be something needed for other custom stuff that does not fit instrumentation category - that's the issue we try to solve by introducing .distribution node.
Adding one more method to ConfigProvider seems to be an easy and flexible solution. API specification for ConfigProvider contains a note - "TODO: decide if additional operations are needed to improve API ergonomics". I think distribution config access is a good candidate for this new operation.

[zeitlinger]

agree - we could have ExtendedAgentListener that has an additional parameter.

I think we need to solve this issue for other languages as well, not for Java only. ExtendedAgentListener is Java only solution.

It's only an example - and other languages can have other solutions.
I agree that it would be more natural to extend the ConfigProvider instead though.

This is good for instrumentation related settings, but there may be something needed for other custom stuff that does not fit instrumentation category

That's a great point - the Java agent has no way around that, unless the extend ConfigProvider.

[robsunday]

Do we need anything else to merge this PR?