Skip to content

feat(nitro): Add unstorage tracing channel instrumentation - #20615

Merged
logaretm merged 3 commits into
developfrom
awad/js-1089-nitro-use-tracing-channels-for-unstorage
May 5, 2026
Merged

feat(nitro): Add unstorage tracing channel instrumentation#20615
logaretm merged 3 commits into
developfrom
awad/js-1089-nitro-use-tracing-channels-for-unstorage

Conversation

@logaretm

@logaretmlogaretm commented Apr 30, 2026

Copy link
Copy Markdown
Member

Subscribes to unstorage tracing events to create storage and cache spans for all storage operations (getItem, setItem, hasItem, removeItem, getKeys, clear, etc.).

The storage instrumentation now has full feature parity with Nuxt's SDK.

Note: The instrumentation takes effect if the user uses ^3.0.260429-beta since that's the version that shipped storage events, before that version the channel will be inert.

I also sneaked in a minor refactor, we don't need the NOOP subscribers anymore since we patched the type in the tracingChannel helper we expose from @sentry/opentelemetry.

closes#18022

CopilotAI review requested due to automatic review settings April 30, 2026 18:33
@logaretm
logaretm requested a review from a team as a code ownerApril 30, 2026 18:33
@linear-code

Copy link
Copy Markdown

Comment threadpackages/nitro/src/runtime/hooks/captureStorageEvents.ts

CopilotAI 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.

Pull request overview

Adds Nitro runtime instrumentation for unstorage tracing channels so storage/cache operations are captured as Sentry spans, aligning Nitro’s SDK behavior with Nuxt’s storage instrumentation.

Changes:

  • Register captureStorageEvents() in the Nitro server runtime plugin to subscribe to unstorage.<operation> tracing channels.
  • Introduce new storage-span creation logic (op/origin/cache attributes + cache-hit detection) for common storage operations.
  • Add Nitro-3 e2e coverage for storage operations, storage alias methods, and cachedFunction/cachedHandler cache hit/miss behavior; bump Nitro test-app dependency to the first version with storage tracing events.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
packages/nitro/src/runtime/plugins/server.tsHooks storage tracing initialization into Nitro runtime startup.
packages/nitro/src/runtime/hooks/captureStorageEvents.tsNew tracing-channel subscriber that creates cache/storage spans for unstorage operations.
dev-packages/e2e-tests/test-applications/nitro-3/tests/storage.test.tsE2E assertions for spans + attributes for core storage methods.
dev-packages/e2e-tests/test-applications/nitro-3/tests/storage-aliases.test.tsE2E assertions for spans created by storage alias APIs (get/set/has/del/remove).
dev-packages/e2e-tests/test-applications/nitro-3/tests/cache.test.tsE2E assertions for cachedFunction/cachedHandler cache spans and hit/miss semantics.
dev-packages/e2e-tests/test-applications/nitro-3/server/api/test-storage.tsTest endpoint exercising key storage operations.
dev-packages/e2e-tests/test-applications/nitro-3/server/api/test-storage-aliases.tsTest endpoint exercising alias APIs.
dev-packages/e2e-tests/test-applications/nitro-3/server/api/test-cache.tsTest endpoint exercising defineCachedFunction / defineCachedHandler.
dev-packages/e2e-tests/test-applications/nitro-3/package.jsonBumps Nitro to ^3.0.260429-beta to ensure storage tracing events exist.
Comments suppressed due to low confidence (1)

packages/nitro/src/runtime/hooks/captureStorageEvents.ts:168

  • isResponseCacheEntry checks key.startsWith('nitro:handlers:'), which won't match if the key includes a storage mount prefix (e.g. cache:nitro:handlers:...). If keys are prefixed, this will prevent response-cache validation for cached handlers and can skew cache.hit. Consider adjusting the check to handle optional prefixes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadpackages/nitro/src/runtime/hooks/captureStorageEvents.ts
Comment threadpackages/nitro/src/runtime/hooks/captureStorageEvents.ts
Comment threadpackages/nitro/src/runtime/hooks/captureStorageEvents.ts
Comment threadpackages/nitro/src/runtime/hooks/captureStorageEvents.ts

@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 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 cae896c. Configure here.

Comment threadpackages/nitro/src/runtime/hooks/captureStorageEvents.ts
Comment threaddev-packages/e2e-tests/test-applications/nitro-3/tests/cache.test.ts Outdated
@github-actions

github-actionsBot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize% ChangeChange
@sentry/browser26.16 kB--
@sentry/browser - with treeshaking flags24.63 kB--
@sentry/browser (incl. Tracing)44.13 kB--
@sentry/browser (incl. Tracing + Span Streaming)46.34 kB--
@sentry/browser (incl. Tracing, Profiling)49.08 kB--
@sentry/browser (incl. Tracing, Replay)83.48 kB--
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags72.96 kB--
@sentry/browser (incl. Tracing, Replay with Canvas)88.15 kB--
@sentry/browser (incl. Tracing, Replay, Feedback)100.8 kB--
@sentry/browser (incl. Feedback)43.4 kB--
@sentry/browser (incl. sendFeedback)30.96 kB--
@sentry/browser (incl. FeedbackAsync)36.14 kB--
@sentry/browser (incl. Metrics)27.44 kB--
@sentry/browser (incl. Logs)27.59 kB--
@sentry/browser (incl. Metrics & Logs)28.28 kB--
@sentry/react27.9 kB--
@sentry/react (incl. Tracing)46.36 kB--
@sentry/vue31.03 kB--
@sentry/vue (incl. Tracing)45.96 kB--
@sentry/svelte26.18 kB--
CDN Bundle28.85 kB--
CDN Bundle (incl. Tracing)46.91 kB--
CDN Bundle (incl. Logs, Metrics)30.27 kB--
CDN Bundle (incl. Tracing, Logs, Metrics)48.03 kB--
CDN Bundle (incl. Replay, Logs, Metrics)69.35 kB--
CDN Bundle (incl. Tracing, Replay)84.07 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics)85.14 kB--
CDN Bundle (incl. Tracing, Replay, Feedback)89.86 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics)90.96 kB--
CDN Bundle - uncompressed84.55 kB--
CDN Bundle (incl. Tracing) - uncompressed140.16 kB--
CDN Bundle (incl. Logs, Metrics) - uncompressed88.75 kB--
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed143.62 kB--
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed212.71 kB--
CDN Bundle (incl. Tracing, Replay) - uncompressed257.96 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed261.41 kB--
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed271.66 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed275.1 kB--
@sentry/nextjs (client)48.85 kB--
@sentry/sveltekit (client)44.58 kB--
@sentry/node-core59.06 kB+0.02%+8 B 🔺
@sentry/node170.35 kB+0.01%+9 B 🔺
@sentry/node - without tracing96.92 kB+0.01%+9 B 🔺
@sentry/aws-serverless113.78 kB+0.03%+30 B 🔺
@sentry/cloudflare (withSentry) - minified164.96 kB--
@sentry/cloudflare (withSentry)417.1 kB--

View base workflow run

- hasItem: use boolean result directly instead of generic isCacheHit
- isCacheHit: handle both string and already-deserialized cache values
- cache tests: replace silent if-guards with hard assertions
@logaretm
logaretmforce-pushed the awad/js-1089-nitro-use-tracing-channels-for-unstorage branch from 92ea118 to 6b37d88CompareApril 30, 2026 18:58
[SEMANTIC_ATTRIBUTE_CACHE_KEY]: cacheKeys.length > 1 ? cacheKeys : cacheKeys[0],
'db.operation.name': operation,
'db.collection.name': mountBase(data),
'db.system.name': data.driver?.name ?? 'unknown',

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.

btw, we also have db.driver.name if this is more fitting here (not sure what's in data.driver.name)

https://getsentry.github.io/sentry-conventions/attributes/db/#db-driver-name

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.

Usually redis, memory, fs, etc...

We could add it, I was mostly following what we had in Nuxt.

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.

Okay then system.name makes sense 👍

{
name: cacheKeys.join(', ') || operation,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: `cache.${normalizeMethodName(operation)}`,

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.

Just a general question: Should we define all unstorage database calls to cache.? Is it always used as a cache?

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.

That's one of its primary use-cases, but it is not strictly a caching layer.

We could instead assume that nitro: prefixes are cache calls (set by defineCachedFunction/defineCachedEventHandler) and remove this attribute from other user calls. WDYT?

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.

yes, this makes sense if nitro: is always attached to defineCached...

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.

I evaluated and there is not alot of other OPs that work here, closest thing is db which doesn't make sense given we will add that later. so cache fits here as it is the primary use-case, it has a hit/miss, it has an expiry, so it works.

happy to change later!

const SEMANTIC_ATTRIBUTE_CACHE_KEY = 'cache.key';
const SEMANTIC_ATTRIBUTE_CACHE_HIT = 'cache.hit';

test('instruments storage alias methods (get, set, has, del, remove) and creates spans', async ({ request }) => {

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.

It's probably worth creating multiple tests for each method to keep the test cases small. You can also use the /write-tests skill for refactoring. But I see that this is only using one waitForTransaction so it's probably not feasable to have multiple tests here when everything happens in one transaction.

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.

We would need to split it over multiple endpoints, which is tedious. Doing it in a describe breaks the test isolation.

Do we want to do that, what do u think in this case?

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.

then better leave it as it is :D

@logaretm
logaretm merged commit 8f7f640 into developMay 5, 2026
39 checks passed
@logaretm
logaretm deleted the awad/js-1089-nitro-use-tracing-channels-for-unstorage branch May 5, 2026 15:47
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.

[Nitro] Use tracing channels for unstorage

3 participants

@logaretm@s1gr1d