Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

meta(changelog): Add changelog for v8.0.0-beta.0 #11606

Merged
merged 65 commits into from
Apr 15, 2024

Conversation

mydea
Copy link
Member

@mydea mydea commented Apr 15, 2024

FIRST BETA INCOMING!

lforst and others added 30 commits April 8, 2024 12:00
It may happen that we have different modules, different versions etc. so
the weakmaps may not be in sync. Just storing a non-enumerable property
should be the safer choice here.

Partially required for next.js 13.4.19 to work....
This can be used to lazy load a pluggable integration.

Usage:

```js
async function getOrLazyLoadFeedback() {
  const existing = Sentry.getFeedback(); // check if it has already been installed
  if (existing) {
    return existing;
  }
  try {
    const feedbackIntegration = await Sentry.lazyLoadIntegration('feedbackIntegration');
    client.addIntegration(feedbackIntegration());
  } catch(error) {
    // this can error, we need to handle this!
  }
}
```

Closes #10905
…mentations (#11434)

Concrete changes:
1. `addRequestDataToEvent` no longer applies its transaction name to
_non-transaction_ events
2. Http instrumentation sets the isolation scope's `transactionName` in
the Otel `requestHook` to the unparameterized request URL path (w/o
query params or fragments).
3. Express instrumentation sets the isolation scope's `transactionName`
in the Otel `spanName` hook to a parameterized route, once a request
handler span (e.g. `app.get(...)`) is created.
4. As a consequence of the above, non-transaction events now get
assigned `event.transaction` correctly from the scopes.
To ensure we can use this in an isomorphic way.

This means we do not rely on this being a mutable object anymore, but we
need to ensure to update it (in http integration) on both the span _and_
for the current context.
ref #9832

Building off #11381,
this moves browser related utils into the browser utils package.

Next step is to move `browserTracingIntegration` into browser!
Next week I'm going to play around with optimizing feedback/replay
packages as per what we saw in
#11452.

In prep for that work I re-organized size limit and made the limits a
little more sensible.

The size bot comment will be a little messed up because we renamed the
fields, but after we merge should be a lot better.
All of the ANR tests were verified over stdout. This PR ensures some of
the basic tests actually send events over http to ensure the transport
is fully tested.
Also adds tests for this for core, vercel-edge and opentelemetry.

NOTE: In core, this does not do anything, as we cannot actually update
the isolation scope in browser.

Co-authored-by: Luca Forstner <[email protected]>
[Gitflow] Merge master into develop
This updates handling of next.js instrumentation to re-use an isolation
scope from a root span.

This should ensure we have consistent isolation scopes, no matter if
next.js auto creates spans or not.
I want to remove the karma/mocha based tests in the browser package. To
accomplish this, I'll be porting 1 test suite a day from the old
integration tests to playwright. Today is Day 3:
`packages/browser/test/integration/suites/config.js`

I was surprised we never had `ignoreErrors` or `denyUrls` tests in
playwright, so it's good to get the confidence that everything works
here.

ref #11084
day 2: #11436
Font sizes were set instead of using config option
…11149)

Fixes: #11105

Skips span creation for `OPTIONS` and `HEAD` requests in
`wrapRequestHandler`, which apparently are the unwanted requests
mentioned in #11105. We can also implement a more precise filtering but
this seems to resolve it on my local test application.
…the exported surface area (#11355)

You can read this commit-by-commit to watch the refactor unfold

The situation is: after
#11342 is merged, all
the code inside each of the
`packages/feedback/src/[core|modal|screenshot]` will be split up into
separate integrations. These integrations can't share the same
`src/types/*.ts` definitions anymore. Therefore type definitions will
need to live in @sentry/types instead.

This PR moves all the types, and since they'll be public now in that
package, i refactored things to reduce the public surface area and
improve names where possible.
The only non-type changes in here are where we move `createDialog.ts`
and `createInput.ts` into their respective `integration.ts` files, no
code changes at all.

Related to #11435
These are deps we may want to keep more up to date than we usually do,
so generating update PRs for them may be helpful.
When interacting with the Breadcrumd API, I had some trouble finding the
correct documentation. I retrieved my changes for the previous
[PR](#11119) to make
sure the JSDoc is updated with the actual doc.
…ng spans (#11503)

Our Http instrumentation didn't clone the isolation scope
correctly when the request span is not recording.
We early returned if the span was not a server kind
span. However, non-recording spans apparently have no span kind, so we
wrongfully early returned in the check for non-recording spans.
Next.js provides their own OTel http integration, which conflicts with
ours
ref #11016
added commit from this PR:
#11319

---------

Co-authored-by: Luca Forstner <[email protected]>
Co-authored-by: Francesco Novy <[email protected]>
Fixes the Canary tests by ensuring we cache node modules and files in
dev-packages as well.

In the debugging process, I also moved a few things around - e.g. the
playwright dependency stuff now happens in CI, not in the build commands
everywhere.
Instead of exporting `_INTERNAL` here, it's OK IMHO to export this. The
API is stable enough.
…de request (#11511)

Update the scope's `transactionName` in the `sentryHandle`
instrumentation that's always invoked when a request is made to the
SvelteKit server.
…11413)

Modifies `makeOfflineTransport` to allow replay envelopes to be
queued when offline and to ensure they are always send in order.

To do this we:
- Modify the store interface so it has `push`, `unshift` and `shift`
methods (BREAKING)
- Always queue replay envelopes in the store so there is never more than
one in-flight
- Always `unshift` failed envelopes so they're always sent in the same
order from the queue
)

This PR updates the scope's `transactionName` in our Hapi error handler.
Conveniently, we can access the parameterized route within our Hapi
plugin, meaning we're not constrained by reading route info from a
potential non-recording span.
…11510)

This PR adds scope transactionName updating to our NestJS
instrumentation. Similarly to Hapi and Fastify, we can use a
framework-native component, an Interceptor, to assign the parameterized
route.

ref #10846
…11447)

This PR updates the isolation scope's `transactionName` by adding another
hook callback in our `setupFastifyErrorHandler` fastify plugin. This is
better than accessing the otel span for two reasons:

1. We can always update the transactionName, not just when a span is
sampled and recording
2. The `onRequest` hook is executed earlier in the lifecycle than the
`preHandler` hook that Otel uses to trigger the `requestHook` callback.
AbhiPrasad and others added 9 commits April 12, 2024 09:42
As per
#4784 (comment)

make sure fastify types use any is that we don't collide with what
fastify types expect.

Also convert e2e tests to use typescript to validate build.
Instead, users can use `registerSpanErrorInstrumentation` which is now
does much less things than before.

I opted to not fully remove this yet because I guess the chance that
somebody may have been using this is somewhat high, and we can easily
continue to have it with no bundle size hit for those who don't need it.
We also couldn't really deprecate this in v7 so I think it's OK to leave
this in for now.
…nitely (#11578)

When experimenting for #11525 I found that `Runtime.evaluate` only
returns when the event loop becomes unblocked. This means that we are
not sending ANR events if the event loop is blocked indefinitely.

This PR adds a timeout that sends the ANR event if we have not been able
to evaluate the scope within 5 seconds.
This also forward-ports the tests from
#11555, and adds some
more tests.

Node is a different story, will tackle it separately.
Adds a test to ensure isolation scope is correctly applied to express
spans, and also that console.log is correctly instrumented.

We have related tests in E2E tests and in other places, but it doesn't
hurt to have this explicitly here I guess...
Removes the `getCurrentHub` property from the
`AsyncContextStrategy` interface and consequently also the property in
the interface implementers.
This fixes the node SDK to make tracing without performance work as
expected.

This has three aspects:

1. Ensure we always have an url to check against
`tracePropagationTargets`, even if the span is not sampled.
2. Ensure we use the correct propagation context always
3. Ensure we actually consistently handle ougoing http.client spans
without an active transaction

The fix for 2. was to have a fallback behavior if we don't have a scope
linked to a context yet (we'll just use the current scope then). This is
needed because we won't necessarily have an assigned scope yet at the
time this is called, because at this point in time this is still a
floating context. This also meant removing the fallback code to look at
the remote span context (because we'll always have a scope now).

The fix for 3. was to move this out of the http integration and do this
in the sampler instead.

The fix for 1. was a bit more elaborate, because we require different
things to solve the http instrumentation and the fetch instrumentation.

For fetch, we leverage our sampler, which can attach trace state that
even unsampled spans will have. We attach the URL as trace state which
we can read in the propagator, ensuring this works even for unsampled
spans.

Sadly, for http this is not sufficient, because our usage of
`onlyIfParentForOutgoingRequest` lead to some spans being created
outside of the sampler. By removing this option (see 3) and doing stuff
in the sampler instead, we can ensure that all spans for http are
handled properly.

This also bumps our node-fetch otel instrumentation to
[1.2.0](https://github.com/gas-buddy/opentelemetry-instrumentation-fetch-node/releases/tag/v1.2.0)
which fixes headers for Node 21 (tests were actually failing without
this!).
This is not supported since next 12, so we are good to remove this too:
https://nextjs.org/docs/messages/webpack5
This is duplicated from the trace data, no need to send this anymore.
@mydea mydea self-assigned this Apr 15, 2024
CHANGELOG.md Outdated Show resolved Hide resolved
@mydea mydea force-pushed the prepare-release/8.0.0-beta.0 branch from 020f4d7 to 896d798 Compare April 15, 2024 12:38
@mydea mydea marked this pull request as ready for review April 15, 2024 12:41
mydea and others added 2 commits April 15, 2024 14:47
We don't actually need them anymore, as we've updated stuff I guess.
This also finally fixes the problem that yarn always seemed to do some
work even with the lockfile "up to date".
…n finished (#11600)

Remove the now incorrect propagationContext resetting logic for
pageload spans and add tests to cover this scenario.
This is the first beta release of Sentry JavaScript SDK v8. With this release, there are no more planned breaking
changes for the v8 cycle.

Read the [in-depth migration guide](./MIGRATION.md) to find out how to address any breaking changes in your code. All
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

will add the stuff you wrote there here too! 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

we added this now 👍

@Lms24 Lms24 force-pushed the prepare-release/8.0.0-beta.0 branch 3 times, most recently from ed8d06c to 409ca77 Compare April 15, 2024 13:44
@mydea mydea force-pushed the prepare-release/8.0.0-beta.0 branch from 409ca77 to a1b65e8 Compare April 15, 2024 13:48
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Let's go 🚀

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

🔥

@mydea mydea changed the base branch from develop to master April 15, 2024 14:05
@mydea mydea merged commit 424914c into master Apr 15, 2024
29 checks passed
@mydea mydea deleted the prepare-release/8.0.0-beta.0 branch April 15, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants