feat(server-utils): Add orchestrion aws-sdk channel integration#22142
feat(server-utils): Add orchestrion aws-sdk channel integration#22142andreiborza wants to merge 1 commit into
Conversation
Adds an OTel-free, orchestrion diagnostics-channel implementation of the aws-sdk (v3) instrumentation to `@sentry/server-utils`. The integration subscribes to the `orchestrion:<smithy-pkg>:send` channels the transform injects into the smithy `Client.prototype.send` (`@smithy/core`, `@smithy/smithy-client`, `@aws-sdk/smithy-client`) and emits the same spans as the OTel `@opentelemetry/instrumentation-aws-sdk` integration for every service (S3, DynamoDB, SQS, SNS, Lambda, Kinesis, SecretsManager, StepFunctions, Bedrock), with a distinct `auto.aws.orchestrion.aws-sdk` origin. SQS/SNS/Lambda trace propagation uses Sentry-native `sentry-trace`/`baggage` headers instead of OTel `propagation.inject`. Registered in `channelIntegrations`, so it is reachable via `@sentry/node`'s `experimentalUseDiagnosticsChannelInjection()`. No default behavior changes yet; the `@sentry/aws-serverless` swap follows in the next PR. Co-Authored-By: Claude Opus 4.8 <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 6 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6b85478. Configure here.
| } | ||
|
|
||
| if (request.commandInput?.body) { | ||
| const requestBody = JSON.parse(request.commandInput.body); |
There was a problem hiding this comment.
Binary invoke body parse fails
Medium Severity
InvokeModel request instrumentation calls JSON.parse on commandInput.body directly, while the response path decodes with TextDecoder first. AWS SDK v3 typically supplies the body as Uint8Array/Buffer, so parsing often throws inside safe(), and no span is created for those calls.
Reviewed by Cursor Bugbot for commit 6b85478. Configure here.
|
|
||
| if (request.commandInput?.body) { | ||
| const requestBody = JSON.parse(request.commandInput.body); | ||
| if (modelId.includes('amazon.titan')) { |
There was a problem hiding this comment.
modelId guard missing on includes
Medium Severity
Bedrock hooks call modelId.includes(...) / currentModelId.includes(...) inside branches where body or response data exists, but modelId can still be undefined. That throws inside safe(), skipping span creation or response attributes for otherwise valid calls.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6b85478. Configure here.
| const modelId = response.request.commandInput?.modelId; | ||
| if (!stream || !modelId) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Stream span never ends
High Severity
For InvokeModelWithResponseStream, when isStream is set the channel integration defers span end, but _responseHookInvokeModelWithResponseStream returns early if stream or modelId is missing without calling span.end(). Those spans stay open indefinitely.
Reviewed by Cursor Bugbot for commit 6b85478. Configure here.
| ...extractAttributesFromNormalizedRequest(normalizedRequest), | ||
| ...requestMetadata.spanAttributes, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Missing sentry.op on spans
Medium Severity
AWS channel spans are started with startInactiveSpan and SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, but no sentry.op (via op or SEMANTIC_ATTRIBUTE_SENTRY_OP), unlike peer orchestrion integrations such as ioredis and anthropic.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 6b85478. Configure here.
| * | ||
| * @experimental | ||
| */ | ||
| export const awsChannelIntegration = defineIntegration(_awsChannelIntegration); |
There was a problem hiding this comment.
No integration tests added
Low Severity
This feature PR adds a large new orchestrion AWS SDK channel integration but the diff includes no unit, integration, or E2E test covering span creation, propagation, or service-specific hooks.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 6b85478. Configure here.
|
|
||
| if (Object.keys(attributes).length + headerKeys.length <= MAX_MESSAGE_ATTRIBUTES) { | ||
| for (const key of headerKeys) { | ||
| (attributes as AwsSdkContextObject)[key] = { DataType: 'String', StringValue: headers[key] } as any; |
There was a problem hiding this comment.
Unchecked any cast in attributes
Low Severity
injectPropagationContext assigns message attribute entries with as any and no comment explaining why a safer type is not possible.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 6b85478. Configure here.
size-limit report 📦
|
1 participant
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).Closes #issue_link_here