opentelemetry-collector-contrib: Consider marking changes to config struct fields as non-breaking

Some ideas that were floated around on Slack (putting emojis in case we want to make a poll at some point):

Option A. 🎉 Have a separate changelog for Go API only changes

We create a separate changelog file that lists changes in the Go API that don’t result in user facing changes for users of the Collector as a binary. The rationale for this is that most people don’t care about Go API only changes and it can be confusing to have them on the main changelog.

The main downside of this option is that it complicates the contribution process while we don’t know if people really care about the Go API at all.

Option B. ❤️ Have a separate changelog section for Go API only changes

Same as Option A, but instead of a separate file we can have a separate section on the main changelog for Go API only changes. This still complicates the contribution process, but it’s a bit simpler at the expense of possibly being more confusing.

Option C. 🚀 Move configuration structs to internal packages on each component’s module

We move the Config structs from all components to an internal package, so that they stop being part of the public API. This aligns the Go API stability with the usual stability concept. Creating config structs remains usable if using CreateDefaultConfig + Unmarshal to create a configuration struct. The concern here becomes a nonissue then, since this is no longer part of the public Go API. An open question is if we can still support all use cases if the configuration structures are internal (e.g. OpAMP).

Option D. 👀 Explicitly call out that we make no stability guarantees for Go API on contrib components

Change VERSIONING.md to state that Go API is not considered when it comes to stability guarantees. We won’t list the Go API only changes on the changelog going forward.

IMO, there is 3 separate audiences that care about stability of the Collector and they care about different things.

One is end users. They don’t care about API stability, they care about config YAML format and component behavior.

The second audience is component developers, who care about core API stability (e.g. pdata, consumer, etc) their component depends on. They don’t care much about other component APIs or the config YAML format.

The third audience is Collector maintainers who care about all of these things.

Our changelog indeed is a mix of all of these concerns. I would vote for some form of changelog separation by audience, such that we inform the particular audience about only relevant things that impact them. This probably would be beneficial regardless of whether we think config struct should be part of public API of the component or no.

Discussion in this thread has led me to change my opinion to Option A. In short, I believe Option B does not correctly model the information we should be conveying. If you voted on this issue, please consider reviewing the discussion.

I’ve opened an issue here proposing a solution that would support that option.

🚀 would mean that the only way to change the configuration is through Unmarshal indeed.

This means we are preventing the compiler from helping us to know we broke some code that depends on the config structure. It is not preventing the breakage. The dependent code will still need to make fixes by adjusting whatever the expect to get as a result of unmarshalling.

🚀: doesn’t it mean that people can’t change the config returned by CreateDefaultConfig? The interface returns a generic Config construct, and casting to an internal type wouldn’t work, I suppose. Unmarshal would work, but that’s really cumbersome for people who are creating the component programmatically, without an external config source.

This would be a mistake in my opinion. There are sensible reasons to directly modify the config, some of which are useful in this repo. e.g:

• Lifecycle tests
• Integration tests

Preference for ❤️, although 🎉 is also acceptable. I’m not sure I fully understand 🚀: doesn’t it mean that people can’t change the config returned by CreateDefaultConfig? The interface returns a generic Config construct, and casting to an internal type wouldn’t work, I suppose. Unmarshal would work, but that’s really cumbersome for people who are creating the component programmatically, without an external config source.

That said, can you expand on how ❤️ complicates the contribution process? Wouldn’t it work if we add a new field to the changelog YAMLs, stating whether that’s a change affecting consumers of the API or users of the collector?

Option A has been implemented in the build-tools repo and #24771 would set up this repo to use it.

I think https://github.com/open-telemetry/opentelemetry-go-build-tools/pull/360 might be confusing for go instrumentation library since any change there is a library change. Also, most of the user-facing changes change the go API. Are we going to duplicate the entries?

My intention is that only OpenTelemetry Collector contrib would use this change_type, and only for library only changes. If you can think of a better name, could you comment on the PR?

I think we can keep the collector changelog end-user centric, and if a user interface enhancement breaks the Go API, we add a suffix like [Breaking change to Go API] under the Enhancements section.

I slightly like more the new change_type since that is machine-readable and easy to check on CI (as in, if a change breaks the Go API, we can eventually validate that it has the right changelog type).