ref(aws-serverless): Streamline aws-sdk instrumentation#21564
Merged
Conversation
Contributor
size-limit report 📦
|
mydea
force-pushed
the
ref/streamline-aws-sdk-sentry-api
branch
from
b0d77f5 to
9d2d4ed
Compare
…ry span APIs Replaces the OpenTelemetry tracer with Sentry's `startInactiveSpan` for span creation in the vendored aws-sdk instrumentation, so aws-sdk spans go through Sentry's sampling/scope/processing pipeline. The span is still activated for the operation via the OTel context (`trace.setSpan` + `context.with`) and, for incoming operations, bound to the result promise - there is no Sentry equivalent for that context plumbing, so those OTel context APIs are kept (same approach as the fs instrumentation). * `_startAwsV3Span` now uses `startInactiveSpan` instead of `this.tracer.startSpan` (in the Node SDK this returns the underlying OTel span, so it still works with the OTel context APIs). * Drop `span.recordException` - Sentry captures errors as separate events, not as span exceptions; the error status is still set via `setStatus`. * `this.tracer` is no longer used, so the now-unused `tracer` (and the likewise unused `config`/`startTime`) parameters are removed from the `responseHook` chain across all service extensions. No change to emitted spans - verified by the dual-version (`@smithy/core` + `@smithy/middleware-stack`, ESM + CJS) integration tests. Part of #20944 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rumentation Removes the blanket `/* eslint-disable */` from every vendored aws-sdk file and makes them pass the (type-aware) linter. Instead of disabling all rules per file, the genuinely-vendored-specific allowances are scoped to a dedicated `.oxlintrc` override (modeled on the existing `node-fetch` vendored override): `no-explicit-any`, `no-unsafe-member-access`, `no-this-alias`, `consistent-type-imports`, `no-unnecessary-type-assertion`, `prefer-for-of`, `unbound-method`, `max-lines`, `complexity`, `no-param-reassign`. This is stricter than the previous blanket disable - rules like `no-floating-promises` are now enforced for these files (and pass). A few real lint findings were fixed inline rather than disabled: unused `config`/`diag` parameters (now `_`-prefixed), an unused catch binding, and `spanName` locals that were declared but never assigned. No change to emitted spans - verified by the dual-version (`@smithy/core` + `@smithy/middleware-stack`, ESM + CJS) integration tests. Part of #20944 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stHook The Sentry SDK only used the vendored instrumentation's `preRequestHook` to set the `auto.otel.aws` span origin. Set the origin directly on the span attributes in the instrumentation instead, and remove the now-unused `preRequestHook` config option (and its hook-info types) entirely. * `_startAwsV3Span` sets `SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN` at span creation. * `awsIntegration` no longer wires a `preRequestHook`. * `AwsSdkInstrumentationConfig` collapses to `InstrumentationConfig` (no Sentry-specific options remain). * `moduleVersion` is no longer threaded into the resolve patch (it was only used by the removed hook). No change to emitted spans - verified by the dual-version integration tests. Part of #20944 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mydea
force-pushed
the
ref/streamline-aws-sdk-sentry-api
branch
from
a8dfb9d to
b5dde44
Compare
mydea
requested review from
JPeer264 and
andreiborza
and removed request for
a team
mydea
changed the title
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
2 participants
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.
Steps 3 + 4 of streamlining the vendored
@opentelemetry/instrumentation-aws-sdk(#20944), on top of the unused-code removal in #21563.Step 4 — migrate to Sentry span APIs
_startAwsV3Spannow creates spans viastartInactiveSpanfrom@sentry/coreinstead ofthis.tracer.startSpan, so aws-sdk spans go through Sentry's sampling/scope/processing pipeline. In the Node SDK this returns the underlying OTel span, so it's still activated for the operation via the OTel context (trace.setSpan+context.with) and bound to the result promise for incoming operations — there's no Sentry equivalent for that context plumbing, so those OTel context APIs are kept (same approach as the fs instrumentation).span.recordException(Sentry captures errors as separate events; the error status is still set viasetStatus).this.traceris no longer used, so the now-unusedtracer/config/startTimeparams are removed from theresponseHookchain across all service extensions.Step 3 — remove
eslint-disable, pass the linter/* eslint-disable */from all vendored files..oxlintrcoverride (modeled on thenode-fetchvendored override) instead of disabling everything per-file. This is stricter than before — e.g.no-floating-promisesis now enforced for these files (and passes).config/diagparams (_-prefixed), an unused catch binding, and never-assignedspanNamelocals.Verification
No change to emitted spans, verified by the dual-version (
@smithy/core+@smithy/middleware-stack, ESM + CJS) integration tests (4/4) and the@sentry/aws-serverlessunit tests (72/72). Lint is 0 errors / 0 warnings.Closes #20944