Skip to content

Commit 60f483e

Browse files
authored
ref(astro): Put request as SDK processing metadata instead of span data (#10840)
This removes usage of deprecated options to `startSpan`, and removes the `trackHeaders` option to the astro middleware in favor of using the general request data integration that should pick this up if put on the scope as SDK processing metadata.
1 parent e731837 commit 60f483e

File tree

3 files changed

+70
-47
lines changed

3 files changed

+70
-47
lines changed

MIGRATION.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,22 @@
11
# Upgrading from 7.x to 8.x
22

3+
## Removal of `trackHeaders` option for Astro middleware
4+
5+
Instead of opting-in via the middleware config, you can configure if headers should be captured via
6+
`requestDataIntegration` options, which defaults to `true` but can be disabled like this:
7+
8+
```
9+
Sentry.init({
10+
integrations: [
11+
Sentry.requestDataIntegration({
12+
include: {
13+
headers: false
14+
},
15+
}),
16+
],
17+
});
18+
```
19+
320
## Removal of Client-Side health check transaction filters
421

522
The SDK no longer filters out health check transactions by default. Instead, they are sent to Sentry but still dropped

packages/astro/src/server/middleware.ts

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
startSpan,
99
withIsolationScope,
1010
} from '@sentry/node-experimental';
11-
import type { Client, Scope, Span } from '@sentry/types';
11+
import type { Client, Scope, Span, SpanAttributes } from '@sentry/types';
1212
import { addNonEnumerableProperty, objectify, stripUrlQueryAndFragment } from '@sentry/utils';
1313
import type { APIContext, MiddlewareResponseHandler } from 'astro';
1414

@@ -27,15 +27,6 @@ type MiddlewareOptions = {
2727
* @default false (recommended)
2828
*/
2929
trackClientIp?: boolean;
30-
31-
/**
32-
* If true, the headers from the request will be attached to the event by calling `setExtra`.
33-
*
34-
* Only set this to `true` if you're fine with collecting potentially personally identifiable information (PII).
35-
*
36-
* @default false (recommended)
37-
*/
38-
trackHeaders?: boolean;
3930
};
4031

4132
function sendErrorToSentry(e: unknown): unknown {
@@ -63,7 +54,6 @@ type AstroLocalsWithSentry = Record<string, unknown> & {
6354
export const handleRequest: (options?: MiddlewareOptions) => MiddlewareResponseHandler = options => {
6455
const handlerOptions = {
6556
trackClientIp: false,
66-
trackHeaders: false,
6757
...options,
6858
};
6959

@@ -100,13 +90,9 @@ async function instrumentRequest(
10090
baggage: headers.get('baggage'),
10191
},
10292
async () => {
103-
const allHeaders: Record<string, string> = {};
104-
105-
if (options.trackHeaders) {
106-
headers.forEach((value, key) => {
107-
allHeaders[key] = value;
108-
});
109-
}
93+
// We store this on the current scope, not isolation scope,
94+
// because we may have multiple requests nested inside each other
95+
getCurrentScope().setSDKProcessingMetadata({ request: ctx.request });
11096

11197
if (options.trackClientIp) {
11298
getCurrentScope().setUser({ ip_address: ctx.clientAddress });
@@ -117,22 +103,27 @@ async function instrumentRequest(
117103
const source = interpolatedRoute ? 'route' : 'url';
118104
// storing res in a variable instead of directly returning is necessary to
119105
// invoke the catch block if next() throws
106+
107+
const attributes: SpanAttributes = {
108+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.astro',
109+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
110+
method,
111+
url: stripUrlQueryAndFragment(ctx.url.href),
112+
};
113+
114+
if (ctx.url.search) {
115+
attributes['http.query'] = ctx.url.search;
116+
}
117+
118+
if (ctx.url.hash) {
119+
attributes['http.fragment'] = ctx.url.hash;
120+
}
121+
120122
const res = await startSpan(
121123
{
122-
attributes: {
123-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.astro',
124-
},
124+
attributes,
125125
name: `${method} ${interpolatedRoute || ctx.url.pathname}`,
126126
op: 'http.server',
127-
status: 'ok',
128-
data: {
129-
method,
130-
url: stripUrlQueryAndFragment(ctx.url.href),
131-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
132-
...(ctx.url.search && { 'http.query': ctx.url.search }),
133-
...(ctx.url.hash && { 'http.fragment': ctx.url.hash }),
134-
...(options.trackHeaders && { headers: allHeaders }),
135-
},
136127
},
137128
async span => {
138129
const originalResponse = await next();

packages/astro/test/server/middleware.test.ts

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@ describe('sentryMiddleware', () => {
1919
return {} as Span | undefined;
2020
});
2121
const setUserMock = vi.fn();
22+
const setSDKProcessingMetadataMock = vi.fn();
2223

2324
beforeEach(() => {
2425
vi.spyOn(SentryNode, 'getCurrentScope').mockImplementation(() => {
2526
return {
2627
setUser: setUserMock,
2728
setPropagationContext: vi.fn(),
2829
getSpan: getSpanMock,
30+
setSDKProcessingMetadata: setSDKProcessingMetadataMock,
2931
} as any;
3032
});
3133
vi.spyOn(SentryNode, 'getActiveSpan').mockImplementation(getSpanMock);
@@ -60,15 +62,12 @@ describe('sentryMiddleware', () => {
6062
{
6163
attributes: {
6264
'sentry.origin': 'auto.http.astro',
63-
},
64-
data: {
6565
method: 'GET',
6666
url: 'https://mydomain.io/users/123/details',
6767
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
6868
},
6969
name: 'GET /users/[id]/details',
7070
op: 'http.server',
71-
status: 'ok',
7271
},
7372
expect.any(Function), // the `next` function
7473
);
@@ -97,15 +96,12 @@ describe('sentryMiddleware', () => {
9796
{
9897
attributes: {
9998
'sentry.origin': 'auto.http.astro',
100-
},
101-
data: {
10299
method: 'GET',
103100
url: 'http://localhost:1234/a%xx',
104101
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
105102
},
106103
name: 'GET a%xx',
107104
op: 'http.server',
108-
status: 'ok',
109105
},
110106
expect.any(Function), // the `next` function
111107
);
@@ -142,8 +138,8 @@ describe('sentryMiddleware', () => {
142138
});
143139
});
144140

145-
it('attaches client IP and request headers if options are set', async () => {
146-
const middleware = handleRequest({ trackClientIp: true, trackHeaders: true });
141+
it('attaches client IP if `trackClientIp=true`', async () => {
142+
const middleware = handleRequest({ trackClientIp: true });
147143
const ctx = {
148144
request: {
149145
method: 'GET',
@@ -162,17 +158,36 @@ describe('sentryMiddleware', () => {
162158
await middleware(ctx, next);
163159

164160
expect(setUserMock).toHaveBeenCalledWith({ ip_address: '192.168.0.1' });
161+
});
165162

166-
expect(startSpanSpy).toHaveBeenCalledWith(
167-
expect.objectContaining({
168-
data: expect.objectContaining({
169-
headers: {
170-
'some-header': 'some-value',
171-
},
163+
it('attaches request as SDK processing metadata', async () => {
164+
const middleware = handleRequest({});
165+
const ctx = {
166+
request: {
167+
method: 'GET',
168+
url: '/users',
169+
headers: new Headers({
170+
'some-header': 'some-value',
172171
}),
173-
}),
174-
expect.any(Function), // the `next` function
175-
);
172+
},
173+
clientAddress: '192.168.0.1',
174+
params: {},
175+
url: new URL('https://myDomain.io/users/'),
176+
};
177+
const next = vi.fn(() => nextResult);
178+
179+
// @ts-expect-error, a partial ctx object is fine here
180+
await middleware(ctx, next);
181+
182+
expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({
183+
request: {
184+
method: 'GET',
185+
url: '/users',
186+
headers: new Headers({
187+
'some-header': 'some-value',
188+
}),
189+
},
190+
});
176191
});
177192

178193
it('injects tracing <meta> tags into the HTML of a pageload response', async () => {

0 commit comments

Comments
 (0)