ref(node): Vendor amqplib instrumentation#21003
Open
nicohrubec wants to merge 6 commits into
Open
Conversation
Closes #20144 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8573651. Configure here.
v2 is not within the supported version range (>=0.5.5 <2). The existing test covers v0.10.x, this one covers v1.x. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
size-limit report 📦
|
Avoids conflict with the existing amqplib test's rabbitmq container. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+8
to
+10
| export interface Connection { | ||
| connection: { serverProperties: { product?: string; [key: string]: any } }; | ||
| [key: string]: any; |
Contributor
There was a problem hiding this comment.
Bug: The code accesses conn.serverProperties directly, but the actual amqplib connection object nests this property under conn.connection.serverProperties, causing a TypeError.
Severity: CRITICAL
Suggested Fix
In utils.ts, change the access from conn.serverProperties.product?.toLowerCase?.() to conn.connection.serverProperties.product?.toLowerCase?.() to match the actual structure of the amqplib Connection object.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/node/src/integrations/tracing/amqplib/vendored/amqplib-types.ts#L8-L10
Potential issue: The `getConnectionAttributesFromServer` function in `utils.ts` attempts
to access `conn.serverProperties.product`. However, the `Connection` object from
`amqplib` has a nested structure where server properties are located at
`conn.connection.serverProperties`. Accessing `conn.serverProperties` directly results
in `undefined`, which will cause a `TypeError: Cannot read property 'product' of
undefined` when the code attempts to access the `product` property. This error will
occur every time a RabbitMQ connection is established, crashing the amqplib
instrumentation.
Also affects:
packages/node/src/integrations/tracing/amqplib/vendored/utils.ts:105~107
Did we get this right? 👍 / 👎 to inform future reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Vendors
@opentelemetry/instrumentation-amqplibinto the SDK with no logic changes. Uses latest upstream v0.65.0 which extends supported versions to include amqplib v1.x. Types from@types/amqplibare inlined as simplified interfaces to avoid requiring the package as a dependency. Adds integration test for amqplib v1.x.Closes #20144