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

Fix: Force usage of case-sensitive keys in configurations #6876

Merged
merged 4 commits into from
Jan 20, 2023

Conversation

rapphil
Copy link
Contributor

@rapphil rapphil commented Jan 3, 2023

Signed-off-by: Raphael Silva [email protected]

Description: Fix bug that allowed users to specify invalid configurations that would pass validation but would not work properly or fail silently. E.g.: loglevel. If you specify logLevel, the validation would pass but the collector logging exporter would not produce logs.

Testing: Added unit test before changing the code.

@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Base: 90.64% // Head: 90.66% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (2d4e30d) compared to base (f9d87b5).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6876      +/-   ##
==========================================
+ Coverage   90.64%   90.66%   +0.02%     
==========================================
  Files         241      241              
  Lines       14523    14526       +3     
==========================================
+ Hits        13164    13170       +6     
+ Misses       1091     1089       -2     
+ Partials      268      267       -1     
Impacted Files Coverage Δ
confmap/confmap.go 90.12% <100.00%> (+0.18%) ⬆️
exporter/loggingexporter/config.go 74.19% <0.00%> (+9.67%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rapphil rapphil force-pushed the rapphil-fix-loglevel branch 2 times, most recently from bbf7182 to 923fd8e Compare January 3, 2023 08:38
@rapphil rapphil marked this pull request as ready for review January 3, 2023 08:41
@rapphil rapphil requested review from a team and Aneurysm9 January 3, 2023 08:41
Copy link
Member

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

I hope you'll permit a stranger to start getting involved with nitty reviews :)

.chloggen/case-sensitive-configuration.yaml Outdated Show resolved Hide resolved
confmap/confmap.go Outdated Show resolved Hide resolved
@Aneurysm9 Aneurysm9 added the ready-to-merge Code review completed; ready to merge by maintainers label Jan 19, 2023
@rapphil rapphil force-pushed the rapphil-fix-loglevel branch from e201ab8 to 2d4e30d Compare January 19, 2023 22:01
@bogdandrutu bogdandrutu merged commit 8fa8408 into open-telemetry:main Jan 20, 2023
bogdandrutu pushed a commit that referenced this pull request Jan 21, 2023
dmitryax pushed a commit that referenced this pull request Jan 23, 2023
codeboten pushed a commit to codeboten/opentelemetry-collector that referenced this pull request Jan 23, 2023
…etry#6876)

* Fix: Force usage of case-sensitive keys in configurations

Signed-off-by: Raphael Silva <[email protected]>

* Update confmap/confmap.go

Co-authored-by: Gabriel Aszalos <[email protected]>

* Update .chloggen/case-sensitive-configuration.yaml

Co-authored-by: Gabriel Aszalos <[email protected]>

* convert caseSensitiveMatchName to a plain function

Signed-off-by: Raphael Silva <[email protected]>

Signed-off-by: Raphael Silva <[email protected]>
Co-authored-by: Gabriel Aszalos <[email protected]>
codeboten pushed a commit to codeboten/opentelemetry-collector that referenced this pull request Jan 23, 2023
gbbr added a commit to gbbr/opentelemetry-collector that referenced this pull request Jan 25, 2023
bogdandrutu pushed a commit that referenced this pull request Jan 31, 2023
…ions (#6876)" (#6988)" (#7021)

* Revert "Revert "Fix: Force usage of case-sensitive keys in configurations (#6876)" (#6988)"

This reverts commit 80cabdd.

Closes #7002

* confmap: Remove case-insensitive statement in IsSet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants