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

Centralize configuration defintion #1399

Merged
merged 5 commits into from
Nov 7, 2022

Conversation

bizob2828
Copy link
Member

@bizob2828 bizob2828 commented Nov 1, 2022

Proposed Release Notes

  • Updated configuration system to automatically create an environment variable mapping for a new config value.
    • It will follow a convention of NEW_RELIC_PATH_TO_CONFIG_KEY.
    • For example if there is a new configuration option of config.nested.object_path.enabled the env var would be NEW_RELIC_NESTED_OBJECT_PATH.ENABLED.

Links

Closes NEWRELIC-4964

Details

For internal devs, I centralized configuration defintion to really 2 places now instead of the 4.

In lib/config/default.js you will define the object structure, assign a default value(s) and an env var formatter as all env vars are strings. There's also an env var override but I think this should only exist for legacy options as our configuration values follow a convention specified above. You will still have to add a case in _fromServer if the new configuration is a server side option.

… values,

        respective default, an env var formatter and env var override.  refactored config system to derive env vars from leaf node structure.
@codecov
Copy link

codecov bot commented Nov 1, 2022

Codecov Report

Merging #1399 (de1e4a8) into main (3ae61bf) will decrease coverage by 3.45%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1399      +/-   ##
==========================================
- Coverage   95.10%   91.64%   -3.46%     
==========================================
  Files         193      173      -20     
  Lines       37640    35842    -1798     
  Branches       23       23              
==========================================
- Hits        35796    32846    -2950     
- Misses       1844     2996    +1152     
Flag Coverage Δ
esm-unit-tests-14.x 47.11% <ø> (ø)
esm-unit-tests-16.x 92.21% <ø> (ø)
esm-unit-tests-18.x 92.21% <ø> (ø)
integration-tests-14.x 76.60% <96.48%> (+0.02%) ⬆️
integration-tests-16.x 76.70% <96.48%> (+0.01%) ⬆️
integration-tests-18.x 76.71% <96.48%> (+0.02%) ⬆️
unit-tests-14.x 89.78% <100.00%> (+0.04%) ⬆️
unit-tests-16.x 89.85% <100.00%> (+0.03%) ⬆️
unit-tests-18.x 89.83% <100.00%> (+0.03%) ⬆️
versioned-tests-14.x ?
versioned-tests-16.x ?
versioned-tests-18.x ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/config/default.js 100.00% <100.00%> (ø)
lib/config/formatters.js 100.00% <100.00%> (ø)
lib/config/index.js 94.06% <100.00%> (+0.32%) ⬆️
lib/system-info.js 64.70% <100.00%> (-0.16%) ⬇️
lib/instrumentation/mysql/mysql.js 8.60% <0.00%> (-81.56%) ⬇️
lib/instrumentation/grpc-js/grpc.js 17.72% <0.00%> (-74.55%) ⬇️
lib/instrumentation/hapi/hapi.js 22.70% <0.00%> (-73.92%) ⬇️
lib/instrumentation/pino/pino.js 26.97% <0.00%> (-73.03%) ⬇️
lib/instrumentation/amqplib.js 19.83% <0.00%> (-69.98%) ⬇️
lib/instrumentation/pg.js 46.29% <0.00%> (-50.93%) ⬇️
... and 42 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

lib/config/default.js Outdated Show resolved Hide resolved
test/unit/config/config-formatters.test.js Outdated Show resolved Hide resolved
test/unit/config/config-formatters.test.js Show resolved Hide resolved
jmartin4563
jmartin4563 previously approved these changes Nov 4, 2022
mrickard
mrickard previously approved these changes Nov 4, 2022
@bizob2828 bizob2828 dismissed stale reviews from mrickard and jmartin4563 via de1e4a8 November 4, 2022 19:53
@bizob2828 bizob2828 merged commit c4d0c1e into newrelic:main Nov 7, 2022
@github-actions github-actions bot mentioned this pull request Nov 9, 2022
@bizob2828 bizob2828 deleted the config-refactor branch August 28, 2024 18:18
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.

3 participants