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.

Sign up for GitHub

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

Jump to bottom

[confmap] Set confmap.unifyEnvVarExpansion feature gate to stable #10508

Merged
merged 20 commits into from
Aug 7, 2024
Merged

[confmap] Set confmap.unifyEnvVarExpansion feature gate to stable #10508

merged 20 commits into from
Aug 7, 2024

Conversation

TylerHelmuth
Copy link
Member

@TylerHelmuth TylerHelmuth commented Jul 2, 2024 •
edited
Loading

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

@TylerHelmuth TylerHelmuth requested review from a team and evan-bradley July 2, 2024 15:53
@TylerHelmuth TylerHelmuth mentioned this pull request Jul 2, 2024
Merged
@codecov Codecov
Copy link

codecov bot commented Jul 2, 2024 •
edited
Loading

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.

TylerHelmuth
TylerHelmuth commented Jul 2, 2024
View reviewed changes
confmap/converter/expandconverter/expand_test.go Outdated
Copy link
Member Author

@TylerHelmuth TylerHelmuth Jul 2, 2024 •
edited
Loading

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

@github-actions GitHub Actions
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!

dmitryax
dmitryax reviewed Aug 5, 2024
View reviewed changes
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
@TylerHelmuth TylerHelmuth mentioned this pull request Aug 6, 2024
Closed
dmitryax
dmitryax approved these changes Aug 6, 2024
View reviewed changes
confmap/internal/e2e/expand_test.go Outdated Show resolved Hide resolved
evan-bradley
evan-bradley approved these changes Aug 6, 2024
View reviewed changes
.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
@TylerHelmuth @evan-bradley
Apply suggestions from code review
fd8ca4b 
Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com>
@TylerHelmuth
Merge branch 'unify-env-var-gate-stable' of https://github.com/TylerH…
95126d3 
…elmuth/opentelemetry-collector into unify-env-var-gate-stable
@TylerHelmuth @evan-bradley
Update confmap/internal/e2e/expand_test.go
54cddc3 
Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com>
@TylerHelmuth
Merge branch 'main' into unify-env-var-gate-stable
87fa185
@mx-psi
Copy link
Member

mx-psi commented Aug 7, 2024

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

@TylerHelmuth TylerHelmuth requested a review from codeboten August 7, 2024 16:51
@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
@TylerHelmuth @codeboten @dmitryax
[confmap] Deprecate expandconverter (#10510)
2b9697f 
#### 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>
@dmitryax dmitryax mentioned this pull request Aug 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evan-bradley evan-bradley evan-bradley approved these changes

@dmitryax dmitryax dmitryax approved these changes

@mx-psi mx-psi mx-psi approved these changes

@codeboten codeboten Awaiting requested review from codeboten

Assignees
No one assigned
Labels
release:blocker The issue must be resolved before cutting the next release
Projects
None yet
Milestone
v0.107.0
Development

Successfully merging this pull request may close these issues.
5 participants
@TylerHelmuth @mx-psi @ben-childs-docusign @dmitryax @evan-bradley