getsentry / profiling-node

The code for this repo now lives in https://github.com/getsentry/sentry-javascript/tree/develop/packages/profiling-node
MIT License
29 stars 10 forks source link

`profilesSampler`is not being called #115

Closed Avocher closed 1 year ago

Avocher commented 1 year ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

SDK Version

0.2.2

Link to Sentry event

No response

What environment is your node script running in?

Alpine Linux 3.17.2 Node v18.15.0

How is your code deployed and bundled?

Deployed to k8s with a docker image. Files copied over.

Steps to Reproduce

I tried the following config:

Sentry.init({
            dist: '****',
            dsn: '****',
            integrations: [
                new RewriteFrames(),
                new ProfilingIntegration(),
            ],
            tracesSampleRate: 1.0,
            profilesSampler(samplingContext) {
                console.log(samplingContext);
                if (samplingTransactions.has(samplingContext.transactionContext.name)) {
                    return 1;
                }
                return 0;
            },
            includeLocalVariables: true,
});

And nothing was profiled. Then i tried the following:

Sentry.init({
            dist: '****',
            dsn: '****',
            integrations: [
                new RewriteFrames(),
                new ProfilingIntegration(),
            ],
            tracesSampleRate: 1.0,
            profilesSampleRate: 1.0,
            profilesSampler(samplingContext) {
                console.log(samplingContext);
                if (samplingTransactions.has(samplingContext.transactionContext.name)) {
                    return 1;
                }
                return 0;
            },
            includeLocalVariables: true,
});

And now transactions were getting profiled but profileSampler was never called.

Expected Result

If the profilesSampler function is defined I'd expect it to be used. In similar mannar as traces sampler work.

Actual Result

Sorry nothing the screenshot or log since nothing is happening.

JonasBa commented 1 year ago

Hey @Avocher, thanks for opening an issue - it is something we are aware of unfortunately. The type changes were shipped in the latest JS SDK updates and we will be making a release to support this early next week along with a couple of other updates that will make the SDK better to use.

I will update here once we have released the changes and sorry about that

Avocher commented 1 year ago

Thank you for the quick response!

hckhanh commented 1 year ago

I have the same issue:

Sentry Logger [log]: [Profiling] Profiling disabled, enable it by setting `profilesSampleRate` option to SDK init call.

Only profilesSampleRate will work.

hckhanh commented 1 year ago

I have another question, Do I have to care about the order of plugins in Sentry.init()

Sentry.init({
  dsn: SENTRY_DSN,

  enabled: true,
  environment: appEnvironment,
  release: `${require('../package.json').name}@${isAppProduction ? require('../package.json').version : appEnvironment
    }`,

  debug: true,
  integrations: [new ProfilingIntegration(), new Tracing.Integrations.Prisma({ client: prisma })],
  tracesSampler: (samplingContext) => {
    // console.log('tracesSampler', samplingContext)
    return true
  },
  profilesSampler: (samplingContext) => {
    console.log('profilesSampler', samplingContext)
    return (samplingContext.parentSampled ? 1.0 : 0.0)
  },
})
JonasBa commented 1 year ago

@Avocher @hckhanh we released a new version yesterday, if you upgrade you should see the profilesSampler being called.

@hckhanh you do not need to care about the order of integrations, we also got rid of the import order dependency

JonasBa commented 1 year ago

Closing the issue, feel free to reopen if you have any issues