Jaeger sampler operation sampling strategy not working with tomcat applications

[srinusanduri]

The Jaeger remote sampler implementation allows us to define sampling at operation name/span name. This however is not working with tomcat based applications like spring-boot applications.

As per this thread open-telemetry/opentelemetry-java-instrumentation#7853 (comment), in tomcat instrumentation, the span is started with a generic name as the route can't be determined at start of the span. Later when request processing reaches a framework where route can be determined, the name is updated with a new better name.

Since the sampling decision is made at the start of the span and tomcat span name is not known at the start, operation sampling definitions won't work for tomcat spans. For example, for a request GET /student, tomcat assigns HTTP GET as the span name at start, which is then changed to GET /student later. Sampling configurations defined for operation GET /student won't work in this case. I observed similar behavior for Netty applications as well.

Tomcat and Netty spans are generally the root spans for spring-boot web applications, which means operation/span name sampling definition isn't possible for most of the Java web applications with OpenTelemetry java agent.

Is there any alternative solution to achieve sampling at operation level for spring-boot applications?

[laurit]

Is there any alternative solution to achieve sampling at operation level for spring-boot applications?

If you wish to use the span name you have to use tail based sampling (https://opentelemetry.io/docs/concepts/sampling/#tail-sampling). Samplers in java SDK use head sampling (https://opentelemetry.io/docs/concepts/sampling/#head-sampling) and can not use the span name when it changes after span creation (sampling will be done based on the name when the span was created). Alternatively instead of operation name you could use url.path/http.target attribute that is set when the span is started for sampling.

[srinusanduri]

@laurit @trask

I have implemented a custom Jaeger remote sampler extension to enable operation sampling for HTTP request traces originating from servers (Tomcat, Netty, etc.).

Approach

The method involves constructing the operation name in the format {http-method} {http-path} for HTTP request traces. This is achieved by reading the HTTP method and HTTP path values from the http.request.method and url.path span attributes, respectively. If these attributes do not exist, the operation name is not overridden, and the name passed to the sampler is used for the sampling decision.

Additionally, the Jaeger PerOperationSampler will be updated to match HTTP path patterns instead of exact string matches to accommodate path variables in HTTP paths. For example, we can configure a sampling pattern for the operation GET /op0/{opId}/sub-op0/{subOpId}, which will match all values for {opId} and {subOpId}. For instance, it will match the HTTP request GET /op0/456/sub-op0/xyz123.

Contribution Proposal

I would like to contribute this sampler as an extension to the project: OpenTelemetry Java SDK Extensions. This sampler addresses the issue where operation-level sampling isn't functioning for HTTP requests.

Implementation Options

To add this new sampler AdaptiveJaegerRemoteSampler.java, similar to JaegerRemoteSampler.java, there are two approaches:

Simpler Approach:

Add the new sampler AdaptiveJaegerRemoteSampler.java and a couple of supporting classes within the jaeger-remote-sampler module itself to reuse the existing JaegerRemoteSampler.java implementation code.

Modular Approach:

Create a new module adaptive-jaeger-remote-sampler similar to the existing jaeger-remote-sampler module.
Make the package-protected code of the jaeger-remote-sampler module accessible to other modules.

Looking forward to your thoughts and feedback on this.

[laurit]

I'll move this discussion over to opentelemetry-java.

[jkwatson]

The contents of this repository are primary limited to things that are covered by the OTel specification. I recommend contributing this sampler to the contrib repo, rather than trying to put it into the core repository. We have very limited resources to maintain components here that are not covered by the specification.

[srinusanduri]

Hi @jkwatson,

Thank you for taking the time to review my contribution proposal. I understand that contributions should typically go into the opentelemetry-java-contrib repository.

However, I would like to highlight that the proposed AdaptiveJaegerRemoteSampler.java (similar to JaegerRemoteSampler.java) is designed to reuse the code from the existing jaeger-remote-sampler module in the opentelemetry-java repository. By introducing this new sampler within the existing module, we can leverage the existing implementation code.

If you still believe that this contribution should go into the contrib repository, I am more than willing to proceed accordingly. I wanted to ensure that I provided the full context for my initial proposal to keep this within the opentelemetry-java repository.

Thank you for your guidance.

[jkwatson]

@jack-berg I'll let you weigh in on this one as well, but my feeling is that the maintainers here don't want to take on the maintenance of a new non-specified sampler, and it belongs in contrib, so @srinusanduri can be the official maintainer of it.

[jack-berg]

In our definition of project scope, we say that this project only includes components which are part of the specification. While I haven't seen the source for this proposed AdaptiveJaegerRemoteSampler, I'm fairly confident that what you're describing isn't part of the spec. If it depends on code from the jaeger remote sampler defined in this repo, it can take a dependency on that code. And if there is code it wants to reuse which is not visible, we can consider adjusting the visibility modifiers to make that code accessible.

If it turns out that I'm misunderstanding and that AdaptiveJaegerRemoteSampler is in fact part of the spec, or if someone evolves the spec to include the idea of AdaptiveJaegerRemoteSampler, we can revisit this decision.

[srinusanduri]

Hi @jack-berg, Thank you for reviewing this. AdaptiveJaegerRemoteSampler is not part of the spec. As you suggested, I will contribute this to the contrib project by adding the jaeger-remote-sampler artifact as a dependency by making the required code visible.