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/provider/secretsmanagerprovider] Add support for default values #37535

Merged
merged 1 commit into from
Mar 11, 2025

Conversation

theSuess
Copy link
Contributor

Description

This PR adds support for default values when using the secretsmanager provider.
It uses the same syntax as the envprovider and falls back to the default value when:

  • the selector is empty
  • the json key is not found in the secret map

In both cases, a warning is logged to call out unintentional fallback.

By nesting substitutions, this allows users to specify either a secret manager reference or plain value through environment variables like this:

password: "${secretsmanager:${env:PASSWORD_ARN}:-${env:PASSWORD}}"

When PASSWORD_ARN is set, the collector retrieves the value from AWS Secrets Manager, falling back to the value specified in PASSWORD if the secret is not found or the json key in PASSWORD_ARN is undefined.

Testing

  • Manual testing performed using a custom build of the opentelemetry-lambda extension layer.
  • Added unit tests

Documentation

  • Provider documentation & go API docs

@theSuess theSuess force-pushed the feat/secretmanager-defaults branch from aad8ada to 2ec0176 Compare January 28, 2025 08:38
@theSuess theSuess marked this pull request as ready for review January 28, 2025 08:38
@theSuess theSuess requested review from atoulme and a team as code owners January 28, 2025 08:38
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 Feb 12, 2025
@theSuess
Copy link
Contributor Author

@atoulme @dashpole this is still relevant - would appreciate a review

@dashpole
Copy link
Contributor

cc codeowners @driverpt @atoulme

@github-actions github-actions bot removed the Stale label Feb 13, 2025
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 Feb 28, 2025
@theSuess theSuess force-pushed the feat/secretmanager-defaults branch from 2ec0176 to f4ee8f7 Compare February 28, 2025 09:21
@theSuess
Copy link
Contributor Author

Still relevant, rebased onto latest main now

@github-actions github-actions bot removed the Stale label Mar 1, 2025
@atoulme atoulme added the ready to merge Code review completed; ready to merge by maintainers label Mar 6, 2025
Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

Tests and linter are failing, please fix

@songy23 songy23 removed the ready to merge Code review completed; ready to merge by maintainers label Mar 7, 2025
@theSuess theSuess force-pushed the feat/secretmanager-defaults branch 2 times, most recently from a6f590f to fa92974 Compare March 11, 2025 08:51
@theSuess theSuess force-pushed the feat/secretmanager-defaults branch from fa92974 to 08dc473 Compare March 11, 2025 08:56
@theSuess
Copy link
Contributor Author

Rebased the PR and fixed the previous linter issues

@theSuess theSuess requested a review from songy23 March 11, 2025 08:56
@songy23 songy23 merged commit da5e29b into open-telemetry:main Mar 11, 2025
170 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants