Skip to content

opentelemetry-http: stabilize lifecycle cancellation test#3570

Open
arnabnandy7 wants to merge 1 commit into
apple:mainfrom
arnabnandy7:fix/flaky-test
Open

opentelemetry-http: stabilize lifecycle cancellation test#3570
arnabnandy7 wants to merge 1 commit into
apple:mainfrom
arnabnandy7:fix/flaky-test

Conversation

@arnabnandy7

Copy link
Copy Markdown

Motivation

OpenTelemetryHttpRequesterFilterTest.transportObserver was flaky when cancelling an HTTP/2 request. The test assumed that onExchangeFinally() always runs before the OpenTelemetry span is exported. During cancellation, request termination may complete asynchronously, allowing the span to end before the final lifecycle callback adds its test attribute.

Fixes #3561.

Modifications

  • Added a latch to TestHttpLifecycleObserver to observe onExchangeFinally() deterministically.
  • Updated the cancellation test to await the final lifecycle callback.
  • Removed the assertion that the final callback’s attribute must appear on an already-ended span.
  • Preserved validation of the cancellation attribute and the callback’s active span context.

Result

The cancellation test no longer depends on timing or span mutability after export. All transportObserver HTTP/1.1 and HTTP/2 combinations pass.

There are no production behavior, API, or compatibility changes; only test code is affected.

Signed-off-by: Arnab Nandy <arnab_nandy7@yahoo.com>
@bryce-anderson

Copy link
Copy Markdown
Contributor

I imagine this fixes the flaky test, but I'm a bit concerned about why it's flaky in the first place. Eg, are we papering over a real problem?

@arnabnandy7

Copy link
Copy Markdown
Author

@bryce-anderson Thanks for raising this. I traced the cancellation path to make sure the change is not hiding a production issue.

HttpLifecycleObserver.onExchangeFinally() is invoked only after both the request and response sides terminate. When an HTTP/2 request is cancelled, the response side can terminate first, causing the OpenTelemetry client span to end and be exported while the request side is still completing asynchronously. Once the request side terminates, onExchangeFinally() runs with the expected span context, but setAttribute() has no effect because that span has already ended. Whether the attribute appears in the exported span therefore depends on the relative timing of those two events.

This is consistent with the requester filter documentation, which notes that callbacks executing after the tracing filter closes its scope will not be able to modify the span.

The change still verifies that:

  • onExchangeFinally() is invoked, using a bounded deterministic wait.
  • The callback sees the same valid span context, through the existing identity/error checks.
  • onResponseCancel() executes early enough to add its attribute to the exported span.

It only removes the assumption that onExchangeFinally() can always mutate the span after cancellation. No production behavior is changed. I’ll also add a comment in the cancellation test explaining why its final callback is verified separately from the exported span attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants