[cmd/builder] Allow configuring confmap providers

[evan-bradley]

Description:

Allow configuring confmap providers in the builder's config. If the field isn't set, the default set of providers is used.

Link to tracking Issue:

Resolves #4759.

Testing:

Extended unit tests.

Documentation:

Updated the readme to include the new options in the example manifest file.

cc @mx-psi

[codecov]

Codecov Report

Attention: Patch coverage is 68.29268% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 91.63%. Comparing base (7b046d9) to head (6ef69ce).

Files	Patch %	Lines
cmd/builder/internal/builder/config.go	75.75%	4 Missing and 4 partials ⚠️
cmd/otelcorecol/main.go	0.00%	4 Missing ⚠️
cmd/builder/internal/command.go	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9513      +/-   ##
==========================================
- Coverage   91.65%   91.63%   -0.03%     
==========================================
  Files         360      360              
  Lines       16639    16676      +37     
==========================================
+ Hits        15251    15281      +30     
- Misses       1053     1058       +5     
- Partials      335      337       +2     

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

[evan-bradley]

Making these optional introduces a lot of code complexity, but is necessary unless we want to take one of two routes:

1. Remove the ability for users to specify none of either and check whether the set of modules is empty. In my opinion, we should let users specify they want nothing included in their distribution in case they have unusual requirements.
2. Remove the set of default converters/providers and require the user to always specify the modules they want included.

I don't necessarily mind option 2, but it would be a fairly significant breaking change and would need a long transition period. If we take option 2, we could move the defaults list to otelcol and just always have the modules baked into every distro until we complete the transition.

[mx-psi]

An alternative is to have a custom unmarshaling function that differentiates between unset and set but empty. I think this is cleaner and I agree that even if we go with 2 we would want to have a transition period

[mx-psi]

Both dependencies have been merged, is this ready for review?

[evan-bradley]

@mx-psi I think this is ready to go, please take a look.

[mx-psi]

We don't have a clear policy regarding how big the difference in versions between the builder and the API used is, but given the precedent with #8692, I think we should strive to keep some compatibility with older versions of the API, at least for some amount of time. To that end we should guard uses of new modules or new API with a check of the otelcol_version

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[mx-psi]

Will wait for #9516 before giving another pass to this one

[evan-bradley]

Will wait for #9516 before giving another pass to this one

I've also been waiting for that, I will let you know once I update this PR to include those changes.

[evan-bradley]

@mx-psi I've updated this to use confmap factories and to account for #9897.

I've updated this to not use the new APIs if the builder is working with versions before v0.99.0. It looks like the new strict versioning functionality will force the builder and core API versions to be equal, according to this line. Should we expect that when the flag to skip strict versioning is removed we can also remove the v0.99.0 version check?

[mx-psi]

@evan-bradley I did not intend for that change to go through just yet, and this is also not documented, so I filed #9999 to document it. We can handle this separately.

IMO we should be a bit more lenient and allow for some version skew between the builder and the Collector being built (at least one minor version of difference should be fine based on our deprecation guidelines, and would be in line with the Kubernetes version skew policy)

[evan-bradley]

Thanks for the clarification. I agree we should probably allow some minor skew even if it's a bit inconvenient, it would feel pretty demanding to have to update the builder every single release.

I think this PR should be unaffected by that change and should still be ready to go.

[mx-psi]

Will merge on Monday morning

@MovieStoreGuy block before then if you still have any concerns