From 7e8cc683b01fb1733136cb86867372aff482820e Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Sun, 2 Apr 2023 16:18:54 +0200 Subject: [PATCH 01/12] fixed imports from wrong file --- test/e2e/opentelemetry/instrumentation-node-test.ts | 4 ++-- test/e2e/opentelemetry/instrumentation-node.ts | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/test/e2e/opentelemetry/instrumentation-node-test.ts b/test/e2e/opentelemetry/instrumentation-node-test.ts index d2903d794d5c..b03974c40578 100644 --- a/test/e2e/opentelemetry/instrumentation-node-test.ts +++ b/test/e2e/opentelemetry/instrumentation-node-test.ts @@ -1,11 +1,11 @@ import { Resource } from '@opentelemetry/resources' import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions' +import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node' import { - NodeTracerProvider, SimpleSpanProcessor, SpanExporter, ReadableSpan, -} from '@opentelemetry/sdk-trace-node' +} from '@opentelemetry/sdk-trace-base' import { ExportResult, ExportResultCode, diff --git a/test/e2e/opentelemetry/instrumentation-node.ts b/test/e2e/opentelemetry/instrumentation-node.ts index b44232e29a3d..61f94398ca3a 100644 --- a/test/e2e/opentelemetry/instrumentation-node.ts +++ b/test/e2e/opentelemetry/instrumentation-node.ts @@ -1,9 +1,7 @@ import { Resource } from '@opentelemetry/resources' import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions' -import { - NodeTracerProvider, - SimpleSpanProcessor, -} from '@opentelemetry/sdk-trace-node' +import { SimpleSpanProcessor } from '@opentelemetry/sdk-trace-base' +import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node' // You can use gRPC exporter instead import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http' From 16a8661502c5dfe64e374c8f80793c998c848e15 Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Sun, 2 Apr 2023 16:32:36 +0200 Subject: [PATCH 02/12] revert changes made to otel testing in #47208 --- test/e2e/opentelemetry/opentelemetry.test.ts | 414 +++++++++---------- 1 file changed, 184 insertions(+), 230 deletions(-) diff --git a/test/e2e/opentelemetry/opentelemetry.test.ts b/test/e2e/opentelemetry/opentelemetry.test.ts index 14e9306feb1e..eb12fa5703e8 100644 --- a/test/e2e/opentelemetry/opentelemetry.test.ts +++ b/test/e2e/opentelemetry/opentelemetry.test.ts @@ -23,9 +23,7 @@ createNextDescribe( await check(async () => { const spans = await getTraces() const rootSpans = spans.filter((span) => !span.parentId) - return rootSpans.length >= numberOfRootTraces - ? String(numberOfRootTraces) - : rootSpans.length + return String(rootSpans.length) }, String(numberOfRootTraces)) } @@ -43,7 +41,6 @@ createNextDescribe( return span } const sanitizeSpans = (spans: SavedSpan[]) => { - const seenSpans = new Set() return spans .sort((a, b) => (a.attributes?.['next.span_type'] ?? '').localeCompare( @@ -51,18 +48,6 @@ createNextDescribe( ) ) .map(sanitizeSpan) - .filter((span) => { - const target = span.attributes?.['http.target'] - const result = - !span.attributes?.['http.url']?.startsWith('http://localhost') && - !seenSpans.has(target) - - if (target) { - seenSpans.add(target) - } - - return result - }) } const getSanitizedTraces = async (numberOfRootTraces: number) => { @@ -82,130 +67,118 @@ createNextDescribe( it('should handle RSC with fetch', async () => { await next.fetch('/app/param/rsc-fetch') - await check(async () => { - const traces = await getSanitizedTraces(1) - - for (const entry of [ - { - attributes: { - 'http.method': 'GET', - 'http.url': 'https://vercel.com/', - 'net.peer.name': 'vercel.com', - 'next.span_name': 'fetch GET https://vercel.com/', - 'next.span_type': 'AppRender.fetch', + expect(await getSanitizedTraces(1)).toMatchInlineSnapshot(` + Array [ + Object { + "attributes": Object { + "http.method": "GET", + "http.url": "https://vercel.com/", + "net.peer.name": "vercel.com", + "next.span_name": "fetch GET https://vercel.com/", + "next.span_type": "AppRender.fetch", }, - kind: 2, - name: 'fetch GET https://vercel.com/', - parentId: '[parent-id]', - status: { - code: 0, + "kind": 2, + "name": "fetch GET https://vercel.com/", + "parentId": "[parent-id]", + "status": Object { + "code": 0, }, }, - { - attributes: { - 'next.span_name': 'render route (app) /app/[param]/rsc-fetch', - 'next.span_type': 'AppRender.getBodyResult', + Object { + "attributes": Object { + "next.span_name": "render route (app) /app/[param]/rsc-fetch", + "next.span_type": "AppRender.getBodyResult", }, - kind: 0, - name: 'render route (app) /app/[param]/rsc-fetch', - parentId: '[parent-id]', - status: { - code: 0, + "kind": 0, + "name": "render route (app) /app/[param]/rsc-fetch", + "parentId": "[parent-id]", + "status": Object { + "code": 0, }, }, - { - attributes: { - 'http.method': 'GET', - 'http.route': '/app/[param]/rsc-fetch/page', - 'http.status_code': 200, - 'http.target': '/app/param/rsc-fetch', - 'next.route': '/app/[param]/rsc-fetch/page', - 'next.span_name': 'GET /app/param/rsc-fetch', - 'next.span_type': 'BaseServer.handleRequest', + Object { + "attributes": Object { + "http.method": "GET", + "http.route": "/app/[param]/rsc-fetch/page", + "http.status_code": 200, + "http.target": "/app/param/rsc-fetch", + "next.route": "/app/[param]/rsc-fetch/page", + "next.span_name": "GET /app/param/rsc-fetch", + "next.span_type": "BaseServer.handleRequest", }, - kind: 1, - name: 'GET /app/[param]/rsc-fetch/page', - parentId: undefined, - status: { - code: 0, + "kind": 1, + "name": "GET /app/[param]/rsc-fetch/page", + "parentId": undefined, + "status": Object { + "code": 0, }, }, - { - attributes: { - 'next.route': '/app/[param]/layout', - 'next.span_name': 'generateMetadata /app/[param]/layout', - 'next.span_type': 'ResolveMetadata.generateMetadata', + Object { + "attributes": Object { + "next.route": "/app/[param]/rsc-fetch/page", + "next.span_name": "generateMetadata /app/[param]/rsc-fetch/page", + "next.span_type": "ResolveMetadata.generateMetadata", }, - kind: 0, - name: 'generateMetadata /app/[param]/layout', - parentId: '[parent-id]', - status: { - code: 0, + "kind": 0, + "name": "generateMetadata /app/[param]/rsc-fetch/page", + "parentId": "[parent-id]", + "status": Object { + "code": 0, }, }, - { - attributes: { - 'next.route': '/app/[param]/rsc-fetch/page', - 'next.span_name': - 'generateMetadata /app/[param]/rsc-fetch/page', - 'next.span_type': 'ResolveMetadata.generateMetadata', + Object { + "attributes": Object { + "next.route": "/app/[param]/layout", + "next.span_name": "generateMetadata /app/[param]/layout", + "next.span_type": "ResolveMetadata.generateMetadata", }, - kind: 0, - name: 'generateMetadata /app/[param]/rsc-fetch/page', - parentId: '[parent-id]', - status: { - code: 0, + "kind": 0, + "name": "generateMetadata /app/[param]/layout", + "parentId": "[parent-id]", + "status": Object { + "code": 0, }, }, - ]) { - expect(traces).toContainEqual(entry) - } - return 'success' - }, 'success') + ] + `) }) it('should handle route handlers in app router', async () => { await next.fetch('/api/app/param/data') - await check(async () => { - const traces = await getSanitizedTraces(1) - - for (const entry of [ - { - attributes: { - 'next.span_name': - 'executing api route (app) /api/app/[param]/data/route', - 'next.span_type': 'AppRouteRouteHandlers.runHandler', + expect(await getSanitizedTraces(1)).toMatchInlineSnapshot(` + Array [ + Object { + "attributes": Object { + "next.span_name": "executing api route (app) /api/app/[param]/data/route", + "next.span_type": "AppRouteRouteHandlers.runHandler", }, - kind: 0, - name: 'executing api route (app) /api/app/[param]/data/route', - parentId: '[parent-id]', - status: { - code: 0, + "kind": 0, + "name": "executing api route (app) /api/app/[param]/data/route", + "parentId": "[parent-id]", + "status": Object { + "code": 0, }, }, - { - attributes: { - 'http.method': 'GET', - 'http.route': '/api/app/[param]/data/route', - 'http.status_code': 200, - 'http.target': '/api/app/param/data', - 'next.route': '/api/app/[param]/data/route', - 'next.span_name': 'GET /api/app/param/data', - 'next.span_type': 'BaseServer.handleRequest', + Object { + "attributes": Object { + "http.method": "GET", + "http.route": "/api/app/[param]/data/route", + "http.status_code": 200, + "http.target": "/api/app/param/data", + "next.route": "/api/app/[param]/data/route", + "next.span_name": "GET /api/app/param/data", + "next.span_type": "BaseServer.handleRequest", }, - kind: 1, - name: 'GET /api/app/[param]/data/route', - parentId: undefined, - status: { - code: 0, + "kind": 1, + "name": "GET /api/app/[param]/data/route", + "parentId": undefined, + "status": Object { + "code": 0, }, }, - ]) { - expect(traces).toContainEqual(entry) - } - return 'success' - }, 'success') + ] + `) }) }) @@ -213,156 +186,137 @@ createNextDescribe( it('should handle getServerSideProps', async () => { await next.fetch('/pages/param/getServerSideProps') - await check(async () => { - const traces = await getSanitizedTraces(1) - for (const entry of [ - { - attributes: { - 'http.method': 'GET', - 'http.route': '/pages/[param]/getServerSideProps', - 'http.status_code': 200, - 'http.target': '/pages/param/getServerSideProps', - 'next.route': '/pages/[param]/getServerSideProps', - 'next.span_name': 'GET /pages/param/getServerSideProps', - 'next.span_type': 'BaseServer.handleRequest', + expect(await getSanitizedTraces(1)).toMatchInlineSnapshot(` + Array [ + Object { + "attributes": Object { + "http.method": "GET", + "http.route": "/pages/[param]/getServerSideProps", + "http.status_code": 200, + "http.target": "/pages/param/getServerSideProps", + "next.route": "/pages/[param]/getServerSideProps", + "next.span_name": "GET /pages/param/getServerSideProps", + "next.span_type": "BaseServer.handleRequest", }, - kind: 1, - name: 'GET /pages/[param]/getServerSideProps', - parentId: undefined, - status: { - code: 0, + "kind": 1, + "name": "GET /pages/[param]/getServerSideProps", + "parentId": undefined, + "status": Object { + "code": 0, }, }, - { - attributes: { - 'next.span_name': - 'getServerSideProps /pages/[param]/getServerSideProps', - 'next.span_type': 'Render.getServerSideProps', + Object { + "attributes": Object { + "next.span_name": "getServerSideProps /pages/[param]/getServerSideProps", + "next.span_type": "Render.getServerSideProps", }, - kind: 0, - name: 'getServerSideProps /pages/[param]/getServerSideProps', - parentId: '[parent-id]', - status: { - code: 0, + "kind": 0, + "name": "getServerSideProps /pages/[param]/getServerSideProps", + "parentId": "[parent-id]", + "status": Object { + "code": 0, }, }, - { - attributes: { - 'next.span_name': - 'render route (pages) /pages/[param]/getServerSideProps', - 'next.span_type': 'Render.renderDocument', + Object { + "attributes": Object { + "next.span_name": "render route (pages) /pages/[param]/getServerSideProps", + "next.span_type": "Render.renderDocument", }, - kind: 0, - name: 'render route (pages) /pages/[param]/getServerSideProps', - parentId: '[parent-id]', - status: { - code: 0, + "kind": 0, + "name": "render route (pages) /pages/[param]/getServerSideProps", + "parentId": "[parent-id]", + "status": Object { + "code": 0, }, }, - ]) { - expect(traces).toContainEqual(entry) - } - return 'success' - }, 'success') + ] + `) }) it("should handle getStaticProps when fallback: 'blocking'", async () => { await next.fetch('/pages/param/getStaticProps') - await check(async () => { - const traces = await getSanitizedTraces(1) - - for (const entry of [ - { - attributes: { - 'http.method': 'GET', - 'http.route': '/pages/[param]/getStaticProps', - 'http.status_code': 200, - 'http.target': '/pages/param/getStaticProps', - 'next.route': '/pages/[param]/getStaticProps', - 'next.span_name': 'GET /pages/param/getStaticProps', - 'next.span_type': 'BaseServer.handleRequest', + expect(await getSanitizedTraces(1)).toMatchInlineSnapshot(` + Array [ + Object { + "attributes": Object { + "http.method": "GET", + "http.route": "/pages/[param]/getStaticProps", + "http.status_code": 200, + "http.target": "/pages/param/getStaticProps", + "next.route": "/pages/[param]/getStaticProps", + "next.span_name": "GET /pages/param/getStaticProps", + "next.span_type": "BaseServer.handleRequest", }, - kind: 1, - name: 'GET /pages/[param]/getStaticProps', - parentId: undefined, - status: { - code: 0, + "kind": 1, + "name": "GET /pages/[param]/getStaticProps", + "parentId": undefined, + "status": Object { + "code": 0, }, }, - { - attributes: { - 'next.span_name': - 'getStaticProps /pages/[param]/getStaticProps', - 'next.span_type': 'Render.getStaticProps', + Object { + "attributes": Object { + "next.span_name": "getStaticProps /pages/[param]/getStaticProps", + "next.span_type": "Render.getStaticProps", }, - kind: 0, - name: 'getStaticProps /pages/[param]/getStaticProps', - parentId: '[parent-id]', - status: { - code: 0, + "kind": 0, + "name": "getStaticProps /pages/[param]/getStaticProps", + "parentId": "[parent-id]", + "status": Object { + "code": 0, }, }, - { - attributes: { - 'next.span_name': - 'render route (pages) /pages/[param]/getStaticProps', - 'next.span_type': 'Render.renderDocument', + Object { + "attributes": Object { + "next.span_name": "render route (pages) /pages/[param]/getStaticProps", + "next.span_type": "Render.renderDocument", }, - kind: 0, - name: 'render route (pages) /pages/[param]/getStaticProps', - parentId: '[parent-id]', - status: { - code: 0, + "kind": 0, + "name": "render route (pages) /pages/[param]/getStaticProps", + "parentId": "[parent-id]", + "status": Object { + "code": 0, }, }, - ]) { - expect(traces).toContainEqual(entry) - } - return 'success' - }, 'success') + ] + `) }) it('should handle api routes in pages', async () => { await next.fetch('/api/pages/param/basic') - await check(async () => { - const traces = await getSanitizedTraces(1) - - for (const entry of [ - { - attributes: { - 'http.method': 'GET', - 'http.status_code': 200, - 'http.target': '/api/pages/param/basic', - 'next.span_name': 'GET /api/pages/param/basic', - 'next.span_type': 'BaseServer.handleRequest', + expect(await getSanitizedTraces(1)).toMatchInlineSnapshot(` + Array [ + Object { + "attributes": Object { + "http.method": "GET", + "http.status_code": 200, + "http.target": "/api/pages/param/basic", + "next.span_name": "GET /api/pages/param/basic", + "next.span_type": "BaseServer.handleRequest", }, - kind: 1, - name: 'GET /api/pages/param/basic', - parentId: undefined, - status: { - code: 0, + "kind": 1, + "name": "GET /api/pages/param/basic", + "parentId": undefined, + "status": Object { + "code": 0, }, }, - { - attributes: { - 'next.span_name': - 'executing api route (pages) /api/pages/[param]/basic', - 'next.span_type': 'Node.runHandler', + Object { + "attributes": Object { + "next.span_name": "executing api route (pages) /api/pages/[param]/basic", + "next.span_type": "Node.runHandler", }, - kind: 0, - name: 'executing api route (pages) /api/pages/[param]/basic', - parentId: '[parent-id]', - status: { - code: 0, + "kind": 0, + "name": "executing api route (pages) /api/pages/[param]/basic", + "parentId": "[parent-id]", + "status": Object { + "code": 0, }, }, - ]) { - expect(traces).toContainEqual(entry) - } - return 'success' - }, 'success') + ] + `) }) }) } From 620022acaf6253188f0c8c7611c04145e8eb744b Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Sun, 2 Apr 2023 16:41:35 +0200 Subject: [PATCH 03/12] fix test snapshot order to remove flakes --- test/e2e/opentelemetry/opentelemetry.test.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/test/e2e/opentelemetry/opentelemetry.test.ts b/test/e2e/opentelemetry/opentelemetry.test.ts index eb12fa5703e8..c32e18898921 100644 --- a/test/e2e/opentelemetry/opentelemetry.test.ts +++ b/test/e2e/opentelemetry/opentelemetry.test.ts @@ -42,6 +42,11 @@ createNextDescribe( } const sanitizeSpans = (spans: SavedSpan[]) => { return spans + .sort((a, b) => + (a.attributes?.['next.span_name'] ?? '').localeCompare( + b.attributes?.['next.span_name'] ?? '' + ) + ) .sort((a, b) => (a.attributes?.['next.span_type'] ?? '').localeCompare( b.attributes?.['next.span_type'] ?? '' @@ -115,12 +120,12 @@ createNextDescribe( }, Object { "attributes": Object { - "next.route": "/app/[param]/rsc-fetch/page", - "next.span_name": "generateMetadata /app/[param]/rsc-fetch/page", + "next.route": "/app/[param]/layout", + "next.span_name": "generateMetadata /app/[param]/layout", "next.span_type": "ResolveMetadata.generateMetadata", }, "kind": 0, - "name": "generateMetadata /app/[param]/rsc-fetch/page", + "name": "generateMetadata /app/[param]/layout", "parentId": "[parent-id]", "status": Object { "code": 0, @@ -128,12 +133,12 @@ createNextDescribe( }, Object { "attributes": Object { - "next.route": "/app/[param]/layout", - "next.span_name": "generateMetadata /app/[param]/layout", + "next.route": "/app/[param]/rsc-fetch/page", + "next.span_name": "generateMetadata /app/[param]/rsc-fetch/page", "next.span_type": "ResolveMetadata.generateMetadata", }, "kind": 0, - "name": "generateMetadata /app/[param]/layout", + "name": "generateMetadata /app/[param]/rsc-fetch/page", "parentId": "[parent-id]", "status": Object { "code": 0, From 256848a7ffb0de590d6343d2fce8d1da776545f3 Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Sun, 2 Apr 2023 17:19:29 +0200 Subject: [PATCH 04/12] Handle duplicated spans in dev properly in tests --- test/e2e/opentelemetry/opentelemetry.test.ts | 38 ++++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/test/e2e/opentelemetry/opentelemetry.test.ts b/test/e2e/opentelemetry/opentelemetry.test.ts index c32e18898921..9e3ce328baaf 100644 --- a/test/e2e/opentelemetry/opentelemetry.test.ts +++ b/test/e2e/opentelemetry/opentelemetry.test.ts @@ -10,7 +10,7 @@ createNextDescribe( skipDeployment: true, dependencies: require('./package.json').dependencies, }, - ({ next }) => { + ({ next, isNextDev }) => { const getTraces = async (): Promise => { const traces = await next.readFile(traceFile) return traces @@ -19,14 +19,6 @@ createNextDescribe( .map((line) => JSON.parse(line)) } - const waitForRootSpan = async (numberOfRootTraces: number) => { - await check(async () => { - const spans = await getTraces() - const rootSpans = spans.filter((span) => !span.parentId) - return String(rootSpans.length) - }, String(numberOfRootTraces)) - } - /** * Sanitize (modifies) span to make it ready for snapshot testing. */ @@ -55,9 +47,33 @@ createNextDescribe( .map(sanitizeSpan) } + /** + * Routes can be executed multiple times per request in dev. + */ + const removeDevDuplicateSpans = (spans: SavedSpan[]) => { + const spanNamesSet = new Set() + return spans.filter((span) => { + const spanName = span.attributes?.['next.span_name'] + if (!spanName) return true + if (spanNamesSet.has(spanName)) return false + spanNamesSet.add(spanName) + return true + }) + } + const getSanitizedTraces = async (numberOfRootTraces: number) => { - await waitForRootSpan(numberOfRootTraces) - return sanitizeSpans(await getTraces()) + let traces + await check(async () => { + traces = sanitizeSpans(await getTraces()) + + if (isNextDev) { + traces = removeDevDuplicateSpans(traces) + } + + const rootSpans = traces.filter((span) => !span.parentId) + return String(rootSpans.length) + }, String(numberOfRootTraces)) + return traces } const cleanTraces = async () => { From 40db068eae18c2846cce150a4a7958b8814b410d Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Sun, 2 Apr 2023 17:19:41 +0200 Subject: [PATCH 05/12] Don't trace internal fetches in dev mode --- packages/next/src/server/dev/next-dev-server.ts | 7 ++++++- packages/next/src/server/lib/patch-fetch.ts | 2 ++ packages/next/src/server/lib/trace/tracer.ts | 6 ++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/next/src/server/dev/next-dev-server.ts b/packages/next/src/server/dev/next-dev-server.ts index 1f089c79ec11..f3c52fba9101 100644 --- a/packages/next/src/server/dev/next-dev-server.ts +++ b/packages/next/src/server/dev/next-dev-server.ts @@ -1260,7 +1260,12 @@ export default class DevServer extends Server { const res = await fetch( `http://${this.hostname}:${ipcPort}?method=${ method as string - }&args=${encodeURIComponent(JSON.stringify(args))}` + }&args=${encodeURIComponent(JSON.stringify(args))}`, + { + next: { + hideOTelSpan: true, + } as any, + } ) const body = await res.text() diff --git a/packages/next/src/server/lib/patch-fetch.ts b/packages/next/src/server/lib/patch-fetch.ts index 459603a98697..b65df6f82e56 100644 --- a/packages/next/src/server/lib/patch-fetch.ts +++ b/packages/next/src/server/lib/patch-fetch.ts @@ -50,6 +50,8 @@ export function patchFetch({ 'net.peer.name': url?.hostname, 'net.peer.port': url?.port || undefined, }, + // Untyped option for internal use, to hide traces in dev mode + hideSpan: (init?.next as any)?.hideOTelSpan, }, async () => { const staticGenerationStore = staticGenerationAsyncStorage.getStore() diff --git a/packages/next/src/server/lib/trace/tracer.ts b/packages/next/src/server/lib/trace/tracer.ts index 3a4fb6dbb6f2..f21e815a27e4 100644 --- a/packages/next/src/server/lib/trace/tracer.ts +++ b/packages/next/src/server/lib/trace/tracer.ts @@ -41,6 +41,7 @@ type TracerSpanOptions = Omit & { parentSpan?: Span spanName?: string attributes?: Partial> + hideSpan?: boolean } interface NextTracer { @@ -201,8 +202,9 @@ class NextTracerImpl implements NextTracer { } if ( - !NextVanillaSpanAllowlist.includes(type) && - process.env.NEXT_OTEL_VERBOSE !== '1' + (!NextVanillaSpanAllowlist.includes(type) && + process.env.NEXT_OTEL_VERBOSE !== '1') || + options.hideSpan ) { return fn() } From 2c9ac716d797935a78e25940e00aac15f5afac67 Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Sun, 2 Apr 2023 21:07:10 +0200 Subject: [PATCH 06/12] disable internal fetch otel spans --- packages/next/src/server/next-server.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts index c332883f2a0e..266dde47c752 100644 --- a/packages/next/src/server/next-server.ts +++ b/packages/next/src/server/next-server.ts @@ -1416,6 +1416,9 @@ export default class NextNodeServer extends BaseServer { method: req.method, headers: invokeHeaders as any, redirect: 'manual', + next: { + hideOTelSpan: true, + } as any, ...(req.method !== 'GET' && req.method !== 'HEAD' ? { // @ts-ignore @@ -2246,6 +2249,9 @@ export default class NextNodeServer extends BaseServer { method: req.method, headers: invokeHeaders as any, redirect: 'manual', + next: { + hideOTelSpan: true, + } as any, ...(req.method !== 'GET' && req.method !== 'HEAD' ? { // @ts-ignore From 9a8a41f10b1ade9dc75470cd2815e2f32e58e9d4 Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Mon, 3 Apr 2023 11:02:42 +0200 Subject: [PATCH 07/12] start testing isNextDev --- test/e2e/opentelemetry/opentelemetry.test.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/e2e/opentelemetry/opentelemetry.test.ts b/test/e2e/opentelemetry/opentelemetry.test.ts index bd2b6bddb544..9e3ce328baaf 100644 --- a/test/e2e/opentelemetry/opentelemetry.test.ts +++ b/test/e2e/opentelemetry/opentelemetry.test.ts @@ -11,11 +11,6 @@ createNextDescribe( dependencies: require('./package.json').dependencies, }, ({ next, isNextDev }) => { - // TODO: remove after resolving dev expected behavior - // x-ref: https://github.com/vercel/next.js/pull/47822 - if (isNextDev) { - return it('should skip for dev for now', () => {}) - } const getTraces = async (): Promise => { const traces = await next.readFile(traceFile) return traces From bc8931c14b7d9343867fd5fc580c8aef5f61433c Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Mon, 3 Apr 2023 11:26:50 +0200 Subject: [PATCH 08/12] Fix `next.route` shown in OTel in app dir pages/routes --- packages/next/src/server/next-server.ts | 10 ++++++++-- test/e2e/opentelemetry/opentelemetry.test.ts | 12 ++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts index fbc9e4cb1110..dc1935ab915c 100644 --- a/packages/next/src/server/next-server.ts +++ b/packages/next/src/server/next-server.ts @@ -1054,13 +1054,19 @@ export default class NextNodeServer extends BaseServer { params: Params | null isAppPath: boolean }): Promise { - getTracer().getRootSpanAttributes()?.set('next.route', pathname) + let route = pathname + if (isAppPath) { + // When in App we get page instead of route + route = pathname.replace(/\/[^/]*$/, '') + } + + getTracer().getRootSpanAttributes()?.set('next.route', route) return getTracer().trace( NextNodeServerSpan.findPageComponents, { spanName: `resolving page into components`, attributes: { - 'next.route': pathname, + 'next.route': route, }, }, () => this.findPageComponentsImpl({ pathname, query, params, isAppPath }) diff --git a/test/e2e/opentelemetry/opentelemetry.test.ts b/test/e2e/opentelemetry/opentelemetry.test.ts index 9e3ce328baaf..b6391467fba2 100644 --- a/test/e2e/opentelemetry/opentelemetry.test.ts +++ b/test/e2e/opentelemetry/opentelemetry.test.ts @@ -120,15 +120,15 @@ createNextDescribe( Object { "attributes": Object { "http.method": "GET", - "http.route": "/app/[param]/rsc-fetch/page", + "http.route": "/app/[param]/rsc-fetch", "http.status_code": 200, "http.target": "/app/param/rsc-fetch", - "next.route": "/app/[param]/rsc-fetch/page", + "next.route": "/app/[param]/rsc-fetch", "next.span_name": "GET /app/param/rsc-fetch", "next.span_type": "BaseServer.handleRequest", }, "kind": 1, - "name": "GET /app/[param]/rsc-fetch/page", + "name": "GET /app/[param]/rsc-fetch", "parentId": undefined, "status": Object { "code": 0, @@ -184,15 +184,15 @@ createNextDescribe( Object { "attributes": Object { "http.method": "GET", - "http.route": "/api/app/[param]/data/route", + "http.route": "/api/app/[param]/data", "http.status_code": 200, "http.target": "/api/app/param/data", - "next.route": "/api/app/[param]/data/route", + "next.route": "/api/app/[param]/data", "next.span_name": "GET /api/app/param/data", "next.span_type": "BaseServer.handleRequest", }, "kind": 1, - "name": "GET /api/app/[param]/data/route", + "name": "GET /api/app/[param]/data", "parentId": undefined, "status": Object { "code": 0, From 3079cf3de1a9885630bdc2804e9f6866cccfb625 Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Mon, 3 Apr 2023 12:06:43 +0200 Subject: [PATCH 09/12] Make sure that we always define next/route for all combinations of (pages, app) x (page, api route) --- packages/next/src/server/api-utils/node.ts | 1 + packages/next/src/server/app-render/app-render.tsx | 1 + .../src/server/future/route-modules/app-route/module.ts | 6 +++--- packages/next/src/server/next-server.ts | 1 - packages/next/src/server/render.tsx | 1 + test/e2e/opentelemetry/opentelemetry.test.ts | 8 ++++---- 6 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/next/src/server/api-utils/node.ts b/packages/next/src/server/api-utils/node.ts index 088a31387af5..0a75ff1d47e0 100644 --- a/packages/next/src/server/api-utils/node.ts +++ b/packages/next/src/server/api-utils/node.ts @@ -529,6 +529,7 @@ export async function apiResolver( res.once('pipe', () => (wasPiped = true)) } + getTracer().getRootSpanAttributes()?.set('next.route', page) // Call API route method const apiRouteResult = await getTracer().trace( NodeSpan.runHandler, diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index fe8e995d2cb0..45955573f1f3 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -1226,6 +1226,7 @@ export async function renderToHTMLOrFlight( ) } + getTracer().getRootSpanAttributes()?.set('next.route', pathname) const bodyResult = getTracer().wrap( AppRenderSpan.getBodyResult, { diff --git a/packages/next/src/server/future/route-modules/app-route/module.ts b/packages/next/src/server/future/route-modules/app-route/module.ts index 0d4a03f0baf8..453260198805 100644 --- a/packages/next/src/server/future/route-modules/app-route/module.ts +++ b/packages/next/src/server/future/route-modules/app-route/module.ts @@ -320,13 +320,13 @@ export class AppRouteRouteModule extends RouteModule< } ) + const route = getPathnameFromAbsolutePath(this.resolvedPagePath) + getTracer().getRootSpanAttributes()?.set('next.route', route) return getTracer().trace( AppRouteRouteHandlersSpan.runHandler, { // TODO: propagate this pathname from route matcher - spanName: `executing api route (app) ${getPathnameFromAbsolutePath( - this.resolvedPagePath - )}`, + spanName: `executing api route (app) ${route}`, }, () => handler(wrappedRequest, { diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts index dc1935ab915c..7409d607818f 100644 --- a/packages/next/src/server/next-server.ts +++ b/packages/next/src/server/next-server.ts @@ -1060,7 +1060,6 @@ export default class NextNodeServer extends BaseServer { route = pathname.replace(/\/[^/]*$/, '') } - getTracer().getRootSpanAttributes()?.set('next.route', route) return getTracer().trace( NextNodeServerSpan.findPageComponents, { diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx index ad735e3b3333..787ae425541f 100644 --- a/packages/next/src/server/render.tsx +++ b/packages/next/src/server/render.tsx @@ -1361,6 +1361,7 @@ export async function renderToHTML( } } + getTracer().getRootSpanAttributes()?.set('next.route', renderOpts.pathname) const documentResult = await getTracer().trace( RenderSpan.renderDocument, { diff --git a/test/e2e/opentelemetry/opentelemetry.test.ts b/test/e2e/opentelemetry/opentelemetry.test.ts index b6391467fba2..2cd672954bdd 100644 --- a/test/e2e/opentelemetry/opentelemetry.test.ts +++ b/test/e2e/opentelemetry/opentelemetry.test.ts @@ -184,15 +184,13 @@ createNextDescribe( Object { "attributes": Object { "http.method": "GET", - "http.route": "/api/app/[param]/data", "http.status_code": 200, "http.target": "/api/app/param/data", - "next.route": "/api/app/[param]/data", "next.span_name": "GET /api/app/param/data", "next.span_type": "BaseServer.handleRequest", }, "kind": 1, - "name": "GET /api/app/[param]/data", + "name": "GET /api/app/param/data", "parentId": undefined, "status": Object { "code": 0, @@ -312,13 +310,15 @@ createNextDescribe( Object { "attributes": Object { "http.method": "GET", + "http.route": "/api/pages/[param]/basic", "http.status_code": 200, "http.target": "/api/pages/param/basic", + "next.route": "/api/pages/[param]/basic", "next.span_name": "GET /api/pages/param/basic", "next.span_type": "BaseServer.handleRequest", }, "kind": 1, - "name": "GET /api/pages/param/basic", + "name": "GET /api/pages/[param]/basic", "parentId": undefined, "status": Object { "code": 0, From d77ffa3af5b735579a1359268e53397174a2f49d Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Mon, 3 Apr 2023 12:21:20 +0200 Subject: [PATCH 10/12] make sure that we update `next.span_name` with correct route too --- packages/next/src/server/base-server.ts | 4 +++- test/e2e/opentelemetry/opentelemetry.test.ts | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index 1efd37b671fd..b8cae2cf3037 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -568,11 +568,13 @@ export default abstract class Server { const route = rootSpanAttributes.get('next.route') if (route) { + const newName = `${method} ${route}` span.setAttributes({ 'next.route': route, 'http.route': route, + 'next.span_name': newName, }) - span.updateName(`${method} ${route}`) + span.updateName(newName) } }) ) diff --git a/test/e2e/opentelemetry/opentelemetry.test.ts b/test/e2e/opentelemetry/opentelemetry.test.ts index 2cd672954bdd..3cc235712610 100644 --- a/test/e2e/opentelemetry/opentelemetry.test.ts +++ b/test/e2e/opentelemetry/opentelemetry.test.ts @@ -124,7 +124,7 @@ createNextDescribe( "http.status_code": 200, "http.target": "/app/param/rsc-fetch", "next.route": "/app/[param]/rsc-fetch", - "next.span_name": "GET /app/param/rsc-fetch", + "next.span_name": "GET /app/[param]/rsc-fetch", "next.span_type": "BaseServer.handleRequest", }, "kind": 1, @@ -214,7 +214,7 @@ createNextDescribe( "http.status_code": 200, "http.target": "/pages/param/getServerSideProps", "next.route": "/pages/[param]/getServerSideProps", - "next.span_name": "GET /pages/param/getServerSideProps", + "next.span_name": "GET /pages/[param]/getServerSideProps", "next.span_type": "BaseServer.handleRequest", }, "kind": 1, @@ -264,7 +264,7 @@ createNextDescribe( "http.status_code": 200, "http.target": "/pages/param/getStaticProps", "next.route": "/pages/[param]/getStaticProps", - "next.span_name": "GET /pages/param/getStaticProps", + "next.span_name": "GET /pages/[param]/getStaticProps", "next.span_type": "BaseServer.handleRequest", }, "kind": 1, @@ -314,7 +314,7 @@ createNextDescribe( "http.status_code": 200, "http.target": "/api/pages/param/basic", "next.route": "/api/pages/[param]/basic", - "next.span_name": "GET /api/pages/param/basic", + "next.span_name": "GET /api/pages/[param]/basic", "next.span_type": "BaseServer.handleRequest", }, "kind": 1, From 60da0a877455b017a08b63b73c0c40c608883be2 Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Mon, 3 Apr 2023 12:44:06 +0200 Subject: [PATCH 11/12] make sure that we emmit 2 spans in dev mode --- packages/next/src/server/base-server.ts | 2 ++ test/e2e/opentelemetry/opentelemetry.test.ts | 18 ------------------ 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index b8cae2cf3037..f905c310ce9a 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -543,6 +543,8 @@ export default abstract class Server { 'http.method': method, 'http.target': req.url, }, + // We will fire this from the renderer worker + hideSpan: this.serverOptions.dev && this.isRouterWorker, }, async (span) => this.handleRequestImpl(req, res, parsedUrl).finally(() => { diff --git a/test/e2e/opentelemetry/opentelemetry.test.ts b/test/e2e/opentelemetry/opentelemetry.test.ts index 3cc235712610..2b21dc16838c 100644 --- a/test/e2e/opentelemetry/opentelemetry.test.ts +++ b/test/e2e/opentelemetry/opentelemetry.test.ts @@ -47,29 +47,11 @@ createNextDescribe( .map(sanitizeSpan) } - /** - * Routes can be executed multiple times per request in dev. - */ - const removeDevDuplicateSpans = (spans: SavedSpan[]) => { - const spanNamesSet = new Set() - return spans.filter((span) => { - const spanName = span.attributes?.['next.span_name'] - if (!spanName) return true - if (spanNamesSet.has(spanName)) return false - spanNamesSet.add(spanName) - return true - }) - } - const getSanitizedTraces = async (numberOfRootTraces: number) => { let traces await check(async () => { traces = sanitizeSpans(await getTraces()) - if (isNextDev) { - traces = removeDevDuplicateSpans(traces) - } - const rootSpans = traces.filter((span) => !span.parentId) return String(rootSpans.length) }, String(numberOfRootTraces)) From f0560a9527e89d8d70ef652a1052ca980e4a199e Mon Sep 17 00:00:00 2001 From: Jan Kaifer Date: Mon, 3 Apr 2023 15:10:51 +0200 Subject: [PATCH 12/12] use unpatched fetch instead of adding untyped option to hide otel span --- .../next/src/server/dev/next-dev-server.ts | 19 +++++-------------- packages/next/src/server/lib/patch-fetch.ts | 2 -- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/packages/next/src/server/dev/next-dev-server.ts b/packages/next/src/server/dev/next-dev-server.ts index d694a13f8270..155f37a8ef8e 100644 --- a/packages/next/src/server/dev/next-dev-server.ts +++ b/packages/next/src/server/dev/next-dev-server.ts @@ -136,7 +136,7 @@ export default class DevServer extends Server { private edgeFunctions?: RoutingItem[] private verifyingTypeScript?: boolean private usingTypeScript?: boolean - private originalFetch?: typeof fetch + private originalFetch: typeof fetch private staticPathsCache: LRUCache< string, UnwrapPromise> @@ -187,7 +187,7 @@ export default class DevServer extends Server { Error.stackTraceLimit = 50 } catch {} super({ ...options, dev: true }) - this.persistPatchedGlobals() + this.originalFetch = global.fetch this.renderOpts.dev = true this.renderOpts.appDirDevErrorLogger = (err: any) => this.logErrorWithOriginalStack(err, 'app-dir') @@ -1257,15 +1257,10 @@ export default class DevServer extends Server { private async invokeIpcMethod(method: string, args: any[]): Promise { const ipcPort = process.env.__NEXT_PRIVATE_ROUTER_IPC_PORT if (ipcPort) { - const res = await fetch( + const res = await this.originalFetch( `http://${this.hostname}:${ipcPort}?method=${ method as string - }&args=${encodeURIComponent(JSON.stringify(args))}`, - { - next: { - hideOTelSpan: true, - } as any, - } + }&args=${encodeURIComponent(JSON.stringify(args))}` ) const body = await res.text() @@ -1708,12 +1703,8 @@ export default class DevServer extends Server { return nextInvoke as NonNullable } - private persistPatchedGlobals(): void { - this.originalFetch = global.fetch - } - private restorePatchedGlobals(): void { - global.fetch = this.originalFetch! + global.fetch = this.originalFetch } protected async ensurePage(opts: { diff --git a/packages/next/src/server/lib/patch-fetch.ts b/packages/next/src/server/lib/patch-fetch.ts index b65df6f82e56..459603a98697 100644 --- a/packages/next/src/server/lib/patch-fetch.ts +++ b/packages/next/src/server/lib/patch-fetch.ts @@ -50,8 +50,6 @@ export function patchFetch({ 'net.peer.name': url?.hostname, 'net.peer.port': url?.port || undefined, }, - // Untyped option for internal use, to hide traces in dev mode - hideSpan: (init?.next as any)?.hideOTelSpan, }, async () => { const staticGenerationStore = staticGenerationAsyncStorage.getStore()