Skip to content

fix(server-utils): Require a parent span for Firestore operations - #25855

Merged
Lms24 merged 4 commits into
getsentry:developfrom
lazerg:fix/firestore-require-parent-span
Oct 29, 2026
Merged

Lms24 merged 4 commits into
getsentry:developfrom
lazerg:fix/firestore-require-parent-span

Conversation

@lazerg

@lazerg lazerg commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Why: Firestore calls made outside any active span each became their own root span with no context. This passes requiresParentSpan: true to the channel binding, like the other db integrations.

How to review: The fix is the option in firebase/instrumentation.ts. The Deno test runs addDoc before, inside and after a parent span and checks that only the one inside creates a span.

Closes #25800

@github-actions github-actions Bot added the external PR from an external contributor label Oct 29, 2026
@lazerg
lazerg marked this pull request as ready for review October 29, 2026 07:13
@lazerg
lazerg requested a review from a team as a code owner October 29, 2026 07:13
@lazerg
lazerg requested review from andreiborza and mydea and removed request for a team October 29, 2026 07:13

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lazerg
lazerg force-pushed the fix/firestore-require-parent-span branch from dc25c18 to 5f6dcbf Compare October 29, 2026 07:28
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Lms24 Lms24 self-assigned this Oct 29, 2026
@Lms24
Lms24 self-requested a review October 29, 2026 08:47
@Lms24

Lms24 commented Oct 29, 2026

Copy link
Copy Markdown
Member

@lazerg we'll review this, as well as other PRs but to be very clear: We do not appreciate "contribution farming" and opening 7 PRs in a row without once asking if we're fine with you taking over those changes. Our contribution guidelines clearly tell external contributors to ask a maintainer first. An unfortunate change we had to make given that writing code no longer costs time, but reviewing agent-written code is the painful part now. Please keep this in mind for the next contribution. Thank you.

Add a Node E2E check next to the Deno test: a Firestore call outside any
active span must not produce a root db.query span.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Lms24
Lms24 requested review from a team as code owners October 29, 2026 09:13
@Lms24
Lms24 requested review from nicohrubec and removed request for a team October 29, 2026 09:13

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c854344. Configure here.

Flush before the marker span so an unparented Firestore span would reach the
proxy first, and assert on all spans of a dedicated collection instead of only
root spans. A parented call inside the marker proves the instrumentation is
active.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Lms24
Lms24 merged commit 6bc31a6 into getsentry:develop Oct 29, 2026
337 checks passed
Lms24 added a commit that referenced this pull request Oct 29, 2026
This PR adds the external contributor to the CHANGELOG.md file, so that
they are credited for their contribution. See #25855

Co-authored-by: Lms24 <8420481+Lms24@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external PR from an external contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Only create Firestore db spans when there is a parent span

2 participants