Skip to content

Conversation

@brettmc
Copy link
Contributor

@brettmc brettmc commented Aug 26, 2024

implement recent additions to the otel spec re: config provider. Config provider should return a ConfigProperties, but we were already returning a ConfigurationRegistry. Added a ConfigProperties interface for ConfigurationRegistry to implement, which gets us pretty close to spec. We don't make ConfigProvider globally available, instead passing the ConfigurationRegistry in to each instrumentation as we register it. The spec allows for this.

@brettmc brettmc requested a review from a team August 26, 2024 02:01
@codecov
Copy link

codecov bot commented Aug 26, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 74.06%. Comparing base (69825d3) to head (20ad747).
Report is 3 commits behind head on main.

Files Patch % Lines
src/SDK/SdkAutoloader.php 75.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1366      +/-   ##
============================================
+ Coverage     73.87%   74.06%   +0.18%     
- Complexity     2641     2664      +23     
============================================
  Files           379      383       +4     
  Lines          7545     7634      +89     
============================================
+ Hits           5574     5654      +80     
- Misses         1971     1980       +9     
Flag Coverage Δ
8.1 73.69% <87.50%> (+0.13%) ⬆️
8.2 73.93% <87.50%> (+0.13%) ⬆️
8.3 73.93% <87.50%> (+0.09%) ⬆️
8.4 74.02% <87.50%> (+0.16%) ⬆️

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

Files Coverage Δ
...rc/API/Configuration/Noop/NoopConfigProperties.php 100.00% <100.00%> (ø)
src/API/Configuration/Noop/NoopConfigProvider.php 100.00% <100.00%> (ø)
...tion/AutoInstrumentation/ConfigurationRegistry.php 100.00% <ø> (ø)
src/SDK/SdkAutoloader.php 93.51% <75.00%> (-5.48%) ⬇️

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69825d3...20ad747. Read the comment docs.

implement recent additions to the otel spec re: config provider. Config provider should return a
ConfigProperties, but we were already returning a ConfigurationRegistry. Added a ConfigProperties
interface for ConfigurationRegistry to implement, which gets us pretty close to spec.
We don't make ConfigProvider globally available, instead passing the ConfigurationRegistry in to
each instrumentation as we register it. The spec allows for this.
Copy link
Contributor

@ChrisLightfootWild ChrisLightfootWild left a comment

Choose a reason for hiding this comment

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

LGTM!

Co-authored-by: Chris Lightfoot-Wild <[email protected]>
@brettmc brettmc merged commit 653e453 into open-telemetry:main Aug 26, 2024
@brettmc brettmc deleted the config-provider branch January 29, 2025 21:51
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.

2 participants