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

Use minimal fallback managed channel when none is specified #6110

Merged

Conversation

jack-berg
Copy link
Member

@jack-berg jack-berg commented Jan 3, 2024

Fixes #5958.

See comment describing need for this change:

  • If you include opentelemetry-exporter-sender-grpc-managed-channel and exclude opentelemetry-exporter-sender-okhttp, autoconfigure will fail to initialize because opentelemetry-exporter-sender-grpc-managed-channel expects the user to call setChannel, and there's no chance to do that before the span exporter customizer gets called, so we get a NPE.
  • The NPE occurs because UpstreamGrpcSenderProvider doesn't have any way to accommodate a null managedChannel argument.
  • I'm pushing a fix in Use minimal fallback managed channel when none is specified #6110 which updates UpstreamGrpcSenderProvider to initialize a minimal managed channel is none is explicitly set by the user. This will allow autoconfigure to initialize without error, but still requires the user to use a span exporter customer to customize any of the options of the managed channel.

Copy link

codecov bot commented Jan 3, 2024

Codecov Report

Attention: 88 lines in your changes are missing coverage. Please review.

Comparison is base (de65a4b) 91.12% compared to head (42cd12e) 90.99%.
Report is 6 commits behind head on main.

Files Patch % Lines
.../exporter/prometheus/Otel2PrometheusConverter.java 71.53% 58 Missing and 20 partials ⚠️
...porter/prometheus/PrometheusHttpServerBuilder.java 37.50% 5 Missing ⚠️
...etry/exporter/prometheus/PrometheusHttpServer.java 89.65% 3 Missing ⚠️
...ry/exporter/prometheus/PrometheusMetricReader.java 90.00% 1 Missing ⚠️
...edchannel/internal/UpstreamGrpcSenderProvider.java 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6110      +/-   ##
============================================
- Coverage     91.12%   90.99%   -0.13%     
+ Complexity     5742     5690      -52     
============================================
  Files           630      628       -2     
  Lines         16844    16688     -156     
  Branches       1664     1656       -8     
============================================
- Hits          15349    15186     -163     
- Misses         1029     1047      +18     
+ Partials        466      455      -11     

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

@jack-berg jack-berg added this to the 1.34.0 milestone Jan 3, 2024
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.

Replacing the okhttp based sender using JDK autoconfigure
2 participants