feat: emit X-Ray subsegment on every lambda:// invoke#203
Conversation
Wrap the v3 Lambda client at its single shared construction point in the lambda-invocation adapter with aws-xray-sdk-core's captureAWSv3Client, so every internal lambda:// service->service invoke emits a subsegment naming the downstream service with trace-header propagation. Because all internal traffic funnels through alpha, this lights up the whole mesh from one place, including the config.Lambda injection path. The wrap is a safe no-op for untraced consumers: it only activates when X-Ray is active (_X_AMZN_TRACE_ID or ALPHA_XRAY_TRACING opt-in), softens context-missing to LOG_ERROR without clobbering an explicit AWS_XRAY_CONTEXT_MISSING, and falls back to the unwrapped client if instrumentation throws. Co-authored-by: Cursor <cursoragent@cursor.com>
PR SummaryMedium Risk Overview A new Reviewed by Cursor Bugbot for commit ff24829. Bugbot is set up for automated code reviews on this repo. Configure here. |
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 e5e7187. Configure here.
| try { | ||
| if (!contextMissingConfigured && !process.env.AWS_XRAY_CONTEXT_MISSING) { | ||
| setContextMissingStrategy('LOG_ERROR'); | ||
| contextMissingConfigured = true; |
There was a problem hiding this comment.
Overwrites prior context strategy
Low Severity
The first traced captureLambdaClient call always invokes setContextMissingStrategy('LOG_ERROR') when AWS_XRAY_CONTEXT_MISSING is unset, even if the host app already configured a different missing-context strategy in code. That contradicts the stated guarantee not to override an earlier setContextMissingStrategy choice.
Reviewed by Cursor Bugbot for commit e5e7187. Configure here.
There was a problem hiding this comment.
Assessment
This PR instruments the shared lambda:// client construction path in src/adapters/lambda-invocation.ts via captureLambdaClient (src/adapters/helpers/tracing.ts), adding aws-xray-sdk-core so traced runtimes emit downstream Lambda subsegments with trace-header propagation. That is worthwhile merge value: it recovers internal service-to-service edges in the org X-Ray graph from a single chokepoint without changing untraced behavior (no wrap unless _X_AMZN_TRACE_ID or ALPHA_XRAY_TRACING=true, invoke-time fallback if wrapping throws, and LOG_ERROR context-missing only when the env var is unset). Scope matches changedFiles: package.json, yarn.lock, src/adapters/helpers/tracing.ts, src/adapters/lambda-invocation.ts, and test/lambda-invocation.test.ts.
Tests
The new X-Ray tracing block in test/lambda-invocation.test.ts exercises active tracing (_X_AMZN_TRACE_ID), injected config.Lambda, the untraced no-op path, and instrumentation failure fallback. Adequate for this adapter surface; no separate integration test is required in-repo for the AWS SDK middleware itself.
CI
lifeomic-probot/semantic-commits is green. No need to withhold approval for pending checks beyond what branch protection enforces.
Security & privacy: skipped — no matching sensitive paths in changedFiles; X-Ray data stays on AWS tracing infrastructure, not a new third-party PHI/PII sink.
Performance & UX: skipped — no matching paths or UI patterns in this diff.
Bugbot / static analysis
Cursor Bugbot posted an informational PR summary (medium risk, shared Lambda construction path) with no line-level defects. Nothing treated as merge-blocking beyond normal post-deploy validation.
- Bugbot — still blocking: none
- Bugbot — addressed / cleared: N/A (summary-only signal)
[suggestion] Consider a test that sets ALPHA_XRAY_TRACING=true without _X_AMZN_TRACE_ID to lock in the documented non-Lambda opt-in path.
[nit] tracing.ts documents that an earlier consumer setContextMissingStrategy call is respected, but the implementation only skips when AWS_XRAY_CONTEXT_MISSING is set; align comment or add a guard if overriding programmatic strategy matters for your consumers.
Residual risks / follow-ups
- Validate in a canary that
GetServiceGraph/ the service graph shows previously missing internallambda://edges after deploy. - All consumers pick up the new runtime dependency; overhead is limited to traced environments because wrapping is gated.
None — because the change is narrowly scoped, well-gated for untraced callers, covered by focused unit tests, and CI is green; remaining risk is operational confirmation in traced production traffic rather than a code defect visible in this diff.
Note: Review generated using Cursor model
composer-2.5.
This review was generated by review-bot.
aws-xray-sdk-core resolves @aws-sdk/types to a release requiring Node 20, which breaks yarn install on the repo's Node 14/16/18 matrix. Pin the transitive dep via resolutions and align tracing docs with env-var-only context-missing behavior; add ALPHA_XRAY_TRACING opt-in test. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Assessment
Re-review at ff248298 (head moved from prior e5e7187). Scope remains package.json, yarn.lock, src/adapters/helpers/tracing.ts, src/adapters/lambda-invocation.ts, and test/lambda-invocation.test.ts. The change instruments the shared lambda:// construction path with captureLambdaClient, gating X-Ray wrapping to active traces (_X_AMZN_TRACE_ID or ALPHA_XRAY_TRACING=true), falling back to the unwrapped client on instrumentation failure, and defaulting context-missing to LOG_ERROR only when AWS_XRAY_CONTEXT_MISSING is unset. That delivers concrete merge value: internal service-to-service Lambda invokes become visible in the org X-Ray service graph from a single chokepoint without altering untraced local/test behavior.
Prior review-bot feedback is addressed on this head: ALPHA_XRAY_TRACING opt-in coverage was added, @aws-sdk/types is pinned via resolutions for Node 14+ install compatibility, and tracing.ts docs now describe env-var-only context-missing configuration (no longer claiming programmatic setContextMissingStrategy is preserved).
Tests
The X-Ray tracing block in test/lambda-invocation.test.ts covers opt-in (ALPHA_XRAY_TRACING), active trace ID, injected config.Lambda, untraced no-op, and instrumentation failure fallback. Adequate for this adapter; post-deploy canary validation of GetServiceGraph edges remains operational follow-up.
CI
lifeomic-probot/semantic-commits is green.
Security & privacy: skipped — no matching sensitive paths in changedFiles; trace data stays on AWS X-Ray infrastructure, not a new third-party PHI/PII sink (§2.9 does not apply).
Performance & UX: skipped — no matching paths or UI patterns in this diff.
Bugbot / static analysis
Cursor Bugbot flagged a low-severity context-missing strategy overwrite on first traced wrap when AWS_XRAY_CONTEXT_MISSING is unset. Docs on this head now correctly steer consumers to the env var; a host that sets strategy only in code before the first traced invoke could still be overwritten — acceptable edge case, not merge-blocking.
- Bugbot — still blocking: none
- Bugbot — addressed / cleared: documentation aligned with env-var-only behavior;
ALPHA_XRAY_TRACINGtest added per prior review-bot suggestion
Residual risks / follow-ups
- Validate in a canary that
GetServiceGraphshows previously missing internallambda://edges after deploy. - All consumers inherit the new runtime dependency; overhead is limited to traced environments because wrapping is gated.
None — because the implementation is narrowly scoped, well-gated for untraced callers, covered by focused unit tests, prior review feedback is resolved on the current head, and CI is green; remaining risk is operational confirmation in traced production traffic rather than a defect visible in this diff.
Note: Review generated using Cursor model
composer-2.5.
This review was generated by review-bot.
|
🎉 This PR is included in version 7.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Wrap the v3 Lambda client at its single shared construction point in the
lambda-invocationadapter withaws-xray-sdk-core'scaptureAWSv3Client, so every internallambda://service-to-service invoke emits an X-Ray subsegment naming the downstream service with trace-header propagation. Because all internal traffic funnels through alpha, this lights up the entire internal mesh from one place.Background
X-Ray is deployed org-wide but
service -> serviceedges are largely uncaptured: alpha's default v3 Lambda client is unwrapped, solambda://invokes are not traced (see the investigation in theecosystem-graphbrief). Instrumenting alpha is the single chokepoint that recovers the internal call graph at runtime, consumed downstream viaGetServiceGraph.Changes
src/adapters/helpers/tracing.ts— newcaptureLambdaClienthelper.src/adapters/lambda-invocation.ts— wrap the constructed client (covers theconfig.Lambdainjection path too).package.json/yarn.lock— addaws-xray-sdk-coreruntime dependency.test/lambda-invocation.test.ts— tracing tests.The wrap is a safe no-op for untraced consumers: it activates only when X-Ray is active (
_X_AMZN_TRACE_IDorALPHA_XRAY_TRACINGopt-in), softens context-missing toLOG_ERRORwithout clobbering an explicitAWS_XRAY_CONTEXT_MISSING, and falls back to the unwrapped client if instrumentation throws.Verification
Note: 4 failures in
test/interceptors/aws-v4-signature.test.tsare pre-existing local AWS-credential resolution issues unrelated to this change (untouched file).Risk / rollback
Adds a runtime dependency and changes the shared Lambda construction path for all consumers. Mitigated by the no-op-when-untraced gating; revert is a single commit. Should be validated in a canary deploy that confirms
GetServiceGraphbegins showing previously-absent internal edges.Made with Cursor