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

@sentry/* 7.68.0 breaks @sentry/profiling-node latest #8965

Closed
3 tasks done
tomgrossman opened this issue Sep 7, 2023 · 11 comments
Closed
3 tasks done

@sentry/* 7.68.0 breaks @sentry/profiling-node latest #8965

tomgrossman opened this issue Sep 7, 2023 · 11 comments
Labels
Package: node Issues related to the Sentry Node SDK Type: Bug

Comments

@tomgrossman
Copy link

tomgrossman commented Sep 7, 2023

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

@sentry/* - 7.68.0, @sentry/profiling-node - 1.2.1
Works fine with versions 7.67.0

Framework Version

Node 18.12.1 Nest.js 10.1.17

Link to Sentry event

No response

SDK Setup

import * as Sentry from '@sentry/node';
import * as Tracing from '@sentry/tracing';
import { RewriteFrames } from '@sentry/integrations';
import { ProfilingIntegration } from '@sentry/profiling-node';

Sentry.init({
    dsn: '...',
    environment: '...',
    integrations: [
      new RewriteFrames({
        root: '/app',
      }),
      new Sentry.Integrations.Http({ tracing: true }),
      new Tracing.Integrations.Express({
        app: httpAdapter.getInstance(),
      }),
      new ProfilingIntegration(),
    ],
    tracesSampler: (samplingContext) => {
      if (samplingContext.request?.url?.includes('/health')) {
        return 0;
      }

      return 0.2;
    },
    profilesSampleRate: 1.0,
  });

Steps to Reproduce

  1. install latest @sentry versions
  2. integrate @sentry/profiling-node
  3. build

Expected Result

build successfully :)

Actual Result

Error thrown during nest build:

Type 'ProfilingIntegration' is not assignable to type 'Integration'.
  Types of property 'setupOnce' are incompatible.

Works fine with versions 7.67.0

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Sep 7, 2023
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Sep 7, 2023
@Lms24
Copy link
Member

Lms24 commented Sep 7, 2023

Hey @tomgrossman which TypeScript version are you using? Just wondering if this is related to #8954

@getsantry getsantry bot removed the status in GitHub Issues with 👀 Sep 7, 2023
@getsantry getsantry bot moved this to Waiting for: Community in GitHub Issues with 👀 Sep 7, 2023
@tomgrossman
Copy link
Author

Hey @tomgrossman which TypeScript version are you using? Just wondering if this is related to #8954

@Lms24
"typescript": "^5.2.2"

@getsantry getsantry bot moved this from Waiting for: Community to Waiting for: Product Owner in GitHub Issues with 👀 Sep 7, 2023
@Lms24
Copy link
Member

Lms24 commented Sep 7, 2023

I tried to reproduce this with a barebones Node + Typescript app and couldn't get the type error. It's a little weird because we didn't change anything in the Integration interface lately, especially not between 7.67.0 and 7.68.0. Here's my repro: https://github.com/Lms24/gh-sentry-javascript-8965-node-integration-type-error please take a look and let me know what to change to reproduce this. Alternatively, feel free to provide your own repro app that we can use to look into this further.

Before you get started though, it might be worthwhile to check for version mismatches:

  • Check if your lock file contains any @sentry/ packages (other than profiling-node) that are not aligned with version 7.68.0. If so, please upgrade them.
  • Similarly, have you tried deleting node_modules and reinstalling your deps? Maybe something is off...
  • To avoid version mismatches you can also get rid of the @sentry/tracing package which is not needed anymore. Everything from tracing is now exported from @sentry/node. See https://docs.sentry.io/platforms/node/guides/express/#monitor-performance

@getsantry getsantry bot removed the status in GitHub Issues with 👀 Sep 7, 2023
@getsantry getsantry bot moved this to Waiting for: Community in GitHub Issues with 👀 Sep 7, 2023
@getsantry getsantry bot moved this from Waiting for: Community to Waiting for: Product Owner in GitHub Issues with 👀 Sep 7, 2023
@tomgrossman
Copy link
Author

tomgrossman commented Sep 7, 2023

@Lms24
OK I think I understood what happened.
Before the updates my package.json included the versions:

    "@sentry/integrations": "^7.67.0",
    "@sentry/node": "^7.67.0",
    "@sentry/profiling-node": "^1.2.1",
    "@sentry/tracing": "^7.67.0",

and the package-lock.json included the @sentry/hub as:

    "node_modules/@sentry/hub": {
      "version": "7.66.0",
      "resolved": "https://registry.npmjs.org/@sentry/hub/-/hub-7.66.0.tgz",
      "integrity": "sha512-T+xtxbZm+ZjZxzCKubI4GzYEju2PBK6jYLOk+E/5xVUX029H4qkDLwqk+NlEH7KzgaJIZBRos5CC0+wWCem6PQ==",
      "dependencies": {
        "@sentry/core": "7.66.0",
        "@sentry/types": "7.66.0",
        "@sentry/utils": "7.66.0",
        "tslib": "^2.4.1 || ^1.9.3"
      },
      "engines": {
        "node": ">=8"
      }
    }

it comes from the dependencies of profiling-node and was installed probably when the other versions were 7.66.0 thus installing this version also on the /hub

"node_modules/@sentry/profiling-node": {
      "version": "1.2.1",
      "resolved": "https://registry.npmjs.org/@sentry/profiling-node/-/profiling-node-1.2.1.tgz",
      "integrity": "sha512-+bu3qRG5Yd7a34YhkHfnfkrwPmsAPdp5cUhIx5BKit4hQf7zUaIw9UOUI/+VvK9rn8SUhC0oLNQf6HQJ0CIFUQ==",
      "hasInstallScript": true,
      "dependencies": {
        "@sentry/core": "^7.64.0",
        "@sentry/hub": "^7.64.0",
        "@sentry/node": "^7.64.0",
        "@sentry/types": "^7.64.0",
        "@sentry/utils": "^7.64.0",
        "detect-libc": "^2.0.1",
        "node-abi": "^3.28.0",
        "node-gyp": "^9.3.0"
      },
      "bin": {
        "sentry-prune-profiler-binaries": "scripts/prune-profiler-binaries.mjs"
      },
      "engines": {
        "node": ">=8.0.0"
      }
    }

Which of course installs the core, types, etc... with explicit 7.66.0 versions in additions to the 7.67.0 versions.
Apparently there are no conflicts between 7.66.0 and 7.67. because everything worked fine.

But, when updating the package.json versions to 7.68.0, all the 67 versions are replaced with 68, but the hub, core,... versions stays as 66, which now creates conflicts with 68.

In order to reproduce it in your repository:

  1. in the tsconfig.json change the skipLibCheck to true
  2. remove the node_modules and package-lock.json files
  3. Set the package.json depenencies to explicit 7.66.0 but add also the /hub package so we can simulate how it was before the upgrades to 67 and 68. npm i and npm run build
  "dependencies": {
    "@sentry/hub": "7.66.0",
    "@sentry/integrations": "7.66.0",
    "@sentry/node": "7.66.0",
    "@sentry/profiling-node": "^1.2.1"
  }
  1. The current situation is how I was 2 days ago
  2. Yesterday there was upgrades to 7.67, so set the version to 7.67, you can remove the /hub package and npm i; npm run build
  "dependencies": {
    "@sentry/integrations": "7.67.0",
    "@sentry/node": "7.67.0",
    "@sentry/profiling-node": "^1.2.1"
  }
  1. Set the versions to ^, npm i; npm run build. and this is the exact status of packages and package-lock from yesterday
  "dependencies": {
    "@sentry/integrations": "^7.67.0",
    "@sentry/node": "^7.67.0",
    "@sentry/profiling-node": "^1.2.1"
  }
  1. Now, upgrade to ^7.68.0, npm i; npm run build and you should see the error
  "dependencies": {
    "@sentry/integrations": "^7.68.0",
    "@sentry/node": "^7.68.0",
    "@sentry/profiling-node": "^1.2.1"
  }

@mydea
Copy link
Member

mydea commented Sep 7, 2023

I think you just need to ensure to deduplicate your dependencies. E.g. remove all @sentry/xxx entries from your lockfile and run yarn install or npm install or whatever, this should ensure you get consistent versions there.

(For reference: getsentry/profiling-node#195)

@tomgrossman
Copy link
Author

I think you just need to ensure to deduplicate your dependencies. E.g. remove all @sentry/xxx entries from your lockfile and run yarn install or npm install or whatever, this should ensure you get consistent versions there.

(For reference: getsentry/profiling-node#195)

@mydea yes of course this solves the issue, but it's not viable for Dependabot/Renovate/Etc...
I think profiling-node should depend on minor changes of the other sentry packages (e.g ^7.64.0) and not explicit versions (7.64.0) as today.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Sep 7, 2023
@mydea
Copy link
Member

mydea commented Sep 7, 2023

I think you just need to ensure to deduplicate your dependencies. E.g. remove all @sentry/xxx entries from your lockfile and run yarn install or npm install or whatever, this should ensure you get consistent versions there.
(For reference: getsentry/profiling-node#195)

@mydea yes of course this solves the issue, but it's not viable for Dependabot/Renovate/Etc... I think profiling-node should depend on minor changes of the other sentry packages (e.g ^7.64.0) and not explicit versions (7.64.0) as today.

As far as I can tell it does depend on minor versions? 🤔
See: https://github.com/getsentry/profiling-node/blob/main/package.json#L81

@tomgrossman
Copy link
Author

I think you just need to ensure to deduplicate your dependencies. E.g. remove all @sentry/xxx entries from your lockfile and run yarn install or npm install or whatever, this should ensure you get consistent versions there.
(For reference: getsentry/profiling-node#195)

@mydea yes of course this solves the issue, but it's not viable for Dependabot/Renovate/Etc... I think profiling-node should depend on minor changes of the other sentry packages (e.g ^7.64.0) and not explicit versions (7.64.0) as today.

As far as I can tell it does depend on minor versions? 🤔 See: https://github.com/getsentry/profiling-node/blob/main/package.json#L81

@mydea oops, you are right. It doesn't change the issue that is still not supported by automated dependency managements (dependabot, renovate).
Because for some reason it's on a different tree than the other sentry packages 🤷

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Sep 7, 2023
@mydea
Copy link
Member

mydea commented Sep 7, 2023

Yeah this is a general problem with package managers and deduplication 😬 In the medium run the solution for this is to merge @sentry/profiling-node into the core @sentry/node package. We've been generally moving towards this exactly for this reason, as having slightly mismatching versions leads to a lot of weird problems. For now, manually deduplicating your dependencies is the way to go, even if we know this can sometimes be painful 😬

@tomgrossman
Copy link
Author

Yeah this is a general problem with package managers and deduplication 😬 In the medium run the solution for this is to merge @sentry/profiling-node into the core @sentry/node package. We've been generally moving towards this exactly for this reason, as having slightly mismatching versions leads to a lot of weird problems. For now, manually deduplicating your dependencies is the way to go, even if we know this can sometimes be painful 😬

@mydea yea, I get it. Unfortunately manually deleting and regenerating the package-lock is too painful for CI/CD platforms with Dependabot/Renovate. And with the Sentry's version release rate, it will basically happen every day, so this is terrible.

I guess we will have to re-think about auto updating Sentry packages until this solved :(

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Sep 7, 2023
@mydea
Copy link
Member

mydea commented Sep 7, 2023

Yeah this is a general problem with package managers and deduplication 😬 In the medium run the solution for this is to merge @sentry/profiling-node into the core @sentry/node package. We've been generally moving towards this exactly for this reason, as having slightly mismatching versions leads to a lot of weird problems. For now, manually deduplicating your dependencies is the way to go, even if we know this can sometimes be painful 😬

@mydea yea, I get it. Unfortunately manually deleting and regenerating the package-lock is too painful for CI/CD platforms with Dependabot/Renovate. And with the Sentry's version release rate, it will basically happen every day, so this is terrible.

I guess we will have to re-think about auto updating Sentry packages until this solved :(

yes, I fear for now the best approach is to manually update sentry packages (also to ensure all are always in sync) - not ideal, but we're working on making this less painful in the future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: node Issues related to the Sentry Node SDK Type: Bug
Projects
Archived in project
Archived in project
Development

No branches or pull requests

3 participants