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): Update changelog for 7.94.0 #10252

Merged
merged 103 commits into from
Jan 19, 2024
Merged

Conversation

mydea
Copy link
Member

@mydea mydea commented Jan 18, 2024

Mainly includes a lot of deprecations, and the replay canvas integration.

lforst and others added 30 commits January 10, 2024 11:31
[Gitflow] Merge master into develop
…10135)

For the v7 cycle, in order to make this easier to handle. in v8, we need
to remove the unused `setupOnce()` calls again.

It would be nicer if we could do away with this, but it is pretty hard
as the options for a client currently are types as `Integration[]`, and
we use these both for setting & getting, making it hard to change this
without breaking anything. Now, this is much easier (it basically just
works), with the small downsides of shipping a few more bytes - but we
can simply remove this in v8 then.
Except for `bindClient()` which needs a separate replacement. For
everything else we already have a replacement in place!

Then in a follow up we can deprecate `getCurrentHub()` itself.
Deprecate the `instrumenter` field on the `Span` interface and
class. The field is not part of the Otel Span API and hence it can't
exist publicly on our spans either. We can replace field with an `instanceof` check
when setting `instrumenter` on the event in v8. This is breaking though,
so for now, we only deprecate it. Users shouldn't need to worry about
this field anyway.
Adds a new Node integration test runner that runs the scenario in its
own Node process and reads the envelope back from stdout.

I have converted all the `Anr` and `LocalVariables` tests to use this
new format.

## How to use

First, in your test scenario, you need to set the transport to the
`loggingTransport` from `@sentry-internal/node-integration-tests`. This
will cause envelopes to be logged to stdout as a single line of JSON:
```ts
import * as Sentry from '@sentry/node';
import { loggingTransport } from '@sentry-internal/node-integration-tests';

Sentry.init({
  dsn: 'https://public@dsn.ingest.sentry.io/1337',
  transport: loggingTransport,
});
```

Then in your test, use the `createRunner` function to create your test.
The `createRunner` args are the path segments to the scenario. If you
pass a typescript entry file, `-r ts-node/register` gets added to the
node args.

Every `expect` call adds an expected envelope item. Currently you can
expect `event`, `transaction` and `session` but it's easy to add extra
type-safe expectations. You must add an `expect` for every envelope
**item** that will be sent and they must be in the correct order or an
error is thrown and the test fails. The expect
`event`/`transaction`/`session` can be a callback or an object that will
be tested against the received envelope.

If you don't care about specific types of envelope item, you can ignore
them:
```ts
createRunner(__dirname, 'basic.js')
  .ignore('session', 'sessions')
  .expect(//...
```

If you pass `done` to `start`, `done` will be called when all the
envelope items have matched the expected and `done` will be passed any
errors that occur which ends the test without waiting for the timeout.

## Example

Here is an Anr test that ensures we get the expected session envelope
followed by the expected event envelope:
```ts
import { assertSentryEvent, assertSentrySession, createRunner } from '../../utils/runner';

const EXPECTED_ANR_EVENT = {
  // Ensure we have context
  contexts: {
    trace: {
      span_id: expect.any(String),
      trace_id: expect.any(String),
    },
    device: {
      arch: expect.any(String),
    },
    app: {
      app_start_time: expect.any(String),
    },
    os: {
      name: expect.any(String),
    },
    culture: {
      timezone: expect.any(String),
    },
  },
  // and an exception that is our ANR
  exception: {
    values: [
      {
        type: 'ApplicationNotResponding',
        value: 'Application Not Responding for at least 200 ms',
        mechanism: { type: 'ANR' },
        stacktrace: {
          frames: expect.arrayContaining([
            {
              colno: expect.any(Number),
              lineno: expect.any(Number),
              filename: expect.any(String),
              function: '?',
              in_app: true,
            },
            {
              colno: expect.any(Number),
              lineno: expect.any(Number),
              filename: expect.any(String),
              function: 'longWork',
              in_app: true,
            },
          ]),
        },
      },
    ],
  },
};

test('Anr with session', done => {
  createRunner(__dirname, 'basic-session.js')
    .expect({ session: { status: 'abnormal', abnormal_mechanism: 'anr_foreground' } })
    .expect({ event:  EXPECTED_ANR_EVENT })
    .start(done);
});
```

It's also possible to pass flags to node via the `withFlags()` function:
```ts
  test('With --inspect',done => {
    createRunner(__dirname, 'basic.mjs')
      .withFlags('--inspect')
      .expect({ event: EXPECTED_ANR_EVENT })
      .start(done);
  });
```
…10134)

Deprecate the `transaction` field on the `Span` interface and
class. Instead, we'll store the span hierarchy in a different format
external to the span class. In node-experimental, we use a WeakMap
referencing the parent span as a data structure which we might need to
generally do in v8. This, however, is a breaking change, so for now we
deprecate the field but use it in the replacement utility function for
v7.
…)` (#10118)

This adds a new `client.init()` method to be called instead of
`client.setupIntegrations()`.

Note that this method simply initializes the integrations always, and
depends on the check that we don't add integrations multiple times.

This also has a bit of a different semantic, dropping the `force`
argument in favor of just calling `addIntegration()` again - this
depends on #10116 to
really work.
`yarn fix` caught this error, but somehow this made into the repository.
…#10161)

Looks like we forgot to deprecate the method on the interface (it's
already deprecated on the class).

---------

Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
…n` and `Span` (#10164)

Seems like we didn't fully deprecate the `setName` API in #10018 as some
deprecation annotations were still missing on
* `Span` class
* `Transaction` class
* `Transaction` interface
… headers (#10176)

When a `fetch` call is made with a `Request` object as the
input/resource param (i.e. the first) and additionally provided headers
in the fetch init options (the second param), browsers ignore the headers 
from the request object but only add the headers from the init options 
when making the request.

Our `fetch` instrumentation had it the other way around, preferring
headers from the request object over the headers in the options, as
reported in #10172 . This patch fixes this behaviour by essentially turning
around the logic we previously had.
When creating a session, we previously only read the user from the
isolation scope. This was problematic because the user could be set in
`initialScope` which will be applied to the _current_ scope on SDK init.
This would happen before the session is started when using auto session
tracking.

This PR now takes the user from the current scope if it's set and falls
back to the isolation scope should the user be set there. Added an
integration test to cover this regression.
And also deprecate both `getIntegration()` as well as
`getIntegrationById()` which is only on the baseclient, but not on the
client type, anyhow :grimace:.

Usage:

```ts
const replay = getClient().getIntegrationByName('Replay');
```

Or, if you want to have an easier time with types:

```ts
const replay = getClient().getIntegrationByName<Replay>('Replay');
```

## Why do we need this

In v8, integrations will not be classes anymore, so you cannot pass a
class definition anymore to `getIntegration()`. We also decided to
remove the static `id` field, so you cannot find an integration via that
way anymore.

`getIntegrationById()` was always just on the baseclient, so not
properly types anyhow. It is also an inconsistent/weird naming because
we will not have an `id` anymore for integrations at all.
…g`, `setExtra`, and `setContext` to isolation scope (#10163)

Co-authored-by: Francesco Novy <francesco.novy@sentry.io>
This was not really documented anywhere anymore, and it's not clear
what/when/why you would be using this, so we will remove this in v8.
This is only used there, and somehow suddenly seemed to mess with my
`Jest` globals... Also removing `@types/mocha` which seems to have no
effect (?) on the build, but somehow polluted TS...
@AbhiPrasad
Copy link
Member

Let's make sure we include #10233 and #10231

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.

What a 🚢 !

AbhiPrasad and others added 17 commits January 18, 2024 18:00
We forgot to re-export this when we re-exported `Feedback` from the
browser SDK. This makes it easier to use than exposing `sendFeedback`
via the Feedback integration instance itself.
…10227)

As was raised in
#10219, we set some
extra operation based fields on span data for mongodb.

This is unfortunately just serialized output, which means it can contain
PII (it needs to be paramaterized for PII to be removed).

Given this data will change a lot when we switch to OTEL, gate setting
this specific span data behind `sendDefaultPii`.
…experimental` (#10255)

This PR adds auto instrumentation tests for `mysql` for
`@sentry/node-experimental`.

There are no ESM tests because `mysql` does not support ESM!
…es (#10245)

The old implementation of `dropUndefinedKeys()` could break things,
because it used `isPlainObject()` which actually does not check if
something is a POJO, but just any object. So any class instance found
somewhere would be broken by this.

This PR fixes this to check for POJOs better, and leaves class instances
alone.

Supersedes
https://github.com/getsentry/sentry-javascript/pull/10242/files
Closes #9199

This PR vendors the `https-proxy-agent` code and in the process updates
to v7.0.0.

`https-proxy-agent` is our last remaining cjs-only dependency so this is
required for #10046.

This removes the following dependencies:
- `https-proxy-agent@5.0.1`
- `agent-base@6.0.2`
- `debug@4.3.4`
- `ms@2.1.2`

The vendored code has been modified to use the Sentry logger rather than
`debug`.

Initially, rather than modify the vendored code substantially just to
pass our tight lint rules, I've disabled a few of the less important
lints that would make it particularly tricky to pull in upstream bug
fixes:
```ts
/* eslint-disable @typescript-eslint/explicit-member-accessibility */
/* eslint-disable @typescript-eslint/member-ordering */
/* eslint-disable jsdoc/require-jsdoc */
``` 

## Min supported Node version

`https-proxy-agent` has a `@types/node@14.18.45` dev dependency but
apart from adding an import for `URL`, I can't find anything that would
stop it working on older versions of node and there is nothing in the
changelogs that would suggest the min supported version has changed.

---------

Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>
…-to-span-data

feat(core): Add domain information to resource span data
@AbhiPrasad AbhiPrasad changed the base branch from develop to master January 18, 2024 22:43
@onurtemizkan onurtemizkan mentioned this pull request Jan 19, 2024
3 tasks
@mydea mydea merged commit 0a65896 into master Jan 19, 2024
90 checks passed
@mydea mydea deleted the prepare-release/7.94.0 branch January 19, 2024 08:33
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.

None yet

10 participants