Skip to content

feat: Pass parentSampleRate to tracesSampler #15024

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.

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

Merged
merged 53 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
ebcf7bf
Remove unused function
lforst Jan 9, 2025
a83a511
Merge remote-tracking branch 'origin/develop' into lforst-sample-rand
lforst Jan 13, 2025
1646f0b
Write and use sample rand on propagation context
lforst Jan 13, 2025
7fbba52
Implement propagating sampling decision properly
lforst Jan 13, 2025
0b12615
Inject sample rand
lforst Jan 13, 2025
1eac9af
Merge remote-tracking branch 'origin/develop' into lforst-sample-rand
lforst Jan 13, 2025
38fe823
format
lforst Jan 13, 2025
4fb937f
put onto initial dsc
lforst Jan 14, 2025
6c27931
Add tests for `tracesSampleRate` behavior
lforst Jan 14, 2025
8d6984f
mv
lforst Jan 14, 2025
9b394c3
todo comment
lforst Jan 14, 2025
1d4a641
unit tests
lforst Jan 14, 2025
2d35988
node integration tests
lforst Jan 14, 2025
957bc09
browser integration tests
lforst Jan 14, 2025
e1c531f
Fix test
lforst Jan 14, 2025
1fbbf82
Merge remote-tracking branch 'origin/develop' into lforst-sample-rand
lforst Jan 14, 2025
a1d5dd3
test fix
lforst Jan 15, 2025
83ac885
feat: Pass `parentSampleRate` to `tracesSampler`
lforst Jan 15, 2025
045b41f
always apply root span sample rate to dsc
lforst Jan 15, 2025
52ea3d2
e l a b o r a t e
lforst Jan 15, 2025
631ed1b
Dont reinvent util we already have
lforst Jan 15, 2025
37b6ca2
beep boop actually do what I am supposed to beep boop
lforst Jan 15, 2025
5e3f706
don't leak attribute into actual data
lforst Jan 15, 2025
8c83fb9
Merge branch 'lforst-sample-rand' into lforst-parent-sampling-decision
lforst Jan 15, 2025
c4282b8
.
lforst Jan 15, 2025
4a2a7ae
.
lforst Jan 15, 2025
5b81700
.
lforst Jan 15, 2025
9b2afff
tests
lforst Jan 16, 2025
2c0910a
Merge remote-tracking branch 'origin/develop' into lforst-parent-samp…
lforst Jan 16, 2025
6c6febf
something's off
lforst Jan 16, 2025
4092e09
pain
lforst Jan 16, 2025
72c3a64
Merge remote-tracking branch 'origin/develop' into lforst-parent-samp…
lforst Jan 20, 2025
d835de7
otel side of things
lforst Jan 20, 2025
c96c706
help
lforst Jan 20, 2025
fdf0fa7
.
lforst Jan 20, 2025
e9471eb
bless
lforst Jan 20, 2025
2b491f2
beep boop
lforst Jan 20, 2025
a96e9c6
lint
lforst Jan 20, 2025
063984b
fix and test 0% sample rate
lforst Jan 20, 2025
8607cc8
apply before emit
lforst Jan 20, 2025
51526ec
Merge branch 'develop' into lforst-parent-sampling-decision
lforst Jan 20, 2025
08b6561
fix merge mistake
lforst Jan 20, 2025
bceb9f3
Refactor again
lforst Jan 21, 2025
e2ffc52
lint
lforst Jan 21, 2025
feb43ee
update tests
lforst Jan 21, 2025
2a127b2
.
lforst Jan 21, 2025
70c3fd9
fix crap
lforst Jan 21, 2025
0ca98e3
update tests
lforst Jan 21, 2025
812e750
more tests
lforst Jan 21, 2025
2882ca4
e2e tests
lforst Jan 21, 2025
f5a8220
.
lforst Jan 21, 2025
fbee088
Merge remote-tracking branch 'origin/develop' into lforst-parent-samp…
lforst Jan 21, 2025
f683884
.
lforst Jan 21, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ sentryTest('should capture an INP click event span after pageload', async ({ bro
'sentry.exclusive_time': inpValue,
'sentry.op': 'ui.interaction.click',
'sentry.origin': 'auto.http.browser.inp',
'sentry.sample_rate': 1,
'sentry.source': 'custom',
transaction: 'test-url',
'user_agent.original': expect.stringContaining('Chrome'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ sentryTest(
'sentry.exclusive_time': inpValue,
'sentry.op': 'ui.interaction.click',
'sentry.origin': 'auto.http.browser.inp',
'sentry.sample_rate': 1,
'sentry.source': 'custom',
transaction: 'test-route',
'user_agent.original': expect.stringContaining('Chrome'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ sentryTest(
const navigationTraceId = navigationTraceContext?.trace_id;
expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`));
expect(headers['baggage']).toEqual(
`sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${navigationTraceHeader?.sample_rand}`,
`sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sampled=true,sentry-sample_rand=${navigationTraceHeader?.sample_rand},sentry-sample_rate=1`,
);
},
);
Expand Down Expand Up @@ -313,7 +313,7 @@ sentryTest(
const navigationTraceId = navigationTraceContext?.trace_id;
expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`));
expect(headers['baggage']).toEqual(
`sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${navigationTraceHeader?.sample_rand}`,
`sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sampled=true,sentry-sample_rand=${navigationTraceHeader?.sample_rand},sentry-sample_rate=1`,
);
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ sentryTest(
// sampling decision is propagated from active span sampling decision
expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`));
expect(headers['baggage']).toBe(
`sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${pageloadTraceHeader?.sample_rand}`,
`sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sampled=true,sentry-sample_rand=${pageloadTraceHeader?.sample_rand},sentry-sample_rate=1`,
);
},
);
Expand Down Expand Up @@ -297,7 +297,7 @@ sentryTest(
// sampling decision is propagated from active span sampling decision
expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`));
expect(headers['baggage']).toBe(
`sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true,sentry-sample_rand=${pageloadTraceHeader?.sample_rand}`,
`sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sampled=true,sentry-sample_rand=${pageloadTraceHeader?.sample_rand},sentry-sample_rate=1`,
);
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => {
data: expect.objectContaining({
'sentry.op': 'pageload',
'sentry.origin': 'auto.pageload.browser',
'sentry.sample_rate': 1,
'sentry.source': 'url',
}),
op: 'pageload',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ test.describe('tracing in static/pre-rendered routes', () => {
data: expect.objectContaining({
'sentry.op': 'pageload',
'sentry.origin': 'auto.pageload.browser',
'sentry.sample_rate': 1,
'sentry.source': 'url',
}),
op: 'pageload',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => {
data: expect.objectContaining({
'sentry.op': 'pageload',
'sentry.origin': 'auto.pageload.browser',
'sentry.sample_rate': 1,
'sentry.source': 'url',
}),
op: 'pageload',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ test.describe('tracing in static routes with server islands', () => {
data: expect.objectContaining({
'sentry.op': 'pageload',
'sentry.origin': 'auto.pageload.browser',
'sentry.sample_rate': 1,
'sentry.source': 'url',
}),
op: 'pageload',
Expand Down Expand Up @@ -75,7 +74,6 @@ test.describe('tracing in static routes with server islands', () => {
data: expect.objectContaining({
'sentry.op': 'http.server',
'sentry.origin': 'auto.http.astro',
'sentry.sample_rate': 1,
'sentry.source': 'route',
}),
op: 'http.server',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ test.describe('tracing in static/pre-rendered routes', () => {
data: expect.objectContaining({
'sentry.op': 'pageload',
'sentry.origin': 'auto.pageload.browser',
'sentry.sample_rate': 1,
'sentry.source': 'url',
}),
op: 'pageload',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ test('Event emitter', async () => {
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
data: {
'sentry.source': 'custom',
'sentry.sample_rate': 1,
'sentry.op': 'event.nestjs',
'sentry.origin': 'auto.event.nestjs',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => {
'sentry.source': 'route',
'sentry.origin': 'auto.http.otel.http',
'sentry.op': 'http.server',
'sentry.sample_rate': 1,
url: `http://localhost:3030/test-inbound-headers/${id}`,
'otel.kind': 'SERVER',
'http.response.status_code': 200,
Expand Down Expand Up @@ -204,7 +203,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => {
'sentry.source': 'route',
'sentry.origin': 'auto.http.otel.http',
'sentry.op': 'http.server',
'sentry.sample_rate': 1,
url: `http://localhost:3030/test-inbound-headers/${id}`,
'otel.kind': 'SERVER',
'http.response.status_code': 200,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ test('Sends a pageload transaction', async ({ page }) => {
data: expect.objectContaining({
'sentry.op': 'pageload',
'sentry.origin': 'auto.pageload.nextjs.app_router_instrumentation',
'sentry.sample_rate': 1,
'sentry.source': 'url',
}),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => {
'sentry.source': 'route',
'sentry.origin': 'auto.http.otel.http',
'sentry.op': 'http.server',
'sentry.sample_rate': 1,
url: `http://localhost:3030/test-inbound-headers/${id}`,
'otel.kind': 'SERVER',
'http.response.status_code': 200,
Expand Down Expand Up @@ -204,7 +203,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => {
'sentry.source': 'route',
'sentry.origin': 'auto.http.otel.http',
'sentry.op': 'http.server',
'sentry.sample_rate': 1,
url: `http://localhost:3030/test-inbound-headers/${id}`,
'otel.kind': 'SERVER',
'http.response.status_code': 200,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => {
'sentry.source': 'route',
'sentry.origin': 'auto.http.otel.http',
'sentry.op': 'http.server',
'sentry.sample_rate': 1,
url: `http://localhost:3030/test-inbound-headers/${id}`,
'otel.kind': 'SERVER',
'http.response.status_code': 200,
Expand Down Expand Up @@ -204,7 +203,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => {
'sentry.source': 'route',
'sentry.origin': 'auto.http.otel.http',
'sentry.op': 'http.server',
'sentry.sample_rate': 1,
url: `http://localhost:3030/test-inbound-headers/${id}`,
'otel.kind': 'SERVER',
'http.response.status_code': 200,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => {
'sentry.source': 'route',
'sentry.origin': 'auto.http.otel.http',
'sentry.op': 'http.server',
'sentry.sample_rate': 1,
url: `http://localhost:3030/test-inbound-headers/${id}`,
'otel.kind': 'SERVER',
'http.response.status_code': 200,
Expand Down Expand Up @@ -204,7 +203,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => {
'sentry.source': 'route',
'sentry.origin': 'auto.http.otel.http',
'sentry.op': 'http.server',
'sentry.sample_rate': 1,
url: `http://localhost:3030/test-inbound-headers/${id}`,
'otel.kind': 'SERVER',
'http.response.status_code': 200,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { parseBaggageHeader } from '@sentry/core';
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
import type { TestAPIResponse } from '../server';

Expand Down Expand Up @@ -102,14 +103,20 @@ test('Should populate and propagate sentry baggage if sentry-trace header does n
const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express');

expect(response).toBeDefined();
expect(response).toMatchObject({
test_data: {
host: 'somewhere.not.sentry',
// TraceId changes, hence we only expect that the string contains the traceid key
baggage: expect.stringMatching(
/sentry-environment=prod,sentry-release=1.0,sentry-public_key=public,sentry-trace_id=[\S]*,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress/,
),
},

const parsedBaggage = parseBaggageHeader(response?.test_data.baggage);

expect(response?.test_data.host).toBe('somewhere.not.sentry');
expect(parsedBaggage).toStrictEqual({
'sentry-environment': 'prod',
'sentry-release': '1.0',
'sentry-public_key': 'public',
// TraceId changes, hence we only expect that the string contains the traceid key
'sentry-trace_id': expect.stringMatching(/[\S]*/),
'sentry-sample_rand': expect.stringMatching(/[\S]*/),
'sentry-sample_rate': '1',
'sentry-sampled': 'true',
'sentry-transaction': 'GET /test/express',
});
});

Expand All @@ -123,13 +130,18 @@ test('Should populate Sentry and ignore 3rd party content if sentry-trace header
});

expect(response).toBeDefined();
expect(response).toMatchObject({
test_data: {
host: 'somewhere.not.sentry',
// TraceId changes, hence we only expect that the string contains the traceid key
baggage: expect.stringMatching(
/sentry-environment=prod,sentry-release=1.0,sentry-public_key=public,sentry-trace_id=[\S]*,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress/,
),
},
expect(response?.test_data.host).toBe('somewhere.not.sentry');

const parsedBaggage = parseBaggageHeader(response?.test_data.baggage);
expect(parsedBaggage).toStrictEqual({
'sentry-environment': 'prod',
'sentry-release': '1.0',
'sentry-public_key': 'public',
// TraceId changes, hence we only expect that the string contains the traceid key
'sentry-trace_id': expect.stringMatching(/[\S]*/),
'sentry-sample_rand': expect.stringMatching(/[\S]*/),
'sentry-sample_rate': '1',
'sentry-sampled': 'true',
'sentry-transaction': 'GET /test/express',
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ test('envelope header for error event during active unsampled span is correct',
public_key: 'public',
environment: 'production',
release: '1.0',
sample_rate: '0',
sampled: 'false',
sample_rand: expect.any(String),
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { cleanupChildProcesses, createRunner } from '../../utils/runner';
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';

describe('sample_rand propagation', () => {
afterAll(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
const Sentry = require('@sentry/node');

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
transport: loggingTransport,
});

// express must be required after Sentry is initialized
const express = require('express');
const cors = require('cors');
const {
startExpressServerAndSendPortToRunner,
getPortAppIsRunningOn,
} = require('@sentry-internal/node-integration-tests');

const app = express();

app.use(cors());

app.get('/check', (req, res) => {
const appPort = getPortAppIsRunningOn(app);
// eslint-disable-next-line no-undef
fetch(`http://localhost:${appPort}/bounce`)
.then(r => r.json())
.then(bounceRes => {
res.json({ propagatedData: bounceRes });
});
});

app.get('/bounce', (req, res) => {
res.json({
baggage: req.headers['baggage'],
});
});

Sentry.setupExpressErrorHandler(app);

startExpressServerAndSendPortToRunner(app);
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';

describe('parentSampleRate propagation with no tracing enabled', () => {
afterAll(() => {
cleanupChildProcesses();
});

test('should propagate an incoming sample rate', async () => {
const runner = createRunner(__dirname, 'server.js').start();
const response = await runner.makeRequest('get', '/check', {
headers: {
'sentry-trace': '530699e319cc067ce440315d74acb312-414dc2a08d5d1dac-1',
baggage: 'sentry-sample_rate=0.1337',
},
});

expect((response as any).propagatedData.baggage).toMatch(/sentry-sample_rate=0\.1337/);
});

test('should not propagate a sample rate for root traces', async () => {
const runner = createRunner(__dirname, 'server.js').start();
const response = await runner.makeRequest('get', '/check');
expect((response as any).propagatedData.baggage).not.toMatch(/sentry-sample_rate/);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
const Sentry = require('@sentry/node');

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
transport: loggingTransport,
tracesSampleRate: 0,
});

// express must be required after Sentry is initialized
const express = require('express');
const cors = require('cors');
const {
startExpressServerAndSendPortToRunner,
getPortAppIsRunningOn,
} = require('@sentry-internal/node-integration-tests');

const app = express();

app.use(cors());

app.get('/check', (req, res) => {
const appPort = getPortAppIsRunningOn(app);
// eslint-disable-next-line no-undef
fetch(`http://localhost:${appPort}/bounce`)
.then(r => r.json())
.then(bounceRes => {
res.json({ propagatedData: bounceRes });
});
});

app.get('/bounce', (req, res) => {
res.json({
baggage: req.headers['baggage'],
});
});

Sentry.setupExpressErrorHandler(app);

startExpressServerAndSendPortToRunner(app);
Loading
Loading