Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(nextjs): Give app router prefetch requests a http.server.prefetch op #13600

Merged
merged 12 commits into from
Sep 10, 2024
Merged

feat(nextjs): Give app router prefetch requests a http.server.prefetch op #13600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6231881
feat(nextjs): Give app router prefetch requests a `http.server.prefet…
lforst Sep 5, 2024
539cac3
Add test
lforst Sep 6, 2024
529501b
Merge remote-tracking branch 'origin/develop' into lforst-different-o…
lforst Sep 9, 2024
6d4505f
.
lforst Sep 9, 2024
773bb9c
.
lforst Sep 9, 2024
51d3049
.
lforst Sep 9, 2024
a955edf
.
lforst Sep 10, 2024
aafa4f7
.
lforst Sep 10, 2024
af5e87e
.
lforst Sep 10, 2024
344a726
.
lforst Sep 10, 2024
86080e8
no tests :(
lforst Sep 10, 2024
3e98713
.
lforst Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dev-packages/e2e-tests/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"test:prepare": "ts-node prepare.ts",
"test:validate": "run-s test:validate-configuration test:validate-test-app-setups",
"clean": "rimraf tmp node_modules pnpm-lock.yaml && yarn clean:test-applications",
"clean:test-applications": "rimraf test-applications/**/{node_modules,dist,build,.next,.sveltekit,pnpm-lock.yaml} .last-run.json && pnpm store prune"
"clean:test-applications": "rimraf --glob test-applications/**/{node_modules,dist,build,.next,.sveltekit,pnpm-lock.yaml} .last-run.json && pnpm store prune"
},
"devDependencies": {
"@types/glob": "8.0.0",
Expand Down
12 changes: 12 additions & 0 deletions packages/node/src/integrations/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@ export const instrumentHttp = Object.assign(

isolationScope.setTransactionName(bestEffortTransactionName);

if (isKnownPrefetchRequest(req)) {
span.setAttribute('sentry.http.prefetch', true);
}

_httpOptions.instrumentation?.requestHook?.(span, req);
},
responseHook: (span, res) => {
Expand Down Expand Up @@ -275,3 +279,11 @@ function getBreadcrumbData(request: ClientRequest): Partial<SanitizedRequestData
function _isClientRequest(req: ClientRequest | HTTPModuleRequestIncomingMessage): req is ClientRequest {
return 'outputData' in req && 'outputSize' in req && !('client' in req) && !('statusCode' in req);
}

/**
* Detects if an incoming request is a prefetch request.
*/
function isKnownPrefetchRequest(req: HTTPModuleRequestIncomingMessage): boolean {
// Currently only handles Next.js prefetch requests but may check other frameworks in the future.
return req.headers['next-router-prefetch'] === '1';
Copy link
Member

Choose a reason for hiding this comment

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

Can we look at the Sec-Purpose header instead? This should work with all header types.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Purpose

Copy link
Member

Choose a reason for hiding this comment

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

Oh wait it's not standardized 😭 https://chromestatus.com/feature/6247959677108224

}
5 changes: 5 additions & 0 deletions packages/opentelemetry/src/utils/parseSpanDescription.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ export function descriptionForHttpMethod(
break;
}

// Spans for HTTP requests we have determined to be prefetch requests will have a `.prefetch` postfix in the op
if (attributes['sentry.http.prefetch']) {
opParts.push('prefetch');
}

const { urlPath, url, query, fragment, hasRoute } = getSanitizedUrl(attributes, kind);

if (!urlPath) {
Expand Down
Loading