feat(server-utils): Port Bedrock aws-sdk extension and add integration tests#22166
Draft
andreiborza wants to merge 2 commits into
Draft
feat(server-utils): Port Bedrock aws-sdk extension and add integration tests#22166andreiborza wants to merge 2 commits into
andreiborza wants to merge 2 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
|
|
||
| if (request.commandInput?.body) { | ||
| const requestBody = JSON.parse(request.commandInput.body); |
There was a problem hiding this comment.
InvokeModel body not decoded
Medium Severity
_requestPreSpanHookInvokeModel passes commandInput.body directly to JSON.parse. The AWS SDK often supplies body as a Uint8Array (or Buffer), not a string; unlike the response path, the request path does not decode binary first, so parsing fails or request gen_ai attributes are missing.
Reviewed by Cursor Bugbot for commit cccdbe5. Configure here.
andreiborza
force-pushed
the
ab/aws-sdk-serverless
branch
from
bdc838e to
9297436
Compare
andreiborza
force-pushed
the
ab/aws-sdk-bedrock
branch
2 times, most recently
from
188ac82 to
ff165d4
Compare
andreiborza
force-pushed
the
ab/aws-sdk-serverless
branch
from
9297436 to
31816fd
Compare
andreiborza
force-pushed
the
ab/aws-sdk-bedrock
branch
from
ff165d4 to
a4dd353
Compare
andreiborza
force-pushed
the
ab/aws-sdk-serverless
branch
from
31816fd to
0b89e37
Compare
Contributor
size-limit report 📦
|
andreiborza
force-pushed
the
ab/aws-sdk-bedrock
branch
from
a4dd353 to
968db2f
Compare
andreiborza
force-pushed
the
ab/aws-sdk-serverless
branch
from
0b89e37 to
a3978fa
Compare
andreiborza
force-pushed
the
ab/aws-sdk-bedrock
branch
from
968db2f to
5f2a15f
Compare
andreiborza
force-pushed
the
ab/aws-sdk-serverless
branch
2 times, most recently
from
809da29 to
8907ff9
Compare
andreiborza
force-pushed
the
ab/aws-sdk-bedrock
branch
from
5f2a15f to
8f3599b
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8f3599b. Configure here.
| } | ||
| } | ||
|
|
||
| function recordCohereRAttributes(parsedChunk: any, span: Span): void { |
There was a problem hiding this comment.
Undocumented any in bedrock
Low Severity
New production code uses any for streamed chunk parsing and model-specific record helpers (and an index signature on ConverseStreamOutput) without comments explaining why stricter typing is not used, which the SDK review rules require for each any occurrence.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 8f3599b. Configure here.
andreiborza
force-pushed
the
ab/aws-sdk-serverless
branch
from
8907ff9 to
c0e10d6
Compare
andreiborza
force-pushed
the
ab/aws-sdk-bedrock
branch
from
8f3599b to
0a944ab
Compare
andreiborza
force-pushed
the
ab/aws-sdk-serverless
branch
from
c0e10d6 to
561e41b
Compare
andreiborza
force-pushed
the
ab/aws-sdk-bedrock
branch
from
0a944ab to
5506257
Compare
…n tests Ports the BedrockRuntime gen_ai service extension from the OTel aws-sdk integration to the orchestrion channel integration: chat span name and request parameters for `Converse`/`ConverseStream`, per-model-family request body parsing for `InvokeModel`/`InvokeModelWithResponseStream` (titan, nova, claude, llama, cohere, mistral), token usage and finish reasons from responses, and deferred span end for the streaming commands (the span ends when the wrapped stream is consumed). Adds nock-mocked integration tests for the non-streaming `Converse` and `InvokeModel` commands, asserted against both the OTel and diagnostics-channel paths. The streaming commands use the AWS binary eventstream framing which nock cannot mock, so they stay untested here. This completes orchestrion parity with the OTel aws-sdk integration. Fixes #20946 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
andreiborza
force-pushed
the
ab/aws-sdk-serverless
branch
from
561e41b to
8ffb46b
Compare
andreiborza
force-pushed
the
ab/aws-sdk-bedrock
branch
from
5506257 to
14699e8
Compare
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.
Ports the BedrockRuntime gen_ai service extension from the OTel aws-sdk integration to the orchestrion channel integration: chat span name and request parameters for
Converse/ConverseStream, per-model-family request body parsing forInvokeModel/InvokeModelWithResponseStream(titan, nova, claude, llama, cohere, mistral), token usage and finish reasons from responses, and deferred span end for the streaming commands (the span ends when the wrapped stream is consumed).Adds nock-mocked integration tests for the non-streaming
ConverseandInvokeModelcommands, asserted against both the OTel and diagnostics-channel paths. The streaming commands use the AWS binary eventstream framing which nock cannot mock, so they stay untested here.This completes orchestrion parity with the OTel aws-sdk integration.
User-facing differences to the vendored OTel instrumentation
ConverseStreamcallers receive the original response object with itsstreamwrapped in place, instead of a new object (a channel subscriber can't replace the resolved value); relevant only to identity-sensitive codeFixes #20946