-
Notifications
You must be signed in to change notification settings - Fork 937
refactor(otlp-exporter-base): use getStringFromEnv
instead of process.env
#5594
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
refactor(otlp-exporter-base): use getStringFromEnv
instead of process.env
#5594
Conversation
Updates the code to user the helper utilities to retrieve environment variable values fixes open-telemetry#5561 Signed-off-by: Weyert de Boer <[email protected]>
Signed-off-by: Weyert de Boer <[email protected]>
Signed-off-by: Weyert de Boer <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5594 +/- ##
=======================================
Coverage 95.06% 95.06%
=======================================
Files 307 307
Lines 8025 8025
Branches 1623 1623
=======================================
Hits 7629 7629
Misses 396 396
🚀 New features to boost your workflow:
|
getStringFromEnv
instead of process.env
getStringFromEnv
instead of process.env
Co-authored-by: David Luna <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! A couple nits.
experimental/packages/otlp-exporter-base/src/configuration/otlp-http-env-configuration.ts
Show resolved
Hide resolved
experimental/packages/otlp-exporter-base/src/configuration/otlp-http-env-configuration.ts
Outdated
Show resolved
Hide resolved
@weyert do you have time to address the changes proposed? I would be happy to merge this PR once done and the conflict is resolved. |
Please fix comments and conflicts and we can merge this :) |
…ess.env` (open-telemetry#5594) Signed-off-by: Weyert de Boer <[email protected]> Co-authored-by: David Luna <[email protected]> Co-authored-by: Trent Mick <[email protected]>
Which problem is this PR solving?
Updates the code to user the helper utilities to retrieve environment variable values
Fixes #5561
Short description of the changes
Used the
getStringFromEnv
-helper function instead ofprocess.env
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
I have ran the relevant unit tests to verify the tests still pass as expected.
Checklist: