Skip to content

fix(firestore): prevent adding telemetry events to ended spans (#8847) - #8939

Merged
quirogas merged 2 commits into
googleapis:mainfrom
quirogas:fix/firestore-tracing-ended-span
Jul 23, 2026
Merged

fix(firestore): prevent adding telemetry events to ended spans (#8847)#8939
quirogas merged 2 commits into
googleapis:mainfrom
quirogas:fix/firestore-tracing-ended-span

Conversation

@quirogas

Copy link
Copy Markdown
Contributor

In REST-mode streaming operations (such as getAll(), Query.get(), or DocumentReference.set()), DocumentReader resolves its operation Promise as soon
as response data is processed, which causes startActiveSpan() to close the OpenTelemetry span.

However, the underlying stream's 'end' event fires on a subsequent tick of the Node.js event loop:

resultStream.on('end', () => {
this._traceUtil.currentSpan().addEvent(Firestore.${methodName}: Completed, ...);
});

Because the span is already closed by the time this 'end' listener runs, calling addEvent() throws an error in modern OpenTelemetry runtimes.
Guarding addEvent() ensures late stream events return safely without interrupting execution.

  • Added an isRecording() check in Span.addEvent() (dev/src/telemetry/span.ts) using this.span?.isRecording?.() === false to return early if a span is
    closed.
  • Added a unit test in dev/test/tracing.ts to verify that Span.addEvent() exits safely without throwing when called on an ended span.

Context

OpenTelemetry Version Changes: In older versions of @opentelemetry/api (< 1.9.0), calling addEvent() on a closed span was silently ignored by the
trace SDK. Starting in @opentelemetry/api >= 1.9.0, OpenTelemetry introduced strict runtime checks (SpanImpl._isSpanEnded) that throw Error:
Operation attempted on ended Span.

@quirogas quirogas self-assigned this Jul 22, 2026
@product-auto-label product-auto-label Bot added the api: firestore Issues related to the Firestore API. label Jul 22, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the addEvent method in the Span class to return early if the underlying span is no longer recording (isRecording() === false), preventing errors when adding events to ended spans. A unit test has also been added to verify that calling addEvent on an ended span does not throw an exception. There are no review comments, and I have no feedback to provide.

@quirogas
quirogas force-pushed the fix/firestore-tracing-ended-span branch from 0973999 to 44618e5 Compare July 23, 2026 05:27
@quirogas quirogas added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 23, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 23, 2026
@quirogas
quirogas force-pushed the fix/firestore-tracing-ended-span branch from 0aa4657 to 36742db Compare July 23, 2026 16:32
@quirogas
quirogas marked this pull request as ready for review July 23, 2026 19:58
@quirogas
quirogas requested a review from a team as a code owner July 23, 2026 19:58
@quirogas
quirogas merged commit 1d3c655 into googleapis:main Jul 23, 2026
34 of 35 checks passed
@MarkDuckworth MarkDuckworth added the release-please:force-run To run release-please label Aug 4, 2026
@release-please release-please Bot removed the release-please:force-run To run release-please label Aug 4, 2026
MarkDuckworth added a commit that referenced this pull request Aug 4, 2026
---


##
[8.7.1](firestore-v8.7.0...firestore-v8.7.1)
(2026-08-04)


### Bug Fixes

* **firestore:** Prevent adding telemetry events to ended spans
([#8847](#8847))
([#8939](#8939))
([1d3c655](1d3c655))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: firestore Issues related to the Firestore API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants