Break out GrpcSender, GrpcSenderProvider

[jack-berg]

Resolves #5572.

This PR touches a lot of files, and I'm not sure it can be reasonably split up. The HttpSender version was able to be split up because there was initially only an okhttp implementation - the jdk implementation was green field. In the GrpcSender case, there are two implementations (okhttp and grpc-upstream) which need to be supported out of the box. On the bright side, the patterns here mirror the HttpSender implementation so there's very little new material.

Some details about the implementation:

• The okhttp implementation is rolled into the existing opentelemetry-exporter-sender-okhttp artifact. The GrpcSender implementation doesn't require any new artifacts so better to package it with HttpSender to reduce artifact sprall.
• The grpc upstream implementation is publishe din a new artifact named opentelemetry-exporter-sender-grpc-upstream.
• To use the grpc upstream, you need to:
  • Add a dependency on opentelemetry-exporter-sender-grpc-upstream
  • Exclude the default dependency on opentelemetry-exporter-sender-okhttp
  • Set the channel manually using OtlpGrpc{Signal}ExporterBuilder#setChannel(ManagedChannel)

On that last note, its technically redundant to require users to include the grpc upstream artifact AND explicitly set the managed channel. We might consider adjusting the behavior to use grpc upstream if its the only GrpcSenderProvider implementatino present. Doing so would get us back into the business of configuring ManagedChannel instances from configuration parameters, which we stopped doing when we deprecated #setChannel(ManagedChannel).

[codecov]

Codecov Report

Patch coverage: 92.68% and no project coverage change.

Comparison is base (8770703) 91.25% compared to head (2d84c92) 91.25%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #5617   +/-   ##
=========================================
  Coverage     91.25%   91.25%           
- Complexity     5016     5032   +16     
=========================================
  Files           554      556    +2     
  Lines         14860    14861    +1     
  Branches       1387     1389    +2     
=========================================
+ Hits          13561    13562    +1     
- Misses          895      896    +1     
+ Partials        404      403    -1     

Files Changed	Coverage Δ
...try/exporter/internal/grpc/ManagedChannelUtil.java	85.71% <ø> (ø)
...telemetry/exporter/internal/http/HttpExporter.java	78.18% <0.00%> (ø)
...exporter/jaeger/JaegerGrpcSpanExporterBuilder.java	81.25% <ø> (-0.57%)	⬇️
...er/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java	92.50% <ø> (-0.19%)	⬇️
...er/otlp/metrics/OtlpGrpcMetricExporterBuilder.java	100.00% <ø> (ø)
...porter/otlp/trace/OtlpGrpcSpanExporterBuilder.java	92.50% <ø> (-0.19%)	⬇️
...porter/sender/okhttp/internal/GrpcRequestBody.java	56.52% <ø> (ø)
...orter/sender/okhttp/internal/OkHttpHttpSender.java	98.43% <ø> (ø)
...ry/exporter/sender/okhttp/internal/OkHttpUtil.java	100.00% <ø> (ø)
...orter/sender/okhttp/internal/RetryInterceptor.java	100.00% <ø> (ø)
... and 11 more

... and 3 files with indirect coverage changes

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

[trask]

I don't love the name opentelemetry-exporter-sender-grpc-upstream, but I'm not sure I have any better ideas:

• opentelemetry-exporter-sender-grpc-lib
• opentelemetry-exporter-sender-grpc-api
• opentelemetry-exporter-sender-grpc-java
• opentelemetry-exporter-sender-grpc-dep

[jack-berg]

I don't mind opentelemetry-exporter-sender-grpc-lib.

What about opentelemetry-exporter-sender-grpc-transport? "transport" is the terminology used by the grpc library.

Or opentelemetry-exporter-sender-grpc-channel? "channel" is the abstraction over "transport", and may be a better description of what this new sender artifact contains, since its agnostic about which underlying grpc transport is used.

[trask]

What about opentelemetry-exporter-sender-grpc-transport? "transport" is the terminology used by the grpc library.

"grpc-transport" sounds like it could also describe our hand-rolled grpc transport layer

Or opentelemetry-exporter-sender-grpc-channel? "channel" is the abstraction over "transport", and may be a better description of what this new sender artifact contains, since its agnostic about which underlying grpc transport is used.

it may be a little subtle for some people, but the whole thing is kinda subtle

(similar, not necessarily better) opentelemetry-exporter-sender-grpc-managed-channel meaning, the only reason to use this is if you need access to the ManagedChannel(?)

[jack-berg]

opentelemetry-exporter-sender-grpc-managed-channel is verbose but clear that its an implementation that utilizes ManagedChannel to execute the export requests.

If others agree I'll update the artifact name to this^^.

[jack-berg]

@jkwatson any thoughts on this?

[jkwatson]

@jkwatson any thoughts on this?

I haven't spent a lot of time thinking about this...but my only thought currently is that this adds 3 new published modules, none of which have any public-facing APIs. That feels ... odd to me ... and I haven't really thought through the pros and cons of that.

[jack-berg]

It is a bit odd, but the alternative is using gradle exclude syntax to manage transitive dependencies not part of io.opentelemetry.* to achieve the same effect. I.e. if you want to use the java 11+ http client version, then exclude this set of okhttp dependencies.

I think this is the cleanest way to handle the sprawl of use cases we want to support as dictated by the java ecosystem. And with the HttpSender / GrpcSender SPI setup, we've added some future proofing against having to make significant refactors to accommodate whatever comes next, since users who are unsatisfied can provide their own implementation in a pinch.

I do think that this is complex enough where we need some documentation. I'm imagining a list of use cases like and corresponding dependency management instruments, like: "If using java 11+ and you prefer to avoid extra dependencies, use the following configuration: ...". I can take an AI to add this kind of documentation to opentelemetry.io to help alleviate confusion.

[jkwatson]

Do we want to make these new modules alpha until we've settled on naming? Or are we confident enough that the names are "good enough" to go with?

[jack-berg]

I think the names are good enough. Marking the artifacts as alpha indicates that the've taken a step back in stability / reliability, which isn't the case. I will note that the opentelemetry-exporter-sender-jdk artifact is alpha because it still needs some time in the wild before it can earn the stable designation. So the stable artifacts are opentelemetry-exporter-sender-okhttp and opentelemetry-exporter-sender-grpc-managed-channel.