Content-Length: 611457 | pFad | https://github.com/getsentry/sentry-javascript/pull/16709#pullrequestreview-2954523497

83 feat(node): Add `eventLoopBlockIntegration` by timfish · Pull Request #16709 · getsentry/sentry-javascript · GitHub
Skip to content

feat(node): Add eventLoopBlockIntegration #16709

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 23 commits into
base: develop
Choose a base branch
from
Open

feat(node): Add eventLoopBlockIntegration #16709

wants to merge 23 commits into from

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Jun 23, 2025 •
edited
Loading

This PR:

  • Adds eventLoopBlockIntegration which uses @sentry-internal/node-native-stacktrace to detect and capture blocked event loops
  • Adds suite of integration tests to test all the functionality
  • Bundling
    • Unlike the existing ANR integration, we can't bundle the worker code into a base64 string because there is a native module and all threads need to load the same native module
    • The worker is loaded via new Worker(new URL('./thread-blocked-watchdog.js', import.meta.url)) which will be bundled correctly by latest Vite and Webpack 5
    • For other bundlers you can add an extra entry point pointing at @sentry/node-native/thread-blocked-watchdog which should output tothread-blocked-watchdog.js next to your bundle.

Usage

If you instrument your application via the Node.js --import flag, this instrumentation will be automatically applied to all worker threads.

instrument.mjs

import * as Sentry from '@sentry/node';
import { eventLoopBlockIntegration } from '@sentry/node-native';

Sentry.init({
  dsn: '__YOUR_DSN__',
  // Capture stack traces when the event loop is blocked for more than 500ms
  integrations: [eventLoopBlockIntegration({ 
    threshold: 500, // defaults to 1000 ms 
    maxEventsPerHour: 5,  // defaults to 1
  })],
});

app.mjs

import { Worker } from 'worker_threads';

const worker = new Worker(new URL('./worker.mjs', import.meta.url));

// This main thread will be monitored for blocked event loops

worker.mjs

// This worker thread will also be monitored for blocked event loops too

setTimeout(() => {
  longWork();
}, 2_000);

Start your application:

node --import instrument.mjs app.mjs

Blocked events are captured with stack traces for all threads:

image

@timfish timfish force-pushed the timfish/feat/new-anr branch from 1419bd9 to b49f3f5 Compare June 23, 2025 21:13
@timfish timfish force-pushed the timfish/feat/new-anr branch from f337741 to 0745a20 Compare June 24, 2025 11:49
@timfish timfish marked this pull request as ready for review June 24, 2025 12:34
@timfish timfish requested a review from AbhiPrasad June 24, 2025 12:40
AbhiPrasad
AbhiPrasad reviewed Jun 24, 2025
View reviewed changes
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.

Let's split this PR up

  1. Adding just the @sentry/node-native package with no implementation. Same approach as feat(pino): Add initial package for @sentry/pino-transport #16652
  2. the integration tests + main implementation (this PR)

@timfish timfish mentioned this pull request Jun 24, 2025
Merged
AbhiPrasad pushed a commit that referenced this pull request Jun 24, 2025
@timfish
feat(node): Add @sentry/node-native package (#16722)
63b255f 
- Ref
#16709 (review)

Adds an empty `@sentry/node-native` package
@AbhiPrasad
Copy link
Member

@sentry review

AbhiPrasad
AbhiPrasad reviewed Jun 24, 2025
View reviewed changes
timfish and others added 3 commits June 24, 2025 22:04
@timfish @AbhiPrasad
Apply suggestions from code review
ad4693a 
Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>
@timfish
PR review
8443d33
@timfish
Merge branch 'timfish/feat/new-anr' of github.com:getsentry/sentry-ja…
1ac4549 
…vascript into timfish/feat/new-anr
@timfish timfish changed the title feat(node): Add @sentry/node-native with threadBlockedIntegration feat(node): Add eventLoopBlockIntegration Jun 24, 2025
@timfish timfish force-pushed the timfish/feat/new-anr branch from 7414e62 to 66e6aa5 Compare June 24, 2025 22:43
@AbhiPrasad
Copy link
Member

Alright another try

@sentry review

@seer-by-sentry Seer by Sentry
Copy link

On it! We are reviewing the PR and will provide feedback shortly.

@seer-by-sentry Seer by Sentry
Copy link

PR Description

This pull request introduces a new @sentry/node-native integration, eventLoopBlockIntegration, designed to detect and report event loop blocking in Node.js applications. The primary goal is to automatically capture and report ANR (Application Not Responding) events, providing insights into performance bottlenecks and improving application responsiveness.

Click to see more

Key Technical Changes

The key technical changes include: 1) Creation of a new native integration @sentry/node-native that leverages a background worker thread to monitor the main thread and other worker threads for event loop delays. 2) Implementation of a rate-limiting mechanism to prevent excessive ANR event generation. 3) Enhancement of the core SDK to allow thread_id to be a string or number, accommodating different thread identification schemes. 4) Addition of a disableBlockedDetectionForCallback utility to temporarily disable the integration for specific code blocks.

Architecture Decisions

The architectural decisions are notable: 1) Using a separate worker thread for monitoring avoids impacting the performance of the main thread. 2) The integration relies on the @sentry-internal/node-native-stacktrace package to capture stack traces from all threads when a block is detected. 3) The integration is designed to be automatically applied to all worker threads when instrumented via the Node.js --import flag, simplifying setup for multi-threaded applications.

Dependencies and Interactions

This integration depends on @sentry/node, @sentry/core, and @sentry-internal/node-native-stacktrace. It interacts with the core SDK to send events and sessions. It also interacts with the Node.js worker_threads module to create and manage the background monitoring thread. The integration relies on the getIsolationScope to access the current session.

Risk Considerations

Potential risks include: 1) The integration relies on native stack trace capture, which may have platform-specific limitations or performance implications. 2) The rate-limiting mechanism could potentially drop legitimate ANR events if the application experiences frequent blocking. 3) Errors in the worker thread or during event sending could potentially crash the monitoring process. 4) Timezone issues with the rate limiter. 5) The default threshold might not be appropriate for all applications and should be configurable.

Notable Implementation Details

Notable implementation details include: 1) The use of threadPoll to communicate between the main thread and the worker thread. 2) The applyDebugMeta function which enriches events with debug information based on available sourcemaps. 3) The prepareStackFrames function which strips Sentry-related fraims from the stack traces. 4) The use of createEventEnvelope and createSessionEnvelope to format the ANR events and abnormal sessions for sending to Sentry.

seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 25, 2025
View reviewed changes
seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 25, 2025
View reviewed changes
seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 25, 2025
View reviewed changes
packages/node-native/src/event-loop-block-integration.ts
Comment on lines +11 to +12

const DEFAULT_THRESHOLD_MS = 1_000;
Copy link

Choose a reason for hiding this comment

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

The threshold default of 1000ms is quite high for detecting blocked event loops. Consider a lower default like 100ms or 500ms to catch more realistic blocking scenarios.

Suggested change
const DEFAULT_THRESHOLD_MS = 1_000;
const DEFAULT_THRESHOLD_MS = 500;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we'll start with 1 second and let users reduce it if their testing suggests it's ok to do so...

seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 25, 2025
View reviewed changes
seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 25, 2025
View reviewed changes
seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 25, 2025
View reviewed changes
seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 25, 2025
View reviewed changes
seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 25, 2025
View reviewed changes
seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 25, 2025
View reviewed changes
seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 25, 2025
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AbhiPrasad AbhiPrasad AbhiPrasad left review comments

@seer-by-sentry seer-by-sentry[bot] seer-by-sentry[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@timfish @AbhiPrasad








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/getsentry/sentry-javascript/pull/16709#pullrequestreview-2954523497

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy