Skip to content

Conversation

@jack-berg
Copy link
Member

@jack-berg jack-berg commented Apr 10, 2024

Implements spec PR: open-telemetry/opentelemetry-specification#3877

Replaces prototype PR #6201. Fixes #6197.

To simplify reviewing, I've added all the APIs to the public API surface area. Will plan on moving them to internal packages before merging, but don't want to muddy the waters with reflective access.

For examples of usage see:

  • ScopeConfiguratorTest
  • LoggerConfigTest
  • TracerConfigTest
  • MeterConfigTest

Simple demonstration. The API in this is aspirational - we won't be able to achieve it until the spec is stabilized. For example of the actual API, see ScopeConfiguratorTest.

OpenTelemetrySdk sdk = OpenTelemetrySdk.builder() .setTracerProvider( SdkTracerProvider.builder() .addSpanProcessor(SimpleSpanProcessor.create(spanExporter)) .addTracerConfiguratorCondition(nameEquals("scopeB"), TracerConfig.disabled()) .build()) .setMeterProvider( SdkMeterProvider.builder() .registerMetricReader(metricReader) .addMeterConfiguratorCondition(nameEquals("scopeB"), MeterConfig.disabled()) .build()) .setLoggerProvider( SdkLoggerProvider.builder() .addLogRecordProcessor(SimpleLogRecordProcessor.create(logRecordExporter)) .addLoggerConfiguratorCondition(nameEquals("scopeB"), LoggerConfig.disabled()) .build()) .build(); 
@codecov
Copy link

codecov bot commented Apr 10, 2024

Codecov Report

Attention: Patch coverage is 98.60140% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 91.11%. Comparing base (7b4bb8f) to head (7a82538).
Report is 5 commits behind head on main.

❗ Current head 7a82538 differs from pull request most recent head 1714387. Consider uploading reports for the commit 1714387 to get more accurate results

Files Patch % Lines
...in/java/io/opentelemetry/sdk/metrics/SdkMeter.java 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@ ## main #6375 +/- ## ============================================ + Coverage 91.08% 91.11% +0.03%  - Complexity 5774 5825 +51  ============================================ Files 627 633 +6 Lines 16855 16948 +93 Branches 1721 1728 +7 ============================================ + Hits 15352 15442 +90  - Misses 1007 1009 +2  - Partials 496 497 +1 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@LikeTheSalad LikeTheSalad left a comment

Choose a reason for hiding this comment

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

Thank you 🙏

*
* @see LoggerConfig#configuratorBuilder()
*/
public SdkLoggerProviderBuilder setLoggerConfigurator(
Copy link
Contributor

Choose a reason for hiding this comment

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

These ScopeConfigurator setters are great!

Copy link
Contributor

@breedx-splk breedx-splk left a comment

Choose a reason for hiding this comment

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

Yeah this is great. I had a few ideas that I think are worth considering, and I'd really like to see the glob tests beefed up slightly, but otherwise looking cool to me. 😎

Copy link
Contributor

@breedx-splk breedx-splk left a comment

Choose a reason for hiding this comment

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

Nice!

@jack-berg
Copy link
Member Author

Thanks for the review @breedx-splk / @LikeTheSalad! Now that its received some eyes, I've went ahead and pushed a commit to move all the public APIs to internal packages using patterns we've established accessing experimental APIs.

Should be good to go ahead with this now!

Copy link
Contributor

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

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

Seems good to me. 🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants