fix: carry the tracing span in the request store instead of copying the event

[Nic-Polumeyv]

Fixes #15040. Every server-side copy of the request event existed only to vary tracing.current per span. The span now travels in the request store instead, with current a getter reading it, and the copies are gone.

The data-request copy is gone too; the pathname is now normalized in place instead.

Part of #16519.

---

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]

Install the latest version of @sveltejs/kit from 1ad7c0a:

pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/1ad7c0a16f49b7dd1aaf8893fb11ec26dbe63489

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

Note

This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed.

[changeset-bot]

🦋 Changeset detected

Latest commit: 1ad7c0a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

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

[elliott-with-the-longest-name-on-github]

This would be a breaking change. event.current is supposed to be the enclosing SvelteKit span, not whichever span happens to be active whenever it's accessed -- you should be able to call getReqeustEvent from some deeply-nested place and get access to the currently-active SvelteKit span.

[elliott-with-the-longest-name-on-github]

Is it actually safe to remove this? It seems like probably not?

[Nic-Polumeyv]

Yeah, fair. I'll switch back. Good thing it's just an easy commit revert. The single event identity and the copy deletions don't depend on the getter, so the rest of the PR is unchanged.

[Nic-Polumeyv]

It's still normalized, it just happens in respond.js now, so data requests get the same URL that page requests already get before handle runs.

[Rich-Harris]

This would be a breaking change

Breaking in a way that makes sense, or breaking in a way that we probably don't want to do?

[elliott-with-the-longest-name-on-github]

Breaking in a way we probably do not want to do. getActiveSpan is already available to users if they want to get the current active span, but if event.tracing.current resolves to anything other than the current SvelteKit span, that span is totally lost / unretrievable by the user. Which means if you have a span for like, get_user, and you wanted to annotate your SvelteKit load span or something, you're just out of luck and can't do that.

[Nic-Polumeyv]

Already reverted in 1ad7c0a. tracing.current is the enclosing SvelteKit span again, carried in the request store. There's a test now pinning that attributes set via current land on the load span.

[elliott-with-the-longest-name-on-github]

This adds a lot of complexity to fix something that, at the end of the day, isn't actually a bug. You're not supposed to be able to write to top-level properties of event. Those are owned by Kit. I think there might technically be an issue with event.fetch closing over event, but as far as I can tell we don't actually mutate any of the properties it's depending on from inside the Kit pipeline, so there's no actual bug, just a theoretical one. And I don't really think this amount of additional complexity is warranted to fix a theoretical issue -- especially when event.fetch is going to go the way of the dodo as remote functions become more prevalent.