Skip to content

Conversation

@maryliag
Copy link
Contributor

@maryliag maryliag commented Aug 19, 2025

Part of #5826

This PR focus on adding all attributes and corresponding setup of all environment variables required to migrate from Env Variable approach to a Declarative Config approach, based on the environment variables used here

@maryliag maryliag requested a review from a team as a code owner August 19, 2025 23:01
@codecov
Copy link

codecov bot commented Aug 19, 2025

Codecov Report

❌ Patch coverage is 98.85057% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.10%. Comparing base (fa286b4) to head (e2e9234).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...try-configuration/src/EnvironmentConfigProvider.ts 98.85% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5862      +/-   ##
==========================================
+ Coverage   95.02%   95.10%   +0.07%     
==========================================
  Files         315      315              
  Lines        8225     8396     +171     
  Branches     1666     1727      +61     
==========================================
+ Hits         7816     7985     +169     
- Misses        409      411       +2     
Files with missing lines Coverage Δ
...ges/opentelemetry-configuration/src/configModel.ts 100.00% <ø> (ø)
...try-configuration/src/EnvironmentConfigProvider.ts 98.94% <98.85%> (-1.06%) ⬇️
🚀 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.

@carlosalberto
Copy link

Hey! Sorry for the late review. So long story short, the Specification defines:

  1. Environment Variables configuration (which this PR adds support for).
  2. File based configuration (which is what covers ComponentProvider instances, which are defined mostly for custom components, e.g. MyCustomExporter). This configuration mode explicitly ignores the 1) section, although adding general env var interpolation in order to partially support migration.

As an example, in the Java ecosystem 1) and 2) exist as separate packages, although 1) uses 2) if it is present at runtime (if it detects the proper env var setting the configuration file). But that's mostly an implementation detail and is up to you to define them in the same or in separate packages, and it is your decision what to do organization wise.

The PR looks fine otherwise! Let me know if this clarifies a little bit the current situation.

@maryliag
Copy link
Contributor Author

maryliag commented Sep 9, 2025

Thank you for your input @carlosalberto! I talked with the group defining the spec for the config and we discussed how my implementation differs from the Java (two places vs having everything in one), and they actually said they like the way I'm doing and wished the others were doing it, so I agree is a matter of implementation, and the way I'm doing is still aligned with the specs

I'm trying to break down the PRs as most as possible to make it easier for review, so this one is still preparing things, are handling env variables, there is another one that starts the actual parsing of a config file, that you can find here #5875

@carlosalberto
Copy link

carlosalberto commented Sep 17, 2025

Symbolic approval, as I'm not an approver (went through the validation code and it all looks good).

@maryliag maryliag enabled auto-merge September 22, 2025 20:21
@maryliag maryliag added this pull request to the merge queue Sep 22, 2025
Merged via the queue into open-telemetry:main with commit e5adde2 Sep 22, 2025
25 checks passed
@maryliag maryliag deleted the config-env branch September 22, 2025 20:35
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.

3 participants