Use the HTTP_TARGET attribute for sampling decisions in the Jaeger Remote Sampler

[chrismgrayftsinc]

The starting span name for spans that derive from HTTP requests is always HTTP <method>, which is not very useful for sampling decisions. Using the http target if the name is not matched allows for things like health checks to be suppressed.

[codecov[bot]]

Codecov Report

Merging #4498 (e33fbfd) into main (33d2c22) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##               main    #4498   +/-   ##
=========================================
  Coverage     90.04%   90.04%           
- Complexity     5039     5053   +14     
=========================================
  Files           578      580    +2     
  Lines         15553    15564   +11     
  Branches       1498     1495    -3     
=========================================
+ Hits          14005    14015   +10     
- Misses         1087     1089    +2     
+ Partials        461      460    -1     

Impacted Files	Coverage Δ
...sion/trace/jaeger/sampler/PerOperationSampler.java	93.75% <100.00%> (+15.17%)	:arrow_up:
...entelemetry/exporter/jaeger/PostSpansResponse.java	0.00% <0.00%> (-100.00%)	:arrow_down:
...metry/exporter/internal/okhttp/OkHttpExporter.java	85.71% <0.00%> (-9.53%)	:arrow_down:
...exporter/jaeger/MarshalerCollectorServiceGrpc.java	85.71% <0.00%> (-4.77%)	:arrow_down:
...metry/sdk/metrics/export/PeriodicMetricReader.java	86.95% <0.00%> (-2.90%)	:arrow_down:
.../main/java/io/opentelemetry/api/metrics/Meter.java	0.00% <0.00%> (ø)
...c/main/java/io/opentelemetry/sdk/metrics/View.java	100.00% <0.00%> (ø)
...ava/io/opentelemetry/api/metrics/DefaultMeter.java	89.71% <0.00%> (ø)
...va/io/opentelemetry/api/metrics/BatchCallback.java	0.00% <0.00%> (ø)
...va/io/opentelemetry/api/metrics/MeterProvider.java	100.00% <0.00%> (ø)
... and 66 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 33d2c22...e33fbfd. Read the comment docs.

[jkwatson]

Interesting. I don't know much about the jaeger remote sampler, personally. Is this something that we should have go through the specification before making the change?

[chrismgrayftsinc]

I believe that this would conform to the spec. There is conversation on open-telemetry/opentelemetry-java-contrib#65 and open-telemetry/opentelemetry-specification#1597 that lead me to this belief, but I may have misinterpreted.

[jkwatson]

@anuraaga do you have much exposure to the jaeger remote sampler or know someone who does? @jpkrohling or @yurishkuro can you provide feedback on this?

[anuraaga]

I don't know much about jaeger, but it would indeed be good to add language to the spec if changing behavior.

That being said, most instrumentation would set a span name with a route URL, so perhaps @chrismgrayftsinc your problem is a bug in instrumentation. What framework are you using?

[yurishkuro]

Agree with @anuraaga, if a more descriptive name is available, why is the span name set to HTTP <method>, which is only meant for the worst case?

The change itself is not bad, but a) it introduces unintuitive / unexpected behavior into the SDK, b) it would be different from other implementations, and c) it's fairly hacky since it singles out a specific attribute.

A cleaner solution would be to support attribute-based sampling in the remote sampling configuration, but that would be a much larger change (including extending Jaeger's sampling config data model, something we always wanted to do).

[chrismgrayftsinc]

I am using the javaagent with Spring Boot. The name does get changed somewhere down the line, but at the time that the sampling decision is made it seems to be always HTTP <method>.

[anuraaga]

Thanks @chrismgrayftsinc - I filed https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/6188 to see if there's a way to improve this.

[github-actions[bot]]

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

[github-actions[bot]]

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

[github-actions[bot]]

Closed as inactive. Feel free to reopen if this PR is still being worked on.