Skip to content

Commit 93ec9f5

Browse files
jcstorms1BridgeAR
andauthored
fix(service-bus) Fix synchronous shimming of tryAddMessage (#7098)
* initial * uncomment test * Update packages/datadog-plugin-azure-service-bus/test/integration-test/tryAddMessageRegressionTest/client.spec.js Co-authored-by: Ruben Bridgewater <ruben@bridgewater.de> * Update packages/datadog-plugin-azure-service-bus/test/integration-test/tryAddMessageRegressionTest/client.spec.js Co-authored-by: Ruben Bridgewater <ruben@bridgewater.de> * remove chai import * Merge branch 'storms/fix-azure-service-bus-try-add-message' of github.com:DataDog/dd-trace-js into storms/fix-azure-service-bus-try-add-message --------- Co-authored-by: Ruben Bridgewater <ruben@bridgewater.de>
1 parent 25b7924 commit 93ec9f5

6 files changed

Lines changed: 108 additions & 4 deletions

File tree

packages/datadog-instrumentations/src/azure-service-bus.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ addHook({ name: '@azure/service-bus', versions: ['>=7.9.2'] }, (obj) => {
3838
shimmer.wrap(batch, 'tryAddMessage', tryAddMessage => function (msg) {
3939
const functionName = tryAddMessage.name
4040
const config = this._context.config
41-
return producerCh.tracePromise(
41+
return producerCh.traceSync(
4242
tryAddMessage, { config, functionName, batch, msg }, this, ...arguments)
4343
})
4444
return batch

packages/datadog-plugin-azure-service-bus/src/producer.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ class AzureServiceBusProducerPlugin extends ProducerPlugin {
7575
asyncEnd (ctx) {
7676
super.finish(ctx)
7777
}
78+
79+
end (ctx) {
80+
super.finish(ctx)
81+
}
7882
}
7983

8084
function injectTraceContext (tracer, span, msg) {

packages/datadog-plugin-azure-service-bus/test/integration-test/client.spec.js renamed to packages/datadog-plugin-azure-service-bus/test/integration-test/core-test/client.spec.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ const {
88
sandboxCwd,
99
useSandbox,
1010
spawnPluginIntegrationTestProc
11-
} = require('../../../../integration-tests/helpers')
12-
const { withVersions } = require('../../../dd-trace/test/setup/mocha')
11+
} = require('../../../../../integration-tests/helpers')
12+
const { withVersions } = require('../../../../dd-trace/test/setup/mocha')
13+
1314
const spawnEnv = { DD_TRACE_FLUSH_INTERVAL: '2000' }
1415

1516
describe('esm', () => {
@@ -18,7 +19,7 @@ describe('esm', () => {
1819

1920
withVersions('azure-service-bus', '@azure/service-bus', version => {
2021
useSandbox([`'@azure/service-bus@${version}'`], false, [
21-
'./packages/datadog-plugin-azure-service-bus/test/integration-test/*'])
22+
'./packages/datadog-plugin-azure-service-bus/test/integration-test/core-test/*'])
2223

2324
beforeEach(async () => {
2425
agent = await new FakeAgent().start()

packages/datadog-plugin-azure-service-bus/test/integration-test/server.mjs renamed to packages/datadog-plugin-azure-service-bus/test/integration-test/core-test/server.mjs

File renamed without changes.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
'use strict'
2+
3+
const {
4+
FakeAgent,
5+
sandboxCwd,
6+
useSandbox,
7+
spawnPluginIntegrationTestProc
8+
} = require('../../../../../integration-tests/helpers')
9+
const { withVersions } = require('../../../../dd-trace/test/setup/mocha')
10+
const assert = require('assert')
11+
12+
const spawnEnv = { DD_TRACE_FLUSH_INTERVAL: '2000' }
13+
14+
describe('esm', () => {
15+
let agent
16+
let proc
17+
18+
withVersions('azure-service-bus', '@azure/service-bus', version => {
19+
useSandbox([`'@azure/service-bus@${version}'`], false, [
20+
'./packages/datadog-plugin-azure-service-bus/test/integration-test/tryAddMessageRegressionTest/*'])
21+
22+
beforeEach(async () => {
23+
agent = await new FakeAgent().start()
24+
process.env.DD_TRACE_DISABLED_PLUGINS = 'amqplib,amqp10,rhea,net'
25+
})
26+
27+
afterEach(async () => {
28+
proc && proc.kill()
29+
await agent.stop()
30+
})
31+
32+
it('tryAddMessage returns a boolean, not a Promise', async () => {
33+
const res = agent.assertMessageReceived(({ headers, payload }) => {
34+
assert.ok(Array.isArray(payload))
35+
assert.strictEqual(payload.length, 3)
36+
// Verify we got the expected spans from the test
37+
assert.strictEqual(payload[0][0].name, 'azure.servicebus.create')
38+
assert.strictEqual(payload[1][0].name, 'azure.servicebus.create')
39+
assert.strictEqual(payload[2][0].name, 'azure.servicebus.send')
40+
})
41+
42+
// This test file will throw an error if tryAddMessage returns a Promise instead of a boolean
43+
proc = await spawnPluginIntegrationTestProc(sandboxCwd(), 'server.mjs', agent.port, spawnEnv)
44+
45+
await res
46+
}).timeout(60000)
47+
})
48+
})
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import 'dd-trace/init.js'
2+
import { ServiceBusClient } from '@azure/service-bus'
3+
4+
const connectionString = 'Endpoint=sb://127.0.0.1;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE;UseDevelopmentEmulator=true;'
5+
const queueName = 'queue.1'
6+
7+
const client = new ServiceBusClient(connectionString)
8+
const sender = client.createSender(queueName)
9+
10+
const messages = [
11+
{ body: 'Test message 1' },
12+
{ body: 'Test message 2' }
13+
]
14+
15+
// Test that tryAddMessage returns a boolean, not a Promise
16+
const batch = await sender.createMessageBatch()
17+
18+
const result1 = batch.tryAddMessage(messages[0])
19+
const result2 = batch.tryAddMessage(messages[1])
20+
21+
// Verify the return types - throw error if not correct
22+
if (typeof result1 !== 'boolean') {
23+
throw new Error(`tryAddMessage should return a boolean, but returned ${typeof result1}`)
24+
}
25+
26+
if (result1 instanceof Promise) {
27+
throw new Error('tryAddMessage should not return a Promise')
28+
}
29+
30+
if (typeof result2 !== 'boolean') {
31+
throw new Error(`tryAddMessage should return a boolean, but returned ${typeof result2}`)
32+
}
33+
34+
if (result2 instanceof Promise) {
35+
throw new Error('tryAddMessage should not return a Promise')
36+
}
37+
38+
// Verify the values are correct
39+
if (result1 !== true) {
40+
throw new Error(`Expected first tryAddMessage to return true, got ${result1}`)
41+
}
42+
43+
if (result2 !== true) {
44+
throw new Error(`Expected second tryAddMessage to return true, got ${result2}`)
45+
}
46+
47+
// Send the batch to complete the operation
48+
await sender.sendMessages(batch)
49+
50+
await sender.close()
51+
await client.close()

0 commit comments

Comments
 (0)