Skip to content

fix: externalise @opentelemetry/api during build - #17081

Merged
elliott-with-the-longest-name-on-github merged 1 commit into
version-3from
externalise-opentelemetry
Sep 9, 2026
Merged

fix: externalise @opentelemetry/api during build#17081
elliott-with-the-longest-name-on-github merged 1 commit into
version-3from
externalise-opentelemetry

Conversation

@teemingc

@teemingc teemingc commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

I think this was accidentally added such that it only affected vite dev. This PR moves it out of the dev-only conditional block.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

@pkg-svelte-dev

pkg-svelte-dev Bot commented Sep 9, 2026

Copy link
Copy Markdown

Install the latest version of @sveltejs/kit from 89b8b52:

pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/89b8b524399b893bb4e9dc181fcd12385d5d8f22

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/kit/pr/17081

@changeset-bot

changeset-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 89b8b52

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@svelte-docs-bot

svelte-docs-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

Preview: https://svelte-dev-git-preview-kit-17081-svelte.vercel.app/

@teemingc teemingc mentioned this pull request Sep 9, 2026
Merged
5 of 6 tasks
@svelte-triage-bot

svelte-triage-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

I investigated this failed test run, but the investigation came up empty.

What I tried

  • Called get_github_actions_evidence for PR fix: externalise @opentelemetry/api during build #17081, workflow 34378070789, job 102555788508 before checkout.: Trusted head repository was exactly sveltejs/kit. Complete job log and artifact test-failure-34378070789-21 were downloaded.
  • Fetched refs/pull/17081/head from origin and verified the fetched SHA against both supplied head SHAs.: Fetched and checked out detached 89b8b52, matching the PR and workflow head exactly.
  • Inspected the complete Windows job log, all 10 error contexts, and all downloaded Playwright traces.: The sole final failure was [chromium-build] Errors › unexpected errors reach the client handleError hook at client.test.js:826 on Windows, Node 24, Chrome 151.0.7922.34. browser.newContext failed before the test body/assertions ran because the worker browser had closed. Nine unrelated tests were flaky with the same fixture-level error.
  • Compared origin base 494ac02...PR head, including the exact failing declaration and body.: The PR modifies only packages/kit/src/exports/vite/index.js. The failed test file and exact test body are byte-for-byte unchanged.
  • Ran gh pr list --repo sveltejs/kit --state open --limit 20 --search "sort:created-desc" --json number,title,body,url,files and reviewed all 20 results; diffed plausible PR test: verify handler is exported #17077.: No recent open PR fixes this browser.newContext/browser-process failure. PR test: verify handler is exported #17077 only moves test files and adjusts import paths.
  • Preserved the package prelude and CI build setup, then ran the exact failed test in five separate Playwright processes with one worker and retries disabled.: All five valid chromium-build runs reached the target and passed: 5/5 passes. This establishes flakiness rather than a deterministic PR regression.
  • Repeated the exact failed chromium-build test 100 times in one Playwright process with retries disabled.: 100/100 passed in 2.6 minutes.
  • Ran the exact failed test in chromium-dev 20 times with retries disabled.: 20/20 passed.

The unchanged target is flaky, but its test logic never failed: Windows CI lost the Chrome worker before browser.newContext could create the test context, causing one final failure and nine unrelated retry-pass flakes. Linux cannot reproduce the Windows runner itself, but broad-suite runs reproduced the same cross-test browser-death mechanism explicitly as Chrome SEGV_MAPERR 0x1b0, while five separate focused runs, 100 same-process build repetitions, and 20 dev repetitions all passed. Crucially, the identical process-level failure reproduced on the exact base SHA and was attributed to varying unrelated tests, so PR #17081 is not responsible. Historical SvelteKit PR #15209 records the exact Chrome/Playwright crash signature. No safe standalone repository fix was found: changing this test cannot prevent a worker browser crash, lowering parallelism is not robust because one one-worker run also crashed, and workflow changes are prohibited. No branch was pushed.

elliott-with-the-longest-name-on-github
elliott-with-the-longest-name-on-github approved these changes Sep 9, 2026
View reviewed changes

@elliott-with-the-longest-name-on-github elliott-with-the-longest-name-on-github left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, thanks

@elliott-with-the-longest-name-on-github
elliott-with-the-longest-name-on-github merged commit 7bf4fee into version-3 Sep 9, 2026
73 of 74 checks passed
@elliott-with-the-longest-name-on-github
elliott-with-the-longest-name-on-github deleted the externalise-opentelemetry branch September 9, 2026 17:44
@Lms24 Lms24 mentioned this pull request Sep 25, 2026
Merged
Lms24 added a commit to getsentry/sentry-javascript that referenced this pull request Sep 25, 2026
@Lms24 @claude
fix(sveltekit): Resolve @opentelemetry/api via the SDK on SvelteKit…
2a58c97 
… 3 (#24736)

Since `@sveltejs/kit@next.28`, PR
sveltejs/kit#17081, `@opentelemetry/api` is
externalized from the sveltekit server build. Thus the dynamic import of
`@opentelemetry/api` remains untouched during build, meaning the API
package is now a required dependency module during runtime. This is
technically correct and fine for anyone using a default OTel setup,
where users would install `@opentelemetry/api` anyway.

However, for Sentry users, `@opentelemetry/api` is a transitive
dependency of `@sentry/node`, so not a direct app dependency. When users
use `pnpm` or Yarn `pnp`, dynamic imports can only resolve direct
dependencies and not transitive deps. This broke our e2e tests.

To avoid having to make the Sentry SDK more complicated by instructing
users to manually install `@opentelemetry/api`, this PR adds a bit of a
"redirection workaround" to our package, to get the dynamic import
working again without user intervention:

This PR:
- Adds `@opentelemetry/api` as a dep of `@sentry/sveltekit`
- Exports `@opentelemetry/api` from Sveltekit SDK package as subpath
export
- Adds a vite plugin to redirect id resolving of `@opentelemetry/api`
And point Vite to our subpath export instead.

Kit 3 E2E tests pass again with this fix.

Caveat: This redirection always applies, so if users actively installed
both `@sentry/sveltekit` and `@opentelemetry/api`, our Vite plugin now
always redirects the import to our version of the OTel API package. If
this causes problems, we can revisit the fix and offer an opt out
possibility of the redirection for example.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@s1gr1d s1gr1d mentioned this pull request Oct 6, 2026
Merged
s1gr1d added a commit to getsentry/sentry-javascript that referenced this pull request Oct 6, 2026
@s1gr1d @Lms24 @claude
fix(v10/sveltekit): Resolve @opentelemetry/api via the SDK on Svelt…
1e06821 
…eKit 3 (#25105)

Backport of #24736

Since `@sveltejs/kit@3.0.0-next.28` (sveltejs/kit#17081), the SvelteKit
server build keeps `@opentelemetry/api` external. With `tracing.server`
enabled, SvelteKit imports it at runtime from the app. Under pnpm or
Yarn PnP, the app cannot resolve it, because it is only a transitive
dependency of `@sentry/node`. Then SvelteKit fails every request with
`tracing_api_missing`. This is also why the `sveltekit-3` e2e app fails
on v10 against stable SvelteKit 3.

`@sentry/sveltekit` now depends on `@opentelemetry/api`, re-exports it
as `@sentry/sveltekit/opentelemetry-api`, and the `sentrySvelteKit()`
Vite plugin redirects SvelteKit's import to that re-export.

## Differences to the original PR

- `packages/sveltekit/package.json`: v10 has no `./vite` subpath export,
so only the `./opentelemetry-api` export is added. The dependency list
keeps the v10 packages and only adds `@opentelemetry/api`.
- `packages/sveltekit/rollup.npm.config.mjs`: only adds the
`src/opentelemetryApi.ts` entry point, not `src/vite/index.ts`.
- `packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts`: v10 has
no orchestrion Vite plugin, so the expected plugin counts are one lower
than on `develop`.

Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@elliott-with-the-longest-name-on-github elliott-with-the-longest-name-on-github elliott-with-the-longest-name-on-github approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@teemingc @elliott-with-the-longest-name-on-github