Require spec changes to consider declarative config schema

[jack-berg]

With declarative configuration stable (#4374), I propose adjusting the spec contribution process to be "declarative config first".

What this means: If a spec PR is proposing changes to an SDK component's configuration surface area, it should include a reference to an PR against opentelemetry-configuration to propose corresponding changes to the declarative config schema.

Why this is important:

• Ensures spec PRs are evaluated holistically. The declarative config schema is on track to become a crucial cross-language user facing API for the whole project. Operators can't or don't want to modify source code to use the programmatic API, and env vars are insufficient for non-trivial setups. By doing this, we add UX evaluation criteria in addition to the existing conceptual criteria.
• Ensures declarative config schema stays in sync with the spec.
• Helps drive consistency across language implementations, since its much harder to misinterpret / reinterpret a feature when there's a config schema spec to adhere to.
• Further incentivizes the remaining languages to implement declarative config, helping the project achieve its mission of telemetry concepts being universal across languages.

cc @open-telemetry/configuration-approvers

[dyladan]

I wonder if we're at a point that it makes sense to merge the config into the spec? I think it is likely a lot easier to keep things in sync if a single PR can make both changes. Otherwise we can run into a deadlock situation where spec is waiting for config and config is waiting for spec.

[jack-berg]

I wonder if we're at a point that it makes sense to merge the config into the spec?

In similar situations in the past, we've tended to move logical chunks out of the spec rather than into it. E.g. semantic conventions, OTLP. However, we did bring OTEPs into the spec from a separate repo.

Otherwise we can run into a deadlock situation where spec is waiting for config and config is waiting for spec.

It shouldn't be deadlock it we treat synchronizing config changes as guidance rather than a mandate. Related convo here.

[dyladan]

In similar situations in the past, we've tended to move logical chunks out of the spec rather than into it. E.g. semantic conventions, OTLP. However, we did bring OTEPs into the spec from a separate repo.

I think the main difference between config and OTLP/SemConv is that changes to the spec are not blocked on required changes in those places. Spec defines a data model/SDK/API and the proto is an implementation of the spec same as the language client libraries. This change proposes that config is required for spec, which implies that it is a part of the spec, not an implementation of it. In that way it is more similar to OTEPs.

[jack-berg]

This change proposes that config is required for spec

That's not my intent and I'll need to adjust the language if that's not clear. I view the relationship as similar to the prototyping requirement for spec PRs. "Prove that this idea can be reasonably implemented in the config schema and what that would look like so that we can properly evaluate it"

There could be merit in strengthening this and having a strict coupling like you're suggesting. The main downside to that I see is that at this point, the spec maintainers (i.e. the TC) aren't experts in the opentelemetry-configuration tooling - i.e. JSON schema and some custom scripting for compiling and validating the schema.

[carlosalberto]

In similar situations in the past, we've tended to move logical chunks out of the spec rather than into it. E.g. semantic conventions, OTLP. However, we did bring OTEPs into the spec from a separate repo.

I'm imagining a new user who has a nice idea and the time to help with it, but suddenly he has to work with TWO repos. Imagine if he adds something to semconv too? That's working with THREE repositories, with potentially different people).

(Not saying we should put everything back, but I think we should understand the ramifications. For somebody involved in OTel it won't be hopefully hard to keep track of related PRs, but somebody may suffer some pain).

[dyladan]

Approving this because I think the core idea of "spec changes must consider config" is a good one. The fact it is a separate repo is an implementation detail for now that can be changed, or not, later if desired.

As far as it raising the barrier to spec contribution by stipulating spec authors must learn config process and tooling, I think that's a completely reasonable barrier to erect. In the past it has always ben the de facto process that spec authors create their own config spec by adding env vars and such where appropriate, it was just less formalized.

[marcalff]

• Helps drive consistency across language implementations, since its much harder to misinterpret / reinterpret a feature when there's a config schema spec to adhere to.

+1 here

[github-actions]

This PR was marked stale. It will be closed in 14 days without additional activity.

[jack-berg]

@open-telemetry/technical-committee please take a look. Since this is a point of process, I think this is primarily a TC concern. I don't believe there are any blockers since I opened #4935 to decouple the conversation about whether the configuration data model should be merged into the spec.

[carlosalberto]

Approving as I think this is the right direction - whether the config repository is merged into the Specification is something we can tune if/when needed.

[jack-berg]

Will merge after the 4/7 spec SIG if there are no additional comments.

[jack-berg]

Added 9b916d5 to update the PR template, which was a good recommendation @dashpole made in the 4/7 spec SIG