Uh oh!
There was an error while loading. Please reload this page.
ref(aws-serverless): Remove unused parts of vendored aws-sdk instrumentation - #21563
Merged
Conversation
…ntation First step of streamlining the vendored `@opentelemetry/instrumentation-aws-sdk`: remove code paths the Sentry SDK never exercises (it only ever sets `preRequestHook`). No change to emitted spans - verified by the existing dual-version (`@smithy/core` + `@smithy/middleware-stack`, ESM + CJS) integration tests. Removed: * Unused config options + their code paths: `responseHook`, `exceptionHook`, `suppressInternalInstrumentation`, `dynamoDBStatementSerializer`, `sqsExtractContextPropagationFromPayload` (and the now-dead hook-info types). * OTel metrics: `_updateMetricInstruments`, the per-service `updateMetricInstruments` hook, and the Bedrock token-usage / operation-duration histograms - Sentry does not consume OTel metrics from this instrumentation. * Semconv stability dual-emission: the `OTEL_SEMCONV_STABILITY_OPT_IN`-driven OLD-vs-STABLE branching for http/db attributes. The STABLE branch was off by default, so we keep the default (OLD) attribute set unchanged (`http.status_code`, `db.system`/`db.name`/`db.operation`). `servicesExtensions` is now initialized directly in the constructor (it was previously initialized as a side effect of the removed `_updateMetricInstruments` override). Part of #20944 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
size-limit report 📦
|
mydea
marked this pull request as ready for review
June 16, 2026 08:35
mydea
requested review from
JPeer264 and andreiborza
and removed request for
a teamJune 16, 2026 08:35
JPeer264
approved these changes
Jun 16, 2026
Uh oh!
There was an error while loading. Please reload this page.
mydea
enabled auto-merge (squash)
June 16, 2026 09:14
Uh oh!
There was an error while loading. Please reload this page.
mydea added a commit
that referenced
this pull request
Jun 17, 2026
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 - `_startAwsV3Span` now creates spans via `startInactiveSpan` from `@sentry/core` instead of `this.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). - Dropped `span.recordException` (Sentry captures errors as separate events; the error status is still set via `setStatus`). - `this.tracer` is no longer used, so the now-unused `tracer`/`config`/`startTime` params are removed from the `responseHook` chain across all service extensions. ## Step 3 — remove `eslint-disable`, pass the linter - Removed the blanket `/* eslint-disable */` from all vendored files. - Scoped the genuinely-vendored allowances to a dedicated `.oxlintrc` override (modeled on the `node-fetch` vendored override) instead of disabling everything per-file. This is **stricter** than before — e.g. `no-floating-promises` is now enforced for these files (and passes). - Fixed a few real findings inline rather than disabling: unused `config`/`diag` params (`_`-prefixed), an unused catch binding, and never-assigned `spanName` locals. ## 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-serverless` unit tests (72/72). Lint is 0 errors / 0 warnings. Closes#20944 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
48 tasks
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 freeto 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.
First step of streamlining the vendored
@opentelemetry/instrumentation-aws-sdk(#20944). Removes code paths the Sentry SDK never exercises —awsIntegrationonly ever setspreRequestHook. No change to emitted spans, verified by the existing dual-version (@smithy/core+@smithy/middleware-stack, ESM + CJS) integration tests and the 72@sentry/aws-serverlessunit tests.Removed
responseHook,exceptionHook,suppressInternalInstrumentation,dynamoDBStatementSerializer,sqsExtractContextPropagationFromPayload(plus the now-dead hook-info types). The publicawsIntegrationnever exposed these._updateMetricInstruments, the per-serviceupdateMetricInstrumentshook, and the Bedrock token-usage / operation-duration histograms — Sentry does not consume OTel metrics from this instrumentation.OTEL_SEMCONV_STABILITY_OPT_IN-driven OLD-vs-STABLE branching. The STABLE branch was off by default, so the default (OLD) attribute set is unchanged (http.status_code,db.system/db.name/db.operation).servicesExtensionsis now initialized in the constructor (it was previously a side effect of the removed_updateMetricInstrumentsoverride).Net: +13 / −238.
Follow-up
A second PR will do step 3 (remove
/* eslint-disable */, pass the type-aware linter) + step 4 (replace the OTel tracer API with Sentry'sstartInactiveSpan), on this smaller surface.Part of #20944