Add infrastructure for concurrency limit listeners; use that to improve tracing support for limits #10368
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Resolves #10323
Release Note
——
When Helidon's concurrency limiting algorithm defers processing of an incoming request it creates a tracing span (if this behavior is enabled) for the waiting time. If the incoming request propagated a parent span Helidon did not properly make that span the parent of the waiting time span.
Helidon now correctly sets the parent for concurrency limit waiting time span.
——
PR Overview
General Approach
The existing concurrency limit implementations (fixed and AIMD) update metrics (if enabled) that are related to concurrency limiting processing. The metrics are based only on the limit name, so the code that registers and updates metrics does not need any information that varies from request to request.
In contrast, to correctly parent a tracing span for waiting time the code does need per-request information: the HTTP headers.
The limiting code is—and needs to remain--independent from any particular source of work (HTTP requests is the current use case).
To solve this specific problem and to offer a general way to listen to concurrency limit processing, this PR introduces the concept of a limit listener. The PR implements such a listener to manage the waiting time span.
Changes in
helidon-common-concurrency-limits
Adds the
LimitAlgorithmListener
SPI.Expands the
Limit
interface so any code that invokes a limit’stryAcquire
orinvoke
method can now pass one or more such listeners, and the limit code calls back to these listeners at key points as it processes the work item.The two existing
Limit
implementations (fixed and AIMD) now call back to such listeners as they process work.An enhancement to an existing test verifies that listerers are notified.
Changes in
helidon-http
Adds the predefined headertraceparent
. W3C prescribes this header for propagating a tracing span. (This is used in a new test but it’s a standard header so we might as well have declared for all to use.)I removed the above change. I added it originally to accommodate the new test code, but it triggered a failure in an MP Telemetry TCK which checks in a case-sensitive way for the
traceparent
header. We are on a relatively old MP Telemetry release (1.1) but similar tests remain in the current codebase. I have filed this issue: microprofile/microprofile-telemetry#289.Changes in
helidon-webserver-webserver
Declares a new SPI
HttpLimitListenerProvider
. Any limit listener provider to be used in HTTP request concurrency limiting should implement this interface.Both the http1 and http2 code that handles incoming requests now locates registered implementations of the above SPI and invokes them to create listeners for each incoming request, then passes those listeners to the limit implementation's
tryAcquire
orinvoke
method.Changes in
helidon-webserver-concurrency
Adds a new service singleton
HttpTracingLimitListenerProvider
which implements the SPI above and creates listeners that properly start and end a queueing span as needed (with the correct propagated parent, if any).Changes in
helidon-webserver-tests-resource-limits
Adds a test to verify the correct parent for a queueing span.
Changes to doc
Brief mention on the concurrency limits page about custom listeners with links to the relevant Javadoc.
What this PR does not do
LimitAlgorithmListener
.Limit
implementations into a common superclass.