Add skaffolding for low level exporter SSL API

[jack-berg]

Related to #5280 and this comment.

The overall goal is to provide a low level API for configuring SSL on the exporters for when configuring a static certificate or static mTLS isn't good enough. This should be the last API we add, acting as a catch all for other TLS scenarios.

Ideally we'd be able to just expose a setSslContext(SSLContext) method. SSLContext is initialized using KeyManager(s) and TrustManager(s), and produces a SSLSocketFactory via SSLContext#getSocketFactory().

However, our dependence on okhttp complicates this, since okhttp requires TLS to be configured via OkHttpClient.Builder#sslSocketFactory(SSLSocketFactory, X509TrustManager). You'd think that because a SSLContext is comprised of X509TrustManager and produces a SSLSocketFactory, that SSLContext would be enough, but its unfortunately not possible to access the X509TrustManager from SSLContext.

This means that our low level API has to instead be setSslContext(SSLContext, X509TrustManager). Including the X509TrustManager will feel repetitive to users, since they have likely already included it while initializing SSLContext, but its the best we can do with okhttp.

Note, other http libraries like jdk httpclient and apache httpclient accept SSLContext for configuration, so okhttp's API is really rather strange. There's a stackoverflow post that hints at the okhttp maintainers working through the API contract.

[codecov]

Codecov Report

Patch coverage: 82.85% and project coverage change: +0.04 🎉

Comparison is base (e1fcd15) 91.20% compared to head (7430348) 91.24%.

❗ Current head 7430348 differs from pull request most recent head 30877e4. Consider uploading reports for the commit 30877e4 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #5362      +/-   ##
============================================
+ Coverage     91.20%   91.24%   +0.04%     
+ Complexity     4865     4862       -3     
============================================
  Files           546      546              
  Lines         14369    14324      -45     
  Branches       1351     1352       +1     
============================================
- Hits          13105    13070      -35     
+ Misses          875      865      -10     
  Partials        389      389              

Impacted Files	Coverage Δ
...va/io/opentelemetry/exporter/internal/TlsUtil.java	87.50% <ø> (+1.56%)	⬆️
...try/exporter/internal/grpc/ManagedChannelUtil.java	85.71% <ø> (-1.79%)	⬇️
...xporter/internal/okhttp/OkHttpExporterBuilder.java	88.88% <60.00%> (+5.25%)	⬆️
...ry/exporter/internal/grpc/GrpcExporterBuilder.java	98.68% <75.00%> (-1.32%)	⬇️
...ace/jaeger/sampler/JaegerRemoteSamplerBuilder.java	98.14% <75.00%> (-1.86%)	⬇️
...entelemetry/exporter/internal/TlsConfigHelper.java	87.09% <90.90%> (+5.96%)	⬆️

... and 1 file with indirect coverage changes

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[tylerbenson]

Seems like the better option would be to have multiple constructors with the various argument permutations. That would result in multiple calls being independent instances and not require explicit callout.

[tylerbenson]

Another option would be to have these as static methods that return the constructed object.

[breedx-splk]

Nah, this is an internal class. Think of it like a stateful builder, or really a collaborator for a stateful builder. I think a bunch of overloaded constructors is a muddier option in this case.

[tylerbenson]

Any risk that one of these being unexpectedly/accidentally null breaks things? The builders don't seem to be covered in the tests.

[jack-berg]

Any risk that one of these being unexpectedly/accidentally null breaks things?

If you look closely at TlsConfigHelper both getSslContext and getTrustManager are non-null as long as either setTrustManagerFromCerts or setSslContext is called. These are always called in any valid TLS configuration.

The builders don't seem to be covered in the tests.

Are you referring to OkHttpExporterBuilder#setSslContext not being called in tests? There's a bit of a chicken and an egg problem. Trying to figure out the low level API before exposing it at the higher level (i.e. adding setSslContext methods to OtlpHttp{Signal}Exporterbuilder). Once the API is added in the higher level builders, AbstractHttpTelemetryExporterTest should be extended to call setSslContext.

[breedx-splk]

Cool stuff @jack-berg this is aligned with what we chatted about. Thanks for sticking it out, I think the API is finally shaping up here. Appreciate it.

[breedx-splk]

Nah, this is an internal class. Think of it like a stateful builder, or really a collaborator for a stateful builder. I think a bunch of overloaded constructors is a muddier option in this case.