Skip to content

Conversation

@weyert
Copy link
Contributor

@weyert weyert commented Apr 2, 2025

Which problem is this PR solving?

Migrated away from using process.env to use the getStringFromEnv-function

Fixes #5562

Short description of the changes

Replaced the usage of process.env with the built-in getEnv utility functions

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

weyert added 3 commits April 2, 2025 17:07
Migrated away from using `process.env` to use the `getStringFromEnv`-function

Signed-off-by: Weyert de Boer <wdb@innerfuse.biz>
…tFromEnv`-functions

Signed-off-by: Weyert de Boer <wdb@innerfuse.biz>
@codecov
Copy link

codecov bot commented Apr 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.07%. Comparing base (2d37608) to head (2137244).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5583      +/-   ##
==========================================
- Coverage   95.07%   95.07%   -0.01%     
==========================================
  Files         308      308              
  Lines        8039     8037       -2     
  Branches     1627     1626       -1     
==========================================
- Hits         7643     7641       -2     
  Misses        396      396              
Files with missing lines Coverage Δ
...base/src/configuration/shared-env-configuration.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@weyert weyert changed the title fix: use get*FromEnv() for otlp exporter config refactor: use get*FromEnv() for otlp exporter config Apr 6, 2025
Signed-off-by: Weyert de Boer <wdb@innerfuse.biz>
@weyert weyert marked this pull request as ready for review April 6, 2025 14:39
@weyert weyert requested a review from a team as a code owner April 6, 2025 14:39
@maryliag maryliag changed the title refactor: use get*FromEnv() for otlp exporter config refactor(otlp-exporter-base): use get*FromEnv() for otlp exporter config Apr 7, 2025
trentm
trentm previously requested changes Apr 9, 2025
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

I took the liberty to address the remaining comments and resolve conflicts. This should be good to merge now.

@pichlermarc pichlermarc requested a review from trentm September 9, 2025 09:21
@pichlermarc pichlermarc dismissed trentm’s stale review September 10, 2025 11:24

requested changes were addressed.

@pichlermarc pichlermarc added this pull request to the merge queue Sep 10, 2025
Merged via the queue into open-telemetry:main with commit a9fc9b8 Sep 10, 2025
25 checks passed
@otelbot-js otelbot-js bot mentioned this pull request Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[contribfest] use new get*FromEnv() functions in OTLP exporter shared env var configuration

4 participants