Skip to content

Commit 05ab207

Browse files
CopilotLms24
andauthored
fix(node-integration-tests): Fix flaky kafkajs test race condition (#20189)
The kafkajs integration test asserted producer and consumer transactions in a fixed order, but they can arrive in either order due to Kafka's async nature. To fix the flake, we collect both transactions via callbacks, then assert after both have arrived using `find()` by transaction name instead of relying on arrival order closes #20121 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
1 parent a3a662d commit 05ab207

File tree

1 file changed

+44
-13
lines changed
  • dev-packages/node-integration-tests/suites/tracing/kafkajs

1 file changed

+44
-13
lines changed

dev-packages/node-integration-tests/suites/tracing/kafkajs/test.ts

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { TransactionEvent } from '@sentry/core';
12
import { afterAll, describe, expect } from 'vitest';
23
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';
34

@@ -8,16 +9,50 @@ describe('kafkajs', () => {
89

910
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
1011
test('traces producers and consumers', { timeout: 60_000 }, async () => {
12+
// The producer and consumer transactions can arrive in any order,
13+
// so we collect them and assert after both have been received.
14+
const receivedTransactions: TransactionEvent[] = [];
15+
1116
await createRunner()
1217
.withDockerCompose({
1318
workingDirectory: [__dirname],
1419
readyMatches: ['9092'],
1520
})
1621
.expect({
17-
transaction: {
18-
transaction: 'send test-topic',
19-
contexts: {
20-
trace: expect.objectContaining({
22+
transaction: (transaction: TransactionEvent) => {
23+
receivedTransactions.push(transaction);
24+
},
25+
})
26+
.expect({
27+
transaction: (transaction: TransactionEvent) => {
28+
receivedTransactions.push(transaction);
29+
30+
const producer = receivedTransactions.find(
31+
t => t.contexts?.trace?.data?.['sentry.origin'] === 'auto.kafkajs.otel.producer',
32+
);
33+
const consumer = receivedTransactions.find(
34+
t => t.contexts?.trace?.data?.['sentry.origin'] === 'auto.kafkajs.otel.consumer',
35+
);
36+
37+
expect(producer).toBeDefined();
38+
expect(consumer).toBeDefined();
39+
40+
for (const t of [producer, consumer]) {
41+
// just to assert on the basic shape (for more straight-forward tests, this is usually done by the runner)
42+
expect(t).toMatchObject({
43+
event_id: expect.any(String),
44+
timestamp: expect.anything(),
45+
start_timestamp: expect.anything(),
46+
spans: expect.any(Array),
47+
type: 'transaction',
48+
});
49+
}
50+
51+
expect(producer!.transaction).toBe('send test-topic');
52+
expect(consumer!.transaction).toBe('process test-topic');
53+
54+
expect(producer!.contexts?.trace).toMatchObject(
55+
expect.objectContaining({
2156
op: 'message',
2257
status: 'ok',
2358
data: expect.objectContaining({
@@ -28,14 +63,10 @@ describe('kafkajs', () => {
2863
'sentry.origin': 'auto.kafkajs.otel.producer',
2964
}),
3065
}),
31-
},
32-
},
33-
})
34-
.expect({
35-
transaction: {
36-
transaction: 'process test-topic',
37-
contexts: {
38-
trace: expect.objectContaining({
66+
);
67+
68+
expect(consumer!.contexts?.trace).toMatchObject(
69+
expect.objectContaining({
3970
op: 'message',
4071
status: 'ok',
4172
data: expect.objectContaining({
@@ -46,7 +77,7 @@ describe('kafkajs', () => {
4677
'sentry.origin': 'auto.kafkajs.otel.consumer',
4778
}),
4879
}),
49-
},
80+
);
5081
},
5182
})
5283
.start()

0 commit comments

Comments
 (0)