From 0dee82077472214462a0263816e8f0a7a3cd2de6 Mon Sep 17 00:00:00 2001 From: Volodymyr Vreshch Date: Sun, 9 Aug 2026 16:21:29 +0200 Subject: [PATCH] fix: keep the route template, not the concrete path, on server spans url.path is the concrete request target, so on a content route it carried the customer's own file path - the same data the memory-mcp body redaction already protects. When the instrumentation matched a route we now keep the template instead: same grouping value, no user data. url.full repeats the path, so it goes with it. Only when a route matched. An unmatched path (scanner probes, static assets) is not user content and stays verbatim - that is what makes a routing regression visible. Also un-mangles express RegExp routes, which reached OTel as the regex source: /v1/^\/memories\/([^/]+)\/notes\/(.+)$/ -> /v1/memories/:param/notes/:path. Those were unreadable in a trace list and useless as a facet. --- src/span-names.ts | 40 ++++++++++++++++++++++++++ test/span-names.test.ts | 62 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) diff --git a/src/span-names.ts b/src/span-names.ts index 673f903..4f85dd8 100644 --- a/src/span-names.ts +++ b/src/span-names.ts @@ -22,6 +22,24 @@ export function routeFromUrl(url: string): string { return route === '' ? '/' : route; } +/** + * Express routes registered as a RegExp reach OTel as the regex SOURCE: + * `/v1/^\/memories\/([^/]+)\/notes\/(.+)$/`. Unreadable in a trace list and + * unusable as a facet. Turn it back into a route: `/v1/memories/:param/notes/:path`. + * A route with no regex syntax is returned unchanged. + */ +export function readableRoute(route: string): string { + if (!/[\\^$]|\(\[|\(\.\+\)/.test(route)) return route; + const cleaned = route + .replace(/\(\[\^\/\]\+\)/g, ':param') + .replace(/\(\.\+\)/g, ':path') + .replace(/\\\//g, '/') + .replace(/[\\^$]/g, '') + .replace(/\/{2,}/g, '/'); + // The regex source's own closing delimiter leaves a trailing slash behind. + return cleaned.length > 1 && cleaned.endsWith('/') ? cleaned.slice(0, -1) : cleaned; +} + /** * `fetch GET http://host/api/mcps?page=1` -> `GET /api/mcps` (semconv * `{method} {route}`). Returns null for spans that are not \@vercel/otel @@ -76,6 +94,28 @@ export class FetchSpanNameProcessor implements SpanProcessor { if (typeof tool === 'string' && tool !== '' && m.kind === SpanKind.SERVER) { m.name = tool; } + this.redactRequestPath(m); + } + + /** + * `url.path` is the CONCRETE request target, so on a content route it carries the + * customer's own file path (`/v1/memories/m/notes/00_INBOX/2026-08-04_bugs.md`) - + * as revealing as the note body, which is already redacted upstream. When the + * instrumentation matched a route we keep the TEMPLATE instead: same grouping + * value, no user data. `url.full` repeats the path, so it goes with it. + * + * Only when a route matched. An unmatched path (scanner probes, static assets) is + * not user content and stays verbatim - that is what makes a routing regression + * visible. + */ + private redactRequestPath(m: { attributes: Record; kind: SpanKind }): void { + if (m.kind !== SpanKind.SERVER) return; + const route = m.attributes['http.route']; + if (typeof route !== 'string' || route === '') return; + const readable = readableRoute(route); + m.attributes['http.route'] = readable; + if (typeof m.attributes['url.path'] === 'string') m.attributes['url.path'] = readable; + delete m.attributes['url.full']; } forceFlush(): Promise { diff --git a/test/span-names.test.ts b/test/span-names.test.ts index 2a3b475..2f07f17 100644 --- a/test/span-names.test.ts +++ b/test/span-names.test.ts @@ -85,3 +85,65 @@ describe('RSC fold', () => { expect((span as unknown as { name: string }).name).toBe('GET /browse'); }); }); + +describe('readableRoute', () => { + it('turns a stringified express RegExp back into a route', async () => { + const { readableRoute } = await import('../src/span-names.js'); + expect(readableRoute('/v1/^\\/memories\\/([^/]+)\\/notes\\/(.+)$/')).toBe( + '/v1/memories/:param/notes/:path' + ); + expect(readableRoute('/v1/^\\/vaults\\/([^/]+)\\/notes\\/(.+)$/')).toBe( + '/v1/vaults/:param/notes/:path' + ); + }); + + it('leaves an ordinary route untouched', async () => { + const { readableRoute } = await import('../src/span-names.js'); + expect(readableRoute('/api/memories/:id/folders')).toBe('/api/memories/:id/folders'); + expect(readableRoute('/browse')).toBe('/browse'); + }); +}); + +describe('request-path redaction', () => { + const mkSpan = (kind: number, attributes: Record) => + ({ name: 'GET /x', kind, attributes }) as unknown as Span; + + it('replaces a concrete content path with its route template', async () => { + const { SpanKind } = await import('@opentelemetry/api'); + const p = new FetchSpanNameProcessor(); + const attributes: Record = { + 'http.route': '/v1/^\\/memories\\/([^/]+)\\/notes\\/(.+)$/', + 'url.path': '/v1/memories/m/notes/00_INBOX/2026-08-04_bugs-y-decisiones.md', + 'url.full': 'https://memory.agentage.io/v1/memories/m/notes/00_INBOX/2026-08-04_bugs.md', + }; + p.onEnd(mkSpan(SpanKind.SERVER, attributes)); + expect(attributes['url.path']).toBe('/v1/memories/:param/notes/:path'); + expect(attributes['http.route']).toBe('/v1/memories/:param/notes/:path'); + expect(attributes['url.full']).toBeUndefined(); + expect(JSON.stringify(attributes)).not.toContain('decisiones'); + }); + + it('leaves an unmatched path verbatim - probes are not user content', async () => { + const { SpanKind } = await import('@opentelemetry/api'); + const p = new FetchSpanNameProcessor(); + const attributes: Record = { + 'http.route': '', + 'url.path': '/files/index.php', + }; + p.onEnd(mkSpan(SpanKind.SERVER, attributes)); + expect(attributes['url.path']).toBe('/files/index.php'); + }); + + it('leaves client spans alone', async () => { + const { SpanKind } = await import('@opentelemetry/api'); + const p = new FetchSpanNameProcessor(); + const attributes: Record = { + 'http.route': '/api/mcps', + 'url.path': '/api/mcps/abc', + 'url.full': 'http://b:3001/api/mcps/abc', + }; + p.onEnd(mkSpan(SpanKind.CLIENT, attributes)); + expect(attributes['url.path']).toBe('/api/mcps/abc'); + expect(attributes['url.full']).toBe('http://b:3001/api/mcps/abc'); + }); +});