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] Set confmap.unifyEnvVarExpansion feature gate to stable #10508

Merged
merged 20 commits into from
Aug 7, 2024

Conversation

TylerHelmuth
Copy link
Member

@TylerHelmuth TylerHelmuth commented Jul 2, 2024

Description

This PR promotes the confmap.unifyEnvVarExpansion feature gate to stable and sets a ToVersion of v0.106.0, anticipating that the gate be completely removed in that version.

We should weigh if switching the Stable should be done in v0.105.0 or if it needs more time in Beta to give users more time to switch. Delaying promotion to Stable delays confmap 1.0.

If we merge this we need to commit to merging #10510 in the same release.

Link to tracking issue

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

Copy link

codecov bot commented Jul 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.37%. Comparing base (420bf36) to head (bebb922).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10508      +/-   ##
==========================================
- Coverage   91.64%   91.37%   -0.27%     
==========================================
  Files         406      406              
  Lines       18985    19007      +22     
==========================================
- Hits        17398    17368      -30     
- Misses       1227     1282      +55     
+ Partials      360      357       -3     

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

Copy link
Member Author

@TylerHelmuth TylerHelmuth Jul 2, 2024

Choose a reason for hiding this comment

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

Making the feature gate stable negates these tests, which set the gate to false. Since the module is being deprecated, and will be removed shortly after, removing the tests doesn't feel like a big deal.

@TylerHelmuth
Copy link
Member Author

Let me know if you'd rather this PR and #10510 be merged into 1. If we needed to roll back for some reason 1 PR would be easier.

@mx-psi
Copy link
Member

mx-psi commented Jul 8, 2024

I think we can wait at least until #10554 has been out for one release

Copy link
Contributor

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

@github-actions github-actions bot added the Stale label Jul 23, 2024
@ben-childs-docusign
Copy link

Hi Folks - I just logged this issue it seems like there is still an issue with configurations that use "$$SomeString" syntax to represent the string literal "$SomeString" with this feature flag enabled it is still blocked on startup with version 105.

#10713

@github-actions github-actions bot removed the Stale label Jul 24, 2024
@mx-psi
Copy link
Member

mx-psi commented Aug 2, 2024

@ben-childs-docusign Thanks for the report. Have you tried v0.106.1? The fix for that issue was released on v0.106.0

@ben-childs-docusign
Copy link

Hi @mx-psi yes we deployed the patch and it is working. thanks for the fix!

confmap/resolver.go Outdated Show resolved Hide resolved
@mx-psi mx-psi added the release:blocker The issue must be resolved before cutting the next release label Aug 5, 2024
confmap/internal/e2e/expand_test.go Outdated Show resolved Hide resolved
.chloggen/unify-env-var-gate-stable.yaml Outdated Show resolved Hide resolved
confmap/converter/expandconverter/go.mod Outdated Show resolved Hide resolved
confmap/internal/e2e/expand_test.go Outdated Show resolved Hide resolved
confmap/internal/e2e/expand_test.go Outdated Show resolved Hide resolved
TylerHelmuth and others added 4 commits August 6, 2024 13:47
Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com>
Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com>
@mx-psi
Copy link
Member

mx-psi commented Aug 7, 2024

I am going to wait to merge this until #10510 is approved

@dmitryax dmitryax merged commit 91d6525 into open-telemetry:main Aug 7, 2024
50 checks passed
@github-actions github-actions bot added this to the next release milestone Aug 7, 2024
@TylerHelmuth TylerHelmuth deleted the unify-env-var-gate-stable branch August 7, 2024 20:40
dmitryax added a commit that referenced this pull request Aug 7, 2024
#### Description
This PR deprecates `expandconverter` and removes its use from
`otelcoltest.LoadConfig` and OCB.

This cannot be merged until the `confmap.unifyEnvVarExpansion` [feature
gate is made
stable](#10508).

<!-- Issue number if applicable -->
#### Link to tracking issue
closes
#10161
closes
#7111
closes
#8215

---------

Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
Co-authored-by: Dmitrii Anoshin <anoshindx@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:blocker The issue must be resolved before cutting the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants