opentelemetry-collector
opentelemetry-collector copied to clipboard
[configtls] mark module as stable
Description
Mark module as stable.
Link to tracking issue
Fixes #9377
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 92.36%. Comparing base (
72c23aa) to head (addd9c4).
Additional details and impacted files
@@ Coverage Diff @@
## main #10344 +/- ##
=======================================
Coverage 92.36% 92.36%
=======================================
Files 386 386
Lines 18402 18402
=======================================
Hits 16997 16997
Misses 1052 1052
Partials 353 353
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Reviewing this PR inspired https://github.com/open-telemetry/opentelemetry-collector/pull/10358, that removes the dependency on confmap from this configtls
Thank you @codeboten , indeed the dependency on confmap was only through tests.
@mx-psi @songy23 @atoulme One thing to keep in mind is that this package does expose mapstructure tags for the struct, which would mean that we'd have to keep support for them around for the life of this package. Is this ok? @TylerHelmuth reminded me that this was the reason we'd held on stabilizing other config packages until confmap is stable
@codeboten Right, I forgot about this. I don't see us changing mapstructure tags for something else but it makes sense to discuss explicitly. Should we file an issue about this? (We don't have one, do we?)
@mx-psi i think only issue that comes close is this one around how mapstructure is unmarshaled https://github.com/open-telemetry/opentelemetry-collector/issues/9532, i don't know if we want another issue that specific talks about which tags should be supported moving forward?
Is changing tags on a struct field a breaking change? The go API is unchanged, right?
@atoulme it's currently listed in versioning.md as breaking change https://github.com/open-telemetry/opentelemetry-collector/blob/main/VERSIONING.md#configuration-structures
We had some discussion about bumping MinTLS to 1.3. I think this is something that is not a breaking change given that the Go team themselves have proposed changes of this sort in the past (see golang/go#45428) and this being considered a security best practice.
I am not sure how we should best clarify this on VERSIONING.md, but I think this can be dealt with independently from this PR and that we can merge this now.