Skip to content

fix(server-utils): Only create Redis db spans when there is a parent span - #25854

Closed
lazerg wants to merge 2 commits into
getsentry:developfrom
lazerg:feat/redis-require-parent-span
Closed

lazerg wants to merge 2 commits into
getsentry:developfrom
lazerg:feat/redis-require-parent-span

Conversation

@lazerg

@lazerg lazerg commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Why: Redis commands and connects that run outside any active span (blocking XREAD loops, ioredis cluster slot refreshes, background jobs) each start their own trace and get sampled as root transactions with no context. The older ioredis instrumentation already required a parent, so this does the same for the native node-redis:*/ioredis:* channels, the node-redis v4/v5 orchestrion channels, and legacy redis v2/v3.

How to review: The fix is requiresParentSpan: true on each bindTracingChannelToSpan call, plus an early return in the legacy subscriber. The existing tests expected a standalone redis-connect segment, which is now gone. scenario-no-parent.mjs checks that only the command inside the parent span gets a span.

Fixes #25793
Closes #25798

…span

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@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:05
@lazerg
lazerg requested a review from a team as a code owner October 29, 2026 07:05
@lazerg
lazerg requested review from andreiborza and mydea and removed request for a team October 29, 2026 07:05
@Lms24 Lms24 self-assigned this Oct 29, 2026
@Lms24
Lms24 self-requested a review October 29, 2026 09:35
…ll paths

Move the redis v2-v3 subscriber onto bindTracingChannelToSpan so every
Redis path checks for a parent span the same way. The wrapped command
callback still ends the span; deferSpanEnd keeps the helper from ending
it on the sync return.

Add no-parent tests for ioredis, node-redis v4/v5 (orchestrion), native
batch commands, and a unit test for the legacy path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Lms24

Lms24 commented Oct 29, 2026

Copy link
Copy Markdown
Member

I made some changes to this PR, where I'd appreciate a review from someone on the team with more DC experience:

  • The legacy path used to subscribe to the channel by hand manually called getActiveSpan() to check for a parent. This is yet another pattern of how we check for an active span where we'd have to adjust client reports again. Now it uses bindTracingChannelToSpan, with deferSpanEnd so the span stays open until the command callback fires.
  • Added tests for the various (io)redis versions we had so that the behaviour is covered consistently

@nicohrubec

Copy link
Copy Markdown
Member

after some more internal discussion on the approach for this I'll close this down as superseded by #25879

@nicohrubec nicohrubec closed this Oct 29, 2026
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 redis db spans when there is a parent span Redis diagnostics_channel subscriber creates root spans for commands outside any active span

3 participants