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

Move DefaultConfigProperties to autoconfigure-spi #5001

Merged

Conversation

jack-berg
Copy link
Member

Related to #4949.

As SPI implementations are added in various modules, the test code should follow, which requires some implementation of the ConfigProperties interface.

This PR moves DefaultConfigProperties to an internal package of :sdk-extensions:autoconfigure-spi. This gives implementers of SPIs (including us) a nice test utility, so long as they're willing to accept that the APIs may change.

@jack-berg jack-berg requested a review from a team November 29, 2022 19:20
static DefaultConfigProperties customize(
DefaultConfigProperties previousProperties, Map<String, String> overrides) {
return new DefaultConfigProperties(previousProperties, overrides);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Adjusted this to be an instance method called withOverrides. Also renamed static DefaultConfigProperties get to create, so we now have consistently named factory methods for creating instances.

@codecov
Copy link

codecov bot commented Nov 29, 2022

Codecov Report

Base: 91.23% // Head: 91.27% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (d154cc3) compared to base (e79aad8).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5001      +/-   ##
============================================
+ Coverage     91.23%   91.27%   +0.03%     
- Complexity     4865     4866       +1     
============================================
  Files           552      552              
  Lines         14399    14399              
  Branches       1373     1373              
============================================
+ Hits          13137    13142       +5     
+ Misses          873      871       -2     
+ Partials        389      386       -3     
Impacted Files Coverage Δ
.../autoconfigure/LogRecordExporterConfiguration.java 98.50% <ø> (ø)
.../sdk/autoconfigure/MeterProviderConfiguration.java 77.27% <ø> (ø)
...y/sdk/autoconfigure/SpanExporterConfiguration.java 100.00% <ø> (ø)
...onfigure/spi/internal/DefaultConfigProperties.java 94.11% <100.00%> (ø)
...nfigure/AutoConfiguredOpenTelemetrySdkBuilder.java 97.31% <100.00%> (ø)
...metry/sdk/logs/export/BatchLogRecordProcessor.java 90.07% <0.00%> (+0.70%) ⬆️
...y/exporter/internal/marshal/CodedOutputStream.java 71.00% <0.00%> (+1.18%) ⬆️
...metry/sdk/metrics/export/PeriodicMetricReader.java 90.00% <0.00%> (+2.85%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jack-berg jack-berg merged commit 2a231d7 into open-telemetry:main Nov 30, 2022
dmarkwat pushed a commit to dmarkwat/opentelemetry-java that referenced this pull request Dec 30, 2022
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