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

Configuration: support allowlists #2546

Merged
merged 7 commits into from Apr 12, 2024
Merged

Configuration: support allowlists #2546

merged 7 commits into from Apr 12, 2024

Conversation

fallwith
Copy link
Contributor

@fallwith fallwith commented Apr 10, 2024

Configuration hashes defined by default_source.rb now support an
additional :allowlist key set equal to an array of valid values.

If an invalid value is specified for a parameter that has an allowlist,
a warning will be logged that contains the allowlist and the default
value for the parameter will be used.

resolves #2555

Configuration hashes defined by `default_source.rb` now support an
additional `:allowlist` key set equal to an array of valid values.

If an invalid value is specified for a parameter that has an allowlist,
a warning will be logged that contains the allowlist and the default
value for the parameter will be used.
Copy link
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

Thank you, James! I like this idea. Like we talked about in standup today, it could have more applications, such as:

  • Updating our documentation (config, newrelic.yml) to display allowlists so that we don't need to include them in the description text
  • Revisiting the instrumentation.* settings that permit only auto, prepend, chain, disabled

config/database.yml Outdated Show resolved Hide resolved
lib/new_relic/agent/configuration/manager.rb Outdated Show resolved Hide resolved
::NewRelic -> NewRelic
Remove the class specific config validation that is now redundant with
the config's own allowlist validation
Now that the config handles the allowlist validation, remove the test
expecting the 8T class to do so
Copy link

SimpleCov Report

Coverage Threshold
Line 93.77% 93%
Branch 71.48% 71%

@fallwith fallwith merged commit c71bc05 into dev Apr 12, 2024
30 checks passed
@fallwith fallwith deleted the adamantine_forge branch April 12, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Internal feature request: support allowlist type configuration parameter value validation
2 participants