feat(node,serverless): Add server-only context span attributes via processSegmentSpan hooks#20842
feat(node,serverless): Add server-only context span attributes via processSegmentSpan hooks#20842chargome wants to merge 1 commit into
Conversation
size-limit report 📦
|
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 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 2367768. Configure here.
| instrumentAwsLambda(options); | ||
| }, | ||
| processSegmentSpan(span) { | ||
| const { contexts } = getIsolationScope().getScopeData(); |
There was a problem hiding this comment.
AWS Lambda reads context from wrong scope
High Severity
The processSegmentSpan hook reads aws.lambda and aws.cloudwatch.logs contexts from getIsolationScope(), but enhanceScopeWithEnvironmentData in sdk.ts sets these contexts on getCurrentScope() (the current scope obtained inside a withScope call). Since setContext only writes to the specific scope instance and doesn't propagate upward, the isolation scope will never contain these contexts. The GCP integration correctly uses getCurrentScope() for both writing and reading. This means the AWS Lambda and CloudWatch span attributes will never be populated.
Reviewed by Cursor Bugbot for commit 2367768. Configure here.
|
|
||
| expect(span.attributes).toEqual({}); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing integration or E2E tests for feat PR
Low Severity
This is a feat PR but only includes unit tests with mocked scopes. There are no integration or E2E tests to verify that the processSegmentSpan hooks actually populate span attributes in a real SDK setup. Given the scope mismatch bug identified in the AWS Lambda integration, an integration test would likely have caught it.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 2367768. Configure here.
node-core: Mapsmissing_instrumentationcontext in existingnodeContextIntegrationhookaws-serverless: AddsprocessSegmentSpantoawsLambdaIntegrationfor aws attributesgoogle-cloud-serverless: NewgcpContextIntegrationfor gcp attributesServer side part of #20828
closes #20385