feat(core): Add isolateTrace option to Sentry.withMonitor()
#18079
Conversation
Add optional isolateTrace boolean property to MonitorConfig interface to allow creating separate traces for each withMonitor execution.
Modify withMonitor to create new traces when isolateTrace option is enabled. When isolateTrace: true, starts a new span with monitor-specific naming to allow distinguishing between different cron job executions.
Add unit tests for the new isolateTrace option in withMonitor: - Test isolateTrace: true option acceptance - Test isolateTrace: false maintains default behavior - Test isolateTrace works with async operations
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
Hi @naaa760 thanks for opening this PR!
I looked over it and I think we still need to make some changes to the PR. We also discussed this in the team internally, so here's how we'd like this option to be implemented:
If isolateTrace is true:
- we only start a new trace. We can achieve this by calling
startNewTraceand basically using the same callback you're currently adding to thestartSpancallback. - we on purpose do not start a new span. We can revisit this in the future but for now, this is something, we'd rather like our users to take care of. The primary reason is that none of our cron instrumentation currently starts a new span, so if we were to do this, we should more universally agree on it (possibly across other SDKs).
=> isolateTrace should only apply on trace level, but not do more than that.
Furthermore, we need to add some tests that assert on the changed behaviour. The easiest way to test this properly would be to add new tests to our node-integration-tests dev package. In this test you can assert that the trace ids of the two created cron check-ins are distinct. Here's a link to a cron integration test. You can't use it 1:1 but it should provide a starting point.
Update withMonitor to use startNewTrace() instead of startSpan() when isolateTrace is enabled, following PR feedback to only isolate traces without creating spans.
Add node integration test to verify that withMonitor calls with isolateTrace: true generate different trace IDs, confirming trace isolation works as expected.
naaa760
force-pushed
the
feat/monitor-trace
branch
from
4b663d6 to
a16ae77
Compare
Lms24
changed the title
isolateTrace option to Sentry.withMonitor()
2 participants
fix: #18058
Problem: Currently
withMonitor()reuses the same trace for all cron executions, making it impossible to distinguish between different runs in Sentry.Solution: Added optional
isolateTrace: booleantoMonitorConfigthat creates a separate trace for each monitor execution when enabled.Benefits:
Changes:
isolateTraceoption toMonitorConfiginterfacewithMonitor()to start new spans when enabled