Skip to content

feat(deno): redis diagnostics channel based integration for deno - #21087

Merged
isaacs merged 5 commits into
developfrom
isaacs/deno-redis-dc
May 29, 2026
Merged

feat(deno): redis diagnostics channel based integration for deno#21087
isaacs merged 5 commits into
developfrom
isaacs/deno-redis-dc

Conversation

@isaacs

@isaacsisaacs commented May 20, 2026

Copy link
Copy Markdown
Member

Refactor the redis and ioredis diagnostics_channel integration logic into core/src/integrations, and create a Deno integration that uses the same patterns.

Instead of the @sentry/opentelemetry/tracing-channel, the Deno integration adds _sentrySpan onto the data in a RedisTracingChannelFactory which is passed to the core utility.

Add deno-redis e2e test covering both redis clients.

fix: #21221
fix: JS-2630

@isaacs
isaacs requested a review from a team as a code ownerMay 20, 2026 21:20
@isaacs
isaacs requested review from andreiborza and mydea and removed request for a teamMay 20, 2026 21:20
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize% ChangeChange
@sentry/browser27.17 kB--
@sentry/browser - with treeshaking flags25.62 kB--
@sentry/browser (incl. Tracing)45.25 kB--
@sentry/browser (incl. Tracing + Span Streaming)47.47 kB--
@sentry/browser (incl. Tracing, Profiling)50.22 kB--
@sentry/browser (incl. Tracing, Replay)84.83 kB--
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags74.39 kB--
@sentry/browser (incl. Tracing, Replay with Canvas)89.53 kB--
@sentry/browser (incl. Tracing, Replay, Feedback)102.15 kB--
@sentry/browser (incl. Feedback)44.34 kB--
@sentry/browser (incl. sendFeedback)31.98 kB--
@sentry/browser (incl. FeedbackAsync)37.08 kB--
@sentry/browser (incl. Metrics)28.25 kB--
@sentry/browser (incl. Logs)28.48 kB--
@sentry/browser (incl. Metrics & Logs)29.19 kB--
@sentry/react28.99 kB--
@sentry/react (incl. Tracing)47.51 kB--
@sentry/vue32.2 kB--
@sentry/vue (incl. Tracing)47.16 kB--
@sentry/svelte27.19 kB--
CDN Bundle29.55 kB--
CDN Bundle (incl. Tracing)47.81 kB--
CDN Bundle (incl. Logs, Metrics)31.05 kB--
CDN Bundle (incl. Tracing, Logs, Metrics)49.04 kB--
CDN Bundle (incl. Replay, Logs, Metrics)70.31 kB--
CDN Bundle (incl. Tracing, Replay)85.18 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics)86.35 kB--
CDN Bundle (incl. Tracing, Replay, Feedback)91.05 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics)92.21 kB--
CDN Bundle - uncompressed87.59 kB--
CDN Bundle (incl. Tracing) - uncompressed144.09 kB--
CDN Bundle (incl. Logs, Metrics) - uncompressed92.08 kB--
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed147.85 kB--
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed216.81 kB--
CDN Bundle (incl. Tracing, Replay) - uncompressed262.87 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed266.61 kB--
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed276.57 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed280.3 kB--
@sentry/nextjs (client)50.03 kB--
@sentry/sveltekit (client)45.68 kB--
@sentry/core/server76.56 kB+0.83%+628 B 🔺
@sentry/core/browser63.09 kB--
@sentry/node-core61.69 kB--
@sentry/node130.51 kB+0.09%+112 B 🔺
@sentry/node - without tracing74.1 kB--
@sentry/aws-serverless86.29 kB--
@sentry/cloudflare (withSentry) - minified171.5 kB--
@sentry/cloudflare (withSentry)429.29 kB--

View base workflow run

isaacs added a commit that referenced this pull request May 20, 2026
)
Refactor the redis-dc integration logic into core/src/integrations, and
create a Deno integration that uses the same patterns.
Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration just adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.
Add deno-redis e2e test.
@isaacs
isaacsforce-pushed the isaacs/deno-redis-dc branch from f176fd8 to 0fc320cCompareMay 20, 2026 22:13
@isaacs
isaacsforce-pushed the isaacs/deno-redis-dc branch from 1048f32 to 72be1faCompareMay 20, 2026 22:18
Comment thread.github/workflows/build.yml
isaacs added a commit that referenced this pull request May 21, 2026
isaacs added a commit that referenced this pull request May 21, 2026
)
Refactor the redis-dc integration logic into core/src/integrations, and
create a Deno integration that uses the same patterns.
Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration just adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.
Add deno-redis e2e test.
@isaacs
isaacsforce-pushed the isaacs/deno-redis-dc branch from 11525d8 to a8d3cb7CompareMay 21, 2026 17:31
Comment threadpackages/deno/src/integrations/redis.ts Outdated
Comment threadpackages/core/src/integrations/redis/redis-statement-serializer.ts Outdated
Comment threadpackages/core/src/integrations/redis/redis-dc-subscriber.ts
Comment threadpackages/node/src/integrations/tracing/redis/index.ts
Comment threadpackages/node/src/utils/redisCache.ts
Comment threadpackages/deno/src/sdk.ts Outdated
isaacs added a commit that referenced this pull request May 21, 2026
isaacs added a commit that referenced this pull request May 21, 2026
@isaacs
isaacsforce-pushed the isaacs/deno-redis-dc branch from 8226aaf to 5863e6cCompareMay 21, 2026 21:54
@isaacs
isaacs enabled auto-merge (squash) May 21, 2026 23:10
isaacs added a commit that referenced this pull request May 22, 2026
)
Refactor the redis-dc integration logic into core/src/integrations, and
create a Deno integration that uses the same patterns.
Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration just adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.
Add deno-redis e2e test.
@isaacs
isaacsforce-pushed the isaacs/deno-redis-dc branch from 5863e6c to cd298faCompareMay 22, 2026 17:01
Comment threadpackages/core/src/integrations/redis/redis-statement-serializer.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor

👋 @mydea, @andreiborza — Please review this PR when you get a chance!

isaacs added a commit that referenced this pull request May 25, 2026
)
Refactor the redis-dc integration logic into core/src/integrations, and
create a Deno integration that uses the same patterns.
Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration just adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.
Add deno-redis e2e test.
@isaacs
isaacsforce-pushed the isaacs/deno-redis-dc branch from e46685b to 1f6c601CompareMay 27, 2026 18:47
Comment threadpackages/core/src/integrations/redis/redis-statement-serializer.ts Outdated

@JPeer264JPeer264 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, just have one m which I want to get sorted before approving

Comment threadpackages/core/src/integrations/redis/redis-dc-subscriber.ts Outdated
Comment threadpackages/core/src/integrations/redis/redis-dc-subscriber.ts
Comment threadpackages/core/src/integrations/redis/redis-dc-subscriber.ts
Comment threadpackages/core/src/server-exports.ts
@github-actions

Copy link
Copy Markdown
Contributor

👋 @mydea, @andreiborza — Please review this PR when you get a chance!

@mydea
mydea requested review from logaretm and nicohrubec and removed request for mydeaMay 28, 2026 12:35

@logaretmlogaretm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do u think we can merge this first

#21187

since it will also allow this PR to do it for both ioredis and redis clients.

@JPeer264

Copy link
Copy Markdown
Member

Do u think we can merge this first

I'm personally fine with this. Approved the other PR, waiting for @isaacs decision.

isaacs added a commit that referenced this pull request May 28, 2026
)
Refactor the redis-dc integration logic into core/src/integrations, and
create a Deno integration that uses the same patterns.
Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration just adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.
Add deno-redis e2e test.
@isaacs
isaacsforce-pushed the isaacs/deno-redis-dc branch from 1f6c601 to 6a2f9b6CompareMay 28, 2026 18:29
)
Refactor the redis and ioredis diagnostics_channel integration logic
into core/src/integrations, and create a Deno integration that uses the
same patterns.
Instead of the @sentry/opentelemetry/tracing-channel, the Deno
integration adds `_sentrySpan` onto the data in a
RedisTracingChannelFactory which is passed to the core utility.
Add deno-redis e2e test covering both redis clients.
fix: #21221
fix: JS-2630
@isaacs
isaacsforce-pushed the isaacs/deno-redis-dc branch from 6a2f9b6 to ebbbc65CompareMay 29, 2026 01:55
@isaacs
isaacs requested review from JPeer264 and logaretmMay 29, 2026 01:55

@cursorcursorBot 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 1 potential issue.

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 ebbbc65. Configure here.

Comment threadpackages/core/src/integrations/redis/redis-dc-subscriber.ts Outdated

@JPeer264JPeer264 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Lint is failing, but it looks like a small change.

): void {
const channel = tracingChannel<T>(channelName, data => {
const args = getCommandArgs(data);
const statement = safeSerialize(data.command, args);

@logaretmlogaretmMay 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

m: tracing channels on redis already sanitize the command statements or is that a different thing?

https://github.com/redis/ioredis/blob/main/lib/tracing.ts#L10-L22

https://github.com/logaretm/node-redis/blob/master/packages/client/lib/client/tracing.ts#L39-L44

It was based off the one implemented by OTEL originally, and Redis team wanted Redis to own that part.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Ooh, good catch. Yes, this was ported in from the otel implementation, and looks like we missed it the first time around, but it's just spinning extra CPU cycles unnecessarily, since the dc publishers already do this for us.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This also means we don't have to handle Uint8Array/Buffer args, which simplifies it even further, actually.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@logaretm Ended up being a pretty big diff. Care to take another look at that bit? 489eee9

@isaacs
isaacs disabled auto-merge May 29, 2026 14:39
@isaacs
isaacs merged commit 18fbbb7 into developMay 29, 2026
266 of 267 checks passed
@isaacs
isaacs deleted the isaacs/deno-redis-dc branch May 29, 2026 16:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(deno): redis diagnostics channel based integration for deno

3 participants

@isaacs@JPeer264@logaretm