Skip to content

Commit 4cefbaf

Browse files
BridgeARBridgeAR
authored
test: reduce CI flakiness via span-race filters (#8087)
This addresses real span race conditions. Assuming `traces[0][0]` is the span we're looking for across kafkajs, confluentinc-kafka-javascript, redis, ioredis, mariadb, mysql, mysql2, graphql (parse + validate), aws-sdk s3 span-pointers, elasticsearch, opensearch, mongodb-core, pg, prisma, iovalkey, and the IAST `testOutsideRequestHasVulnerability` and `checkVulnerabilityInRequest` helpers is not always correct. Moleculer's `startBroker` now prevents the port race by using the the zero port, and the rhea `before` hook finally returns the `agent.load` promise so mocha actually waits for plugin registration.
1 parent 27c35f7 commit 4cefbaf

19 files changed

Lines changed: 198 additions & 148 deletions

File tree

‎packages/datadog-plugin-aws-sdk/test/s3.spec.js‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ describe('Plugin', () => {
8383
it('should add span pointer for putObject operation', (done) => {
8484
agent.assertSomeTraces(traces => {
8585
try {
86-
const span = traces[0][0]
86+
const span = traces[0].find(s => s.meta?.['aws.operation'] === 'putObject')
87+
assert.ok(span)
8788
const links = JSON.parse(span.meta?.['_dd.span_links'] || '[]')
8889

8990
assert.strictEqual(links.length, 1)
@@ -113,7 +114,8 @@ describe('Plugin', () => {
113114
it('should add span pointer for copyObject operation', (done) => {
114115
agent.assertSomeTraces(traces => {
115116
try {
116-
const span = traces[0][0]
117+
const span = traces[0].find(s => s.meta?.['aws.operation'] === 'copyObject')
118+
assert.ok(span)
117119
const links = JSON.parse(span.meta?.['_dd.span_links'] || '[]')
118120

119121
assert.strictEqual(links.length, 1)
@@ -187,9 +189,8 @@ describe('Plugin', () => {
187189
s3.completeMultipartUpload(completeParams, (err) => {
188190
if (err) done(err)
189191
agent.assertSomeTraces(traces => {
190-
const span = traces[0][0]
191-
const operation = span.meta?.['aws.operation']
192-
if (operation === 'completeMultipartUpload') {
192+
const span = traces[0].find(s => s.meta?.['aws.operation'] === 'completeMultipartUpload')
193+
if (span) {
193194
try {
194195
const links = JSON.parse(span.meta?.['_dd.span_links'] || '[]')
195196
assert.strictEqual(links.length, 1)

‎packages/datadog-plugin-confluentinc-kafka-javascript/test/index.spec.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,8 @@ describe('Plugin', () => {
197197

198198
it('should propagate context', async () => {
199199
const expectedSpanPromise = agent.assertSomeTraces(traces => {
200-
const span = traces[0][0]
200+
const span = traces[0].find(s => s.name === 'kafka.consume')
201+
assert.ok(span)
201202

202203
assertObjectContains(span, {
203204
name: 'kafka.consume',
@@ -523,7 +524,8 @@ describe('Plugin', () => {
523524

524525
it('should propagate context', async () => {
525526
const expectedSpanPromise = agent.assertSomeTraces(traces => {
526-
const span = traces[0][0]
527+
const span = traces[0].find(s => s.name === 'kafka.consume')
528+
assert.ok(span)
527529

528530
assertObjectContains(span, {
529531
name: 'kafka.consume',

‎packages/datadog-plugin-elasticsearch/test/index.spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ describe('Plugin', () => {
5252
agent
5353
.assertSomeTraces(traces => {
5454
assert.strictEqual(traces[0][0].resource, 'POST /logstash-?.?.?/_search')
55-
})
55+
}, { spanResourceMatch: /^POST \/logstash-\?\.\?\.\?\/_search$/ })
5656
.then(done)
5757
.catch(done)
5858

‎packages/datadog-plugin-graphql/test/index.spec.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ describe('Plugin', () => {
353353
meta: { component: 'graphql' },
354354
})
355355
assert.ok(!('graphql.source' in span.meta))
356-
})
356+
}, { spanResourceMatch: /^graphql\.parse$/ })
357357
.then(done)
358358
.catch(done)
359359

@@ -375,7 +375,7 @@ describe('Plugin', () => {
375375
meta: { component: 'graphql' },
376376
})
377377
assert.ok(!('graphql.source' in span.meta))
378-
})
378+
}, { spanResourceMatch: /^graphql\.validate$/ })
379379
.then(done)
380380
.catch(done)
381381

‎packages/datadog-plugin-ioredis/test/index.spec.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ describe('Plugin', () => {
5252
metrics: {
5353
'network.destination.port': 6379,
5454
},
55-
})
55+
}, { spanResourceMatch: /^get$/ })
5656
})
5757

5858
it('should run the callback in the parent context', () => {
@@ -105,7 +105,7 @@ describe('Plugin', () => {
105105
metrics: {
106106
'network.destination.port': 6379,
107107
},
108-
})
108+
}, { spanResourceMatch: /^get$/ })
109109
})
110110

111111
withNamingSchema(

‎packages/datadog-plugin-iovalkey/test/index.spec.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ describe('Plugin', () => {
3434
afterEach(() => agent.close({ ritmReset: false }))
3535

3636
it('should do automatic instrumentation when using callbacks', async () => {
37-
agent.assertSomeTraces(() => {}) // wait for initial info command
3837
const promise = agent.assertSomeTraces(traces => {
3938
assert.strictEqual(traces[0][0].name, expectedSchema.outbound.opName)
4039
assert.strictEqual(traces[0][0].service, expectedSchema.outbound.serviceName)
@@ -48,7 +47,7 @@ describe('Plugin', () => {
4847
assert.strictEqual(traces[0][0].meta['out.host'], 'localhost')
4948
assert.strictEqual(traces[0][0].meta['valkey.raw_command'], 'GET foo')
5049
assert.strictEqual(traces[0][0].metrics['network.destination.port'], 6379)
51-
})
50+
}, { spanResourceMatch: /^get$/ })
5251

5352
await Promise.all([
5453
valkey.get('foo'),

‎packages/datadog-plugin-kafkajs/test/index.spec.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,8 @@ describe('Plugin', () => {
266266

267267
it('should propagate context', async () => {
268268
const expectedSpanPromise = agent.assertSomeTraces(traces => {
269-
const span = traces[0][0]
269+
const span = traces[0].find(s => s.name === 'kafka.consume')
270+
assert.ok(span)
270271

271272
assertObjectContains(span, {
272273
name: 'kafka.consume',

‎packages/datadog-plugin-mariadb/test/index.spec.js‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ describe('Plugin', () => {
118118
component: 'mariadb',
119119
'_dd.integration': 'mariadb',
120120
},
121-
})
121+
}, { spanResourceMatch: /SELECT 1 \+ 1 AS solution/ })
122122
.then(done)
123123
.catch(done)
124124

@@ -141,7 +141,7 @@ describe('Plugin', () => {
141141
'db.type': 'mariadb',
142142
component: 'mariadb',
143143
},
144-
})
144+
}, { spanResourceMatch: /SELECT \? \+ \? AS solution/ })
145145
.then(done)
146146
.catch(done)
147147

@@ -163,7 +163,7 @@ describe('Plugin', () => {
163163
'db.type': 'mariadb',
164164
component: 'mariadb',
165165
},
166-
})
166+
}, { spanResourceMatch: /SELECT \? \+ \? AS solution/ })
167167
.then(done)
168168
.catch(done)
169169

@@ -201,7 +201,8 @@ describe('Plugin', () => {
201201

202202
it('should work without a callback', done => {
203203
agent
204-
.assertFirstTraceSpan({ resource: 'SELECT 1 + 1 AS solution' })
204+
.assertFirstTraceSpan({ resource: 'SELECT 1 + 1 AS solution' },
205+
{ spanResourceMatch: /SELECT 1 \+ 1 AS solution/ })
205206
.then(done)
206207
.catch(done)
207208

‎packages/datadog-plugin-moleculer/test/index.spec.js‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
'use strict'
22

33
const assert = require('node:assert')
4-
const net = require('node:net')
54
const os = require('node:os')
65

76
const { assertObjectContains } = require('../../../integration-tests/helpers')
@@ -10,19 +9,6 @@ const { withNamingSchema, withPeerService, withVersions } = require('../../dd-tr
109
const { expectedSchema, rawExpectedSchema } = require('./naming')
1110
const sort = trace => trace.sort((a, b) => Number(a.start - b.start))
1211

13-
// The returned port could already be in use by another test that was running at
14-
// the same time. This race condition is not prevented by this function.
15-
function getPort () {
16-
return new Promise((resolve, reject) => {
17-
const server = net.createServer()
18-
server.once('error', reject)
19-
server.listen(0, '127.0.0.1', () => {
20-
const { port } = server.address()
21-
server.close(() => resolve(port))
22-
})
23-
})
24-
}
25-
2612
describe('Plugin', () => {
2713
let broker
2814
let port
@@ -32,13 +18,13 @@ describe('Plugin', () => {
3218
const startBroker = async () => {
3319
const { ServiceBroker } = require(`../../../versions/moleculer@${version}`).get()
3420

35-
port = await getPort()
36-
3721
broker = new ServiceBroker({
3822
namespace: 'multi',
3923
nodeID: `server-${process.pid}`,
4024
logger: false,
41-
transporter: `tcp://127.0.0.1:${port}/server-${process.pid}`,
25+
// Port `0` tells the TCP transporter to ask the kernel for a free
26+
// port; the actual port is read back from the broker after start.
27+
transporter: `tcp://127.0.0.1:0/server-${process.pid}`,
4228
})
4329

4430
broker.createService({
@@ -65,7 +51,8 @@ describe('Plugin', () => {
6551
},
6652
})
6753

68-
return broker.start()
54+
await broker.start()
55+
port = broker.transit.tx.opts.port
6956
}
7057

7158
describe('server', () => {

‎packages/datadog-plugin-mongodb-core/test/mongodb.spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ describe('Plugin', () => {
124124
'out.host': '127.0.0.1',
125125
component: 'mongodb',
126126
},
127-
})
127+
}, { spanResourceMatch: new RegExp(`^insert test\\.${collectionName}$`) })
128128
.then(done)
129129
.catch(done)
130130

0 commit comments

Comments
 (0)