Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[confmap] Add featuregate to use stable expansion rules #10391

Merged

Conversation

TylerHelmuth
Copy link
Member

@TylerHelmuth TylerHelmuth commented Jun 12, 2024

Description

This PR adds a feature gate that will handle transitioning users away from expandconverter, specifically expanding $VAR syntax.

The wholistic strategy is:

  1. create a new feature gate, confmap.unifyEnvVarExpansion, that will be the single feature gate to manage unifying collector configuraiton resolution.
  2. Update expandconverter to return an error if the feature gate is enabled and it is about to expand $VAR syntax.
  3. Update otelcol.NewCommand to set a DefaultScheme="env" when the feature gate is enabled and no DefaultScheme is set, this handles ${VAR} syntax.
  4. Separately, deprecate expandconverter.
  5. Follow a normal feature gate cycle.
  6. Removed the confmap.unifyEnvVarExpansion feature gates and expandconverter at the same time

Supersedes #10259

Link to tracking issue

Related to #10161
Related to #8215
Related to #7111

Testing

Unit tests

Copy link

codecov bot commented Jun 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.40%. Comparing base (6ca551e) to head (a31c099).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #10391   +/-   ##
=======================================
  Coverage   92.39%   92.40%           
=======================================
  Files         387      387           
  Lines       18310    18323   +13     
=======================================
+ Hits        16918    16931   +13     
  Misses       1046     1046           
  Partials      346      346           

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

confmap/resolver.go Outdated Show resolved Hide resolved
confmap/converter/expandconverter/expand.go Outdated Show resolved Hide resolved
confmap/resolver.go Outdated Show resolved Hide resolved
otelcol/command.go Outdated Show resolved Hide resolved
@TylerHelmuth TylerHelmuth requested review from codeboten and mx-psi June 12, 2024 20:46
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small change and it's good to go from my side

confmap/resolver.go Outdated Show resolved Hide resolved
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs rebasing

@mx-psi mx-psi merged commit ad2c979 into open-telemetry:main Jun 14, 2024
49 checks passed
@github-actions github-actions bot added this to the next release milestone Jun 14, 2024
@TylerHelmuth TylerHelmuth deleted the confmap-add-expand-featuregate-3 branch June 14, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants