Skip to content

Conversation

logaretm
Copy link
Collaborator

@logaretm logaretm commented Oct 3, 2025

What

This PR adds automatic instrumentation for Nuxt's storage layer (powered by unstorage), enabling performance monitoring for cache and key-value storage operations in Nuxt/Nitro applications.

Storage operations will now automatically create performance spans with detailed attributes for observability in Sentry.

What's New

  • Automatic Storage Instrumentation: Instruments all storage drivers configured in nuxt.config.ts via nitro.storage
  • Comprehensive Coverage: Tracks all storage operations including:
    • getItem, setItem, hasItem, removeItem
    • Raw variants: getItemRaw, setItemRaw
    • Batch operations: getItems, setItems
    • Utility methods: getKeys, clear
    • Aliases: get, set, has, del, remove

Implementation Details

Span Attributes:

  • sentry.op: cache.{operation} (e.g., cache.get_item, cache.set_item)
  • sentry.origin: auto.cache.nuxt
  • cache.key: Full key including mount prefix
  • cache.hit: true for successful get/has operations
  • db.operation.name: Original method name
  • db.collection.name: Storage mount point
  • db.system.name: Driver name (e.g., memory, fs, redis)

Files Changed:

  • packages/nuxt/src/runtime/plugins/storage.server.ts - Runtime instrumentation plugin
  • packages/nuxt/src/vite/storageConfig.ts - Build-time configuration
  • packages/nuxt/src/module.ts - Module integration
  • E2E tests for Nuxt 3 & 4

Copy link

linear bot commented Oct 3, 2025

@logaretm logaretm force-pushed the awad/js-1016-nuxtnitro-instrument-kv-storage-instrumenting-unstorage branch from 07cb542 to 24a5470 Compare October 3, 2025 15:40
@logaretm logaretm linked an issue Oct 3, 2025 that may be closed by this pull request
@logaretm logaretm self-assigned this Oct 4, 2025
@logaretm logaretm marked this pull request as ready for review October 6, 2025 12:28
@logaretm logaretm force-pushed the awad/js-1016-nuxtnitro-instrument-kv-storage-instrumenting-unstorage branch from a6dc41f to fe203a5 Compare October 6, 2025 12:29
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@logaretm logaretm requested review from andreiborza and s1gr1d October 6, 2025 12:55
@logaretm logaretm force-pushed the awad/js-1016-nuxtnitro-instrument-kv-storage-instrumenting-unstorage branch from 911b7fe to abe38de Compare October 8, 2025 10:34
cursor[bot]

This comment was marked as outdated.

@logaretm logaretm force-pushed the awad/js-1016-nuxtnitro-instrument-kv-storage-instrumenting-unstorage branch from 64e5814 to 42ef2c7 Compare October 9, 2025 09:02
Copy link
Contributor

github-actions bot commented Oct 9, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 24.4 kB - -
@sentry/browser - with treeshaking flags 22.9 kB - -
@sentry/browser (incl. Tracing) 40.61 kB -0.01% -1 B 🔽
@sentry/browser (incl. Tracing, Replay) 78.98 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 68.62 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 83.67 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 95.82 kB - -
@sentry/browser (incl. Feedback) 41.1 kB +0.01% +1 B 🔺
@sentry/browser (incl. sendFeedback) 29.06 kB - -
@sentry/browser (incl. FeedbackAsync) 33.97 kB - -
@sentry/react 26.11 kB - -
@sentry/react (incl. Tracing) 42.55 kB - -
@sentry/vue 28.92 kB - -
@sentry/vue (incl. Tracing) 42.39 kB - -
@sentry/svelte 24.44 kB - -
CDN Bundle 26.63 kB - -
CDN Bundle (incl. Tracing) 41.15 kB - -
CDN Bundle (incl. Tracing, Replay) 77.44 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 82.9 kB - -
CDN Bundle - uncompressed 78.18 kB - -
CDN Bundle (incl. Tracing) - uncompressed 122.18 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 237.34 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 250.1 kB - -
@sentry/nextjs (client) 44.58 kB - -
@sentry/sveltekit (client) 40.98 kB - -
@sentry/node-core 50.74 kB - -
@sentry/node 154.36 kB -0.01% -1 B 🔽
@sentry/node - without tracing 92.59 kB -0.01% -1 B 🔽
@sentry/aws-serverless 106.32 kB +0.01% +1 B 🔺

View base workflow run

// Mounts are suffixed with a colon, so we need to add it to the set items
const userMounts = new Set((userStorageMounts as string[]).map(m => `${m}:`));

debug.log('[storage] Starting to instrument storage drivers...');
Copy link
Member

Choose a reason for hiding this comment

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

You probably want to show this log conditionally based on debug: true (also applies to other debug statements).

Copy link
Collaborator Author

@logaretm logaretm Oct 10, 2025

Choose a reason for hiding this comment

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

You mean with DEBUG && ....? I don't think it works in server-added plugins as I tried and it fails to build. It also doesn't log anything when debug is false.

Comment on lines 201 to 203
'nuxt.storage.op': methodName,
'nuxt.storage.mount': mountBase,
'nuxt.storage.driver': driver.name ?? 'unknown',
Copy link
Member

Choose a reason for hiding this comment

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

Those attribute names are new - just wondering what's the decision process behind using those. We have some pre-defined attribute names for databases (probably only partially applicable): https://develop.sentry.dev/sdk/telemetry/traces/span-operations/#database

Copy link
Collaborator Author

@logaretm logaretm Oct 10, 2025

Choose a reason for hiding this comment

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

just wondering what's the decision process behind using those

None! I maybe understood that if we don't have a matching attribute name we could create our own. Sorry about that.

I took a read at this and I think we can use these instead:

    'db.operation.name': methodName,
    'db.collection.name	': mountBase,
    'db.system.name': driver.name ?? 'unknown',


const spanName = KEYED_METHODS.has(methodName)
? `${mountBase}${args?.[0]}`
: `storage.${normalizeMethodName(methodName)}`;
Copy link
Member

Choose a reason for hiding this comment

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

afaik, the span names usually look a bit different than span attributes. Using "storage" here is a bit repetitive and my suggestion would be {operation} {affected keys} based on HTTP span names which are `{method} {endpoint}".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense, this will tie in nicely with the multiple key handling for getItems calls.

Copy link
Collaborator Author

@logaretm logaretm Oct 10, 2025

Choose a reason for hiding this comment

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

Played around with a few name formats and {operation} {keys} feels redundant since the span description will contain the operation as well. Or is the redundancy usually desired in span names?

I changed it so it is in line with this:

Not sure if we should repeat the mount name for each cache key if it happens to be a multiple.

CleanShot 2025-10-10 at 13 21 09@2x

Comment on lines 207 to 208
if (args?.[0] && typeof args[0] === 'string') {
attributes[SEMANTIC_ATTRIBUTE_CACHE_KEY] = `${mountBase}${args[0]}`;
Copy link
Member

Choose a reason for hiding this comment

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

is this always just one key or can there be multiple?

Copy link
Collaborator Author

@logaretm logaretm Oct 10, 2025

Choose a reason for hiding this comment

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

If you mean the first arg. yep it is possible, but it would be serialized to a string eg: key1,key2,key3 etc...

and if you mean if args would contain other keys in [1], then no, all the methods signatures place the keys at the first arg.

{
  async hasItem(key, _opts) {},
  async getItem(key, _opts) {},
  async setItem(key, value, _opts) {},
  async removeItem(key, _opts) {},
  async getKeys(base, _opts) {},
  async clear(base, _opts) {},
}

I also missed the possibility of those keys being objects containing a key property, I will add a key normalizer to handle all these cases and format the multiple keys more nicely for the span names.

cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario Requests/s % of Baseline Prev. Requests/s Change %
GET Baseline 8,877 - 8,903 -0%
GET With Sentry 1,389 16% 1,367 +2%
GET With Sentry (error only) 6,253 70% 6,204 +1%
POST Baseline 1,216 - 1,208 +1%
POST With Sentry 511 42% 526 -3%
POST With Sentry (error only) 1,063 87% 1,069 -1%
MYSQL Baseline 3,353 - 3,350 +0%
MYSQL With Sentry 479 14% 438 +9%
MYSQL With Sentry (error only) 2,722 81% 2,715 +0%

View base workflow run

@logaretm logaretm force-pushed the awad/js-1016-nuxtnitro-instrument-kv-storage-instrumenting-unstorage branch from ab3ecc1 to d943f7d Compare October 10, 2025 10:51
Sign up for free to 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.

[Nuxt/Nitro] Instrument KV Storage (instrumenting unstorage)

2 participants