Add metrics to ZipkinSpanExporter

[Donnerbart]

This is a follow-up to #4487 to complete the metrics for span exporters story.

I'm not sure if you like the distinct transport name for HTTP/JSON encoding in the instrumentationScopeName. This will change the existing metric names for OkHttpExporter when JSON encoding is used. I made that part an extra commit which can easily be dropped, if that change is not wanted.

[codecov]

Codecov Report

Merging #4501 (7d990b3) into main (77be2e0) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #4501      +/-   ##
============================================
+ Coverage     90.08%   90.11%   +0.02%     
- Complexity     5074     5077       +3     
============================================
  Files           584      584              
  Lines         15667    15682      +15     
  Branches       1504     1506       +2     
============================================
+ Hits          14114    14132      +18     
+ Misses         1100     1099       -1     
+ Partials        453      451       -2     

Impacted Files	Coverage Δ
...entelemetry/exporter/internal/ExporterMetrics.java	100.00% <100.00%> (ø)
...metry/exporter/internal/okhttp/OkHttpExporter.java	95.45% <100.00%> (+0.21%)	⬆️
...ntelemetry/exporter/zipkin/ZipkinSpanExporter.java	89.16% <100.00%> (+1.55%)	⬆️
...try/exporter/zipkin/ZipkinSpanExporterBuilder.java	100.00% <100.00%> (ø)
...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.

[Donnerbart]

I think the public field baseLogger and the package-private method produceLocalIp() in ZipkinSpanExporter should be private. This change would break the binary compatibility, but I think it's very unlikely that someone is using these anyway. Is there a way to do this change without failing the exporters:zipkin:jApiCmp check or can't this be changed anymore?

[jkwatson]

I think the public field baseLogger and the package-private method produceLocalIp() in ZipkinSpanExporter should be private. This change would break the binary compatibility, but I think it's very unlikely that someone is using these anyway. Is there a way to do this change without failing the exporters:zipkin:jApiCmp check or can't this be changed anymore?

Unfortunately, we're stuck with that mistake. :(

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[Donnerbart]

Hey, is there anything I can do to move this forward?

[jack-berg]

Sorry for not responding sooner. I've been hesitant to accept this because it introduces a new transitive dependency for the :exporters:zipkin module on com.squareup.okhttp3:okhttp. More generally, the :exporters:otlp:common artifact is becoming a home for utility methods used by a variety of exporters, which feels sloppy.

Maybe we can move the truly common classes to a new module :exporters:common, and merge remaining shared otlp utilities into :exporters:otlp:all. Thoughts @anuraaga / @jkwatson?

[anuraaga]

@jack-berg A module split would definitely be a good idea I think. I don't know if it blocks this PR - doesn't the zipkin exporter already depend on okhttp via the zipkin sender?

[jack-berg]

It shades in okhttp io.zipkin.reporter2:zipkin-sender-okhttp3.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[jack-berg]

This is unblocked now that #4575 is merged. I've pushed a commit to rebase. Thanks @Donnerbart!

[jack-berg]

@jkwatson any additional thoughts before I merge this?

[jkwatson]

🚢