feat(opentelemetry): end the workflow span when a waiting instance is cancelled by edeandrea · Pull Request #1059 · quarkiverse/quarkus-flow

edeandrea · 2026-10-07T15:59:54Z

Description

Cancelling a workflow instance that is WAITING on a listen task never ended or exported its workflow.execute span. The listener logged No instrumentation context was found ... instead.

The cause is the order of operations in the engine's WorkflowMutableInstance.cancel(). Cancelling the future the listen task registered completes the start pipeline right away, and its cleanUp() closes and clears the instance metadata, including WorkflowInstrumentationContext. That happens before onWorkflowCancelled is published, so the listener can no longer find the context when the event arrives. An instance cancelled while waiting on a wait task goes the other way: the event is published first and cleanup runs later.

This PR makes the OTel extension handle both orders. The root cause in the engine is tracked in open-workflow-specification/sdk-java#1749. Once that is fixed, the close() path here becomes a safety net.

Fixes #1058

Changes

WorkflowInstrumentationContext
- New endWorkflowSpan(eventType, statusSetter): closes any open task spans, records the event and ends the workflow span. Only the first call does anything (an AtomicBoolean guards it).
- close() now ends the workflow span with OK status and a workflow.cancelled event when the instance status is already CANCELLED (it is set before cleanup runs). In every other case it behaves as before and only closes task spans.
- The constructor now also takes the WorkflowInstanceData.
OTelWorkflowExecutionListener
- Completed, cancelled and failed events all end the span through endWorkflowSpan, so the span is never ended twice, whichever of the listener event or close() comes first.
- A cancelled event that arrives after the metadata was cleared is logged at DEBUG instead of WARN.
Unit tests (opentelemetry/runtime, the module's first tests: JUnit, AssertJ, Mockito and opentelemetry-sdk-testing, all test-scoped and managed by the Quarkus BOM)
- New WorkflowInstrumentationContextTest (11 tests) runs against a real SdkTracerProvider with an InMemorySpanExporter. It covers endWorkflowSpan() ending open task spans and the workflow span, endWorkflowSpan() only acting once, close() on a CANCELLED instance, close() on every other WorkflowStatus (parameterized), and both orders of close() and the cancelled event.
Integration tests (opentelemetry/integration-tests)
- New CancelWorkflowIT, with two tests that each cancel 5 waiting instances. For every instance they assert that exactly its task span and one workflow span are exported, and that the workflow span has exactly one workflow.cancelled event.
  - listen task: metadata is cleared before the cancelled event. This is the bug; the test fails without the fix.
  - wait task: the cancelled event comes before cleanup. This checks the span is still ended only once.
- New otel-listen-task workflow and POST /otel-workflows/{otel-listen-task,otel-wait-task}/cancel endpoints.
- SpanInfo.getEventNames().
Docs: opentelemetry.adoc now says workflow spans cover cancellation too.

Testing

Test Plan

Unit tests added/updated
Integration tests added/updated (if applicable)
Tested manually (describe below if applicable)

Ran locally (JDK 25): ./mvnw install -pl opentelemetry/runtime,opentelemetry/deployment,opentelemetry/integration-tests -DskipITs=false. The 11 new unit tests and all 16 OTel ITs pass, and the app log has no No instrumentation context warnings. Without the runtime change, the listen test in CancelWorkflowIT fails because the workflow span of each cancelled instance is missing. Disabling the CANCELLED branch in close() makes closeOnACancelledInstanceEndsTheWorkflowSpanAsCancelled fail.

Manual Testing

I ran the integration-test app jar by hand (java -jar target/quarkus-app/quarkus-run.jar), called POST /otel-workflows/otel-listen-task/cancel, and read /otel-span-exporter/spans. The cancelled instance's workflow.execute otel-listen-task span is exported with workflow.started and workflow.cancelled events, next to its task.execute listenTask span.

There is a standalone reproducer at https://github.com/edeandrea/quarkus-flow-reproducers (issue1058, against released quarkus-flow versions).

Checklist

Before submitting this PR, please ensure:

I ran the full build with integration tests locally: ./mvnw clean install -DskipITs=false
Code follows the project's code conventions
Tests have been added/updated to cover the changes
Documentation has been updated (if user-facing changes)
Commit messages are clear and follow conventional commits style
I have read and followed the Contributing Guide
I have read and comply with the LLM Usage Policy (if applicable)

Additional Notes

Only the OTel modules' build and ITs were run locally, not the full multi-module build. The change is limited to opentelemetry/ and one docs page.

The failing Build website check ("Generated documentation is not up to date" for quarkus-flow-persistence*.adoc) is unrelated to this PR. It comes from #1051 changing HashingPersistenceConfig on main without regenerating those config docs.

edeandrea · 2026-10-07T16:07:52Z

FYI the Build website check is failing with

The following generated files were changed or added and must be committed:

*  docs/modules/ROOT/pages/includes/quarkus-flow-persistence.adoc
*  docs/modules/ROOT/pages/includes/quarkus-flow-persistence_quarkus.flow.adoc

I don't think thats anything I've done. I think it was introduced in #1051 . I can fix it here in this PR if you'd like.

ricardozanini · 2026-10-07T17:26:00Z

@edeandrea thank you! I'll have a look. Regarding CI, I'm aware and we have the fix already to be merged.

edeandrea · 2026-10-07T17:27:06Z

@edeandrea thank you! I'll have a look. Regarding CI, I'm aware and we have the fix already to be merged.

You're welcome! I have PRs coming for the other 2 issues I opened today as well!

Copilot

🟢 Approval recommended

All reviewed changes are covered by tests and no unresolved blocking issues were identified.

0 open findings

What changed in this PR

Fixes missing OpenTelemetry workflow spans when waiting instances are cancelled by making span completion idempotent across cleanup and event ordering.

Changes:

Added cancellation-aware workflow span finalization.
Added unit and integration coverage for cancellation paths.
Updated test fixtures, endpoints, dependencies, and documentation.

File	Description
`opentelemetry/runtime/src/test/java/io/quarkiverse/flow/opentelemetry/runtime/WorkflowInstrumentationContextTest.java`	Tests span completion and cancellation ordering.
`opentelemetry/runtime/src/main/java/io/quarkiverse/flow/opentelemetry/runtime/WorkflowInstrumentationContext.java`	Adds idempotent workflow and task span handling.
`opentelemetry/runtime/src/main/java/io/quarkiverse/flow/opentelemetry/runtime/OTelWorkflowExecutionListener.java`	Handles terminal lifecycle events safely.
`opentelemetry/runtime/pom.xml`	Adds test dependencies.
`opentelemetry/integration-tests/src/test/java/io/quarkiverse/flow/opentelemetry/it/util/SpanInfo.java`	Exposes span event names for assertions.
`opentelemetry/integration-tests/src/test/java/io/quarkiverse/flow/opentelemetry/it/CancelWorkflowIT.java`	Verifies exported spans for cancelled workflows.
`opentelemetry/integration-tests/src/main/resources/flow/otel-listen-task.yaml`	Adds a waiting listen workflow fixture.
`opentelemetry/integration-tests/src/main/java/io/quarkiverse/flow/opentelemetry/it/OTelWorkflowsResource.java`	Adds cancellation test endpoints.
`docs/modules/ROOT/pages/opentelemetry.adoc`	Documents cancellation coverage.

🧠 Review effort: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

ricardozanini

LGTM, many thanks! We will wait for the OTel SMEs to have a look before merging.

edeandrea · 2026-10-07T17:54:57Z

LGTM, many thanks! We will wait for the OTel SMEs to have a look before merging.

No problem! I actually discovered these issues while working on a demo app

edeandrea · 2026-10-07T17:55:14Z

OOps sorry didn't mean to close it!

github-actions · 2026-10-07T17:55:24Z

🙈 The PR is closed and the preview is expired.

edeandrea · 2026-10-07T17:56:32Z

FYI, I opened open-workflow-specification/sdk-java#1751 for the engine-side root cause (open-workflow-specification/sdk-java#1749). It changes WorkflowMutableInstance.cancel() / cancelFuture() to publish the CANCELLED status change and onWorkflowCancelled before cancelling the futures registered through addCancelable, so instance metadata is still there when listeners get the cancelled event.

This PR is still needed until a quarkus-flow release picks up an sdk-java version with that fix. After that, the close() path in WorkflowInstrumentationContext only acts as a safety net, and endWorkflowSpan being idempotent keeps the span from being ended twice either way.

wmedvede · 2026-10-07T17:57:30Z

LGTM, many thanks! We will wait for the OTel SMEs to have a look before merging.

I'll look into this ASAP.

wmedvede

Many thanks @edeandrea for this work. Honestly, I think this PR is kind of introducing a workaround due to the issue you pointed out: open-workflow-specification/sdk-java#1749

I can see that you kindly proposed a PR too: open-workflow-specification/sdk-java#1749 , and discussion is ongoing. And, hopefully we'll have a solution for the removed metadata soon.

Since this PR still introduce valuable work with the tests, I think we can:

wait some days to get the solution for the removed metadata and getting a new release. (@fjtirado can deliver a new relase quickly)
bump to get that new relase
remove the workarond from this PR, but keep the tests, since are very much appreciated to should validate the fix in the engine too. And we merge.

WDYT?

wmedvede

see my comments

edeandrea · 2026-10-08T11:58:53Z

Many thanks @edeandrea for this work. Honestly, I think this PR is kind of introducing a workaround due to the issue you pointed out: open-workflow-specification/sdk-java#1749

I can see that you kindly proposed a PR too: open-workflow-specification/sdk-java#1749 , and discussion is ongoing. And, hopefully we'll have a solution for the removed metadata soon.

Since this PR still introduce valuable work with the tests, I think we can:

wait some days to get the solution for the removed metadata and getting a new release. (@fjtirado can deliver a new relase quickly)

bump to get that new relase

remove the workarond from this PR, but keep the tests, since are very much appreciated to should validate the fix in the engine too. And we merge.

WDYT?

Thanks @wmedvede !

I wasn't sure about the relationship between quarkus-flow & open-workflow-specification, so I figured doing something here (more tactical) and there (more strategic) was good. If you say that there is quick turnaround between them then I agree, keeping the solution in one place - the correct strategic solution - is better.

If you think there may be some delay in getting the upstream fix in and worked in here, then as a user I'd prefer to have this workaround in place in the short term. We can always add an issue to track the workaround and remove it once upstream lands.

I'll leave that decision to you as project maintainer to make. I can re-work this pr so it is just the tests and not the workaround. Obviously until the upstream fix is in and the PR gets rebased on it, the checks will fail.

Let me know what you'd like to do.

wmedvede · 2026-10-08T13:19:16Z

then as a user I'd prefer to have this workaround in place in the short term

Hi @edeandrea , I totally agree, I would have done pretty much similar approach.
I think we can give a couple of days to see if there's a quick solution for the long term fix in the runtime. I not, we merge this PR.

edeandrea · 2026-10-08T13:34:34Z

Sounds good! Just FYI I'm out of the office most of next week, so if you need/want me to make any changes I'd need to do it before the end of day Friday this week. Otherwise it would have to wait until I get back.

ricardozanini · 2026-10-09T16:05:44Z

@edeandrea this is the last one to release 1.2.0, can you please rebase?

edeandrea · 2026-10-09T16:07:03Z

@edeandrea this is the last one to release 1.2.0, can you please rebase?

On it!

ricardozanini · 2026-10-09T16:07:13Z

@wmedvede so are we waiting for the SDK? I think I'll have to push this one to 2.0.0. @edeandrea how urgent is this?

edeandrea · 2026-10-09T16:09:17Z

@wmedvede so are we waiting for the SDK? I think I'll have to push this one to 2.0.0. @edeandrea how urgent is this?

If I'm being selfish, I'd like to have it for a demo i'm doing at a conference next week :) But as a project maintainer I can understand if you'd rather wait until the upstream fix is in (avoiding demo-driven-development :D ). Its already been merged. Just needs a release. See open-workflow-specification/sdk-java#1751

… cancelled Cancelling an instance that is WAITING on a listen task makes the engine clear the instance metadata (and close WorkflowInstrumentationContext) before it publishes onWorkflowCancelled, so the listener could no longer find the context and the workflow.execute span was never ended or exported. - WorkflowInstrumentationContext.close() now ends the workflow span with a workflow.cancelled event when the instance status is already CANCELLED - Completed, cancelled and failed events all end the span through endWorkflowSpan(), which only acts once, so the span is never ended twice whichever of the listener event or close() comes first - A cancelled event arriving after the metadata is cleared is logged at DEBUG instead of WARN - WorkflowInstrumentationContextTest unit-tests endWorkflowSpan() and close() for every instance status and both orderings - CancelWorkflowIT covers both cancel orderings end to end (listen and wait tasks) The root cause in the engine is tracked in open-workflow-specification/sdk-java#1749. Fixes quarkiverse#1058

edeandrea · 2026-10-09T16:23:16Z

@ricardozanini changes in

ricardozanini · 2026-10-09T16:48:11Z

@edeandrea, @wmedvede adapted the workaround in #1072, and we can implement the long-term fix after we bring the SDK. I'll close this one, many thanks! Once #1072 is green, I'll release.

EDIT: reopening now since this one is more complete and have been rebased. Walter tried to move faster on the other one, but Eric did the rebase shortly.

github-actions · 2026-10-09T17:49:24Z

📊 Quarkus Flow Coverage Report

Overall Project	29.82%	❌

There is no coverage information present for the Files changed

edeandrea requested a review from a team as a code owner October 7, 2026 15:59

edeandrea force-pushed the issue-1058-otel-cancel-workflow-span branch from 5a22515 to edf27fc Compare October 7, 2026 16:09

ricardozanini requested review from fjtirado, mathecruz and wmedvede October 7, 2026 17:20

ricardozanini requested a lite review from Copilot October 7, 2026 17:26

Copilot started reviewing on behalf of ricardozanini October 7, 2026 17:26 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

mathecruz approved these changes Oct 7, 2026

View reviewed changes

ricardozanini approved these changes Oct 7, 2026

View reviewed changes

edeandrea closed this Oct 7, 2026

edeandrea reopened this Oct 7, 2026

edeandrea mentioned this pull request Oct 7, 2026

[Fix #1749] Publish onWorkflowCancelled before cancelling pending futures open-workflow-specification/sdk-java#1751

Merged

wmedvede approved these changes Oct 8, 2026

View reviewed changes

wmedvede suggested changes Oct 8, 2026

View reviewed changes

edeandrea force-pushed the issue-1058-otel-cancel-workflow-span branch from edf27fc to 1ae7c59 Compare October 9, 2026 16:21

wmedvede mentioned this pull request Oct 9, 2026

issue-1058: feat(opentelemetry): end the workflow span when a waiting instance is cancelled #1072

Closed

10 tasks

ricardozanini closed this Oct 9, 2026

ricardozanini reopened this Oct 9, 2026

wmedvede approved these changes Oct 9, 2026

View reviewed changes

ricardozanini merged commit c2fbc45 into quarkiverse:main Oct 9, 2026
43 checks passed

edeandrea deleted the issue-1058-otel-cancel-workflow-span branch October 9, 2026 17:42

Conversation

edeandrea commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Testing

Test Plan

Manual Testing

Checklist

Additional Notes

Uh oh!

edeandrea commented Oct 7, 2026

Uh oh!

ricardozanini commented Oct 7, 2026

Uh oh!

edeandrea commented Oct 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

ricardozanini left a comment

Choose a reason for hiding this comment

Uh oh!

edeandrea commented Oct 7, 2026

Uh oh!

edeandrea commented Oct 7, 2026

Uh oh!

github-actions Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edeandrea commented Oct 7, 2026

Uh oh!

wmedvede commented Oct 7, 2026

Uh oh!

wmedvede left a comment

Choose a reason for hiding this comment

Uh oh!

wmedvede left a comment

Choose a reason for hiding this comment

Uh oh!

edeandrea commented Oct 8, 2026

Uh oh!

wmedvede commented Oct 8, 2026

Uh oh!

edeandrea commented Oct 8, 2026

Uh oh!

ricardozanini commented Oct 9, 2026

Uh oh!

edeandrea commented Oct 9, 2026

Uh oh!

ricardozanini commented Oct 9, 2026

Uh oh!

edeandrea commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edeandrea commented Oct 9, 2026

Uh oh!

ricardozanini commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 9, 2026

📊 Quarkus Flow Coverage Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

edeandrea commented Oct 7, 2026 •

edited

Loading

github-actions Bot commented Oct 7, 2026 •

edited

Loading

edeandrea commented Oct 9, 2026 •

edited

Loading

ricardozanini commented Oct 9, 2026 •

edited

Loading