test(aws-serverless): Add integration coverage for aws-sdk service instrumentation#21543
Closed
mydea wants to merge 2 commits into
Closed
test(aws-serverless): Add integration coverage for aws-sdk service instrumentation#21543mydea wants to merge 2 commits into
mydea wants to merge 2 commits into
Conversation
![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 4dfe534. Configure here.
…strumentation The vendored `@opentelemetry/instrumentation-aws-sdk` had almost no test coverage - only S3 `PutObject` was exercised, leaving DynamoDB, SQS, SNS, Lambda, Kinesis, SecretsManager and StepFunctions (~97% of the service extension code) completely untested. This adds `nock`-based integration tests (mirroring the existing S3 test) covering the span name, op, origin and service-specific attributes for each service the SDK instruments: * S3 - expanded to PutObject, GetObject and an errored GetObject * DynamoDB - PutItem + Query (db.* attributes, op `db`) * SQS - SendMessage + ReceiveMessage (messaging.* attributes, PRODUCER/CONSUMER) * SNS - Publish (messaging.* attributes, topic ARN) * Lambda - Invoke (faas.* attributes) * Kinesis - PutRecord (stream name) * SecretsManager - GetSecretValue (secret ARN) * StepFunctions - StartExecution (state machine ARN) This is step 1 of streamlining the vendored aws-sdk instrumentation - the tests lock in current behavior so the upcoming cleanup and Sentry-API migration can be done safely. Notes: * The new `@aws-sdk/client-*` packages are pinned to `3.1041.0` (matching the existing S3 dep) because newer clients route through `@smithy/core`, which the current instrumentation does not patch. * The Kinesis client is forced onto the HTTP/1 request handler because it defaults to HTTP/2, which `nock` cannot intercept. Addresses step 1 of #20944 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mydea
force-pushed
the
test/aws-sdk-instrumentation-coverage
branch
from
4dfe534 to
4e7eb9a
Compare
mydea
requested review from
JPeer264 and
andreiborza
and removed request for
a team
Member
Author
|
Replaced by #21548 |
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.
Summary
The vendored
@opentelemetry/instrumentation-aws-sdkhad almost no test coverage — only S3PutObjectwas exercised, leaving DynamoDB, SQS, SNS, Lambda, Kinesis, SecretsManager and StepFunctions (~97% of the service-extension code) completely untested.This adds
nock-based integration tests (mirroring the existing S3 test) that lock in the current span output (name, op, origin, and service-specific attributes) for every service the SDK instruments:status: internal_error)op: db,db.*+aws.dynamodb.*)messaging.*, PRODUCER/CONSUMER, message id, batch count)messaging.*, topic ARN)faas.*)This is step 1 of streamlining the vendored aws-sdk instrumentation — establishing a safety net so the subsequent unused-code cleanup, lint fixes, and Sentry-API migration (steps 2–4) can be done with confidence.
Notes for reviewers
@aws-sdk/client-*packages are pinned to3.1041.0(matching the existing S3 dep). Newer clients route their middleware through@smithy/core, which the current instrumentation does not patch, so they aren't instrumented at all — see the follow-up issue for details. Pinning keeps the tests exercising the code paths the instrumentation actually supports today.NodeHttpHandler) because it defaults to HTTP/2, whichnockcannot intercept (it was otherwise silently reaching real AWS).Addresses step 1 of #20944