feat(core): Aggregate TurboModule call counts and latency per (module, method, kind)#6377
feat(core): Aggregate TurboModule call counts and latency per (module, method, kind)#6377alwx wants to merge 2 commits into
Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
…, method, kind) Adds a small fixed-bucket histogram + counters per `(module, method, kind)` fed by the existing `wrapTurboModule` instrumentation. Aggregates flush on transaction finish (synthetic `turbo_modules.aggregate` child span + headline measurements on the root span) and on a lazy timer (info-level event for long-running sessions without transactions). Closes #6164. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Drop edge-case tests in favor of one happy-path check per surface. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
alwx
force-pushed
the
alwx/features/aggregated-stats
branch
from
933fdb9 to
5702b46
Compare
|
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
| // Don't let a misbehaving observer corrupt the aggregate. | ||
| try { | ||
| onFirstRecordAfterEmpty(); | ||
| } catch { |
There was a problem hiding this comment.
Periodic flush re-arms itself via SDK's own captureEnvelope call, defeating the idle-session guard
After each periodic flush, flushPeriodicAggregate calls client.captureEvent, which (on the native transport) ultimately sends the envelope through RNSentry.captureEnvelope. That method is wrapped by wrapTurboModule (it is not in RNSENTRY_SKIP, and RNSentry is not ignored by default), so it records a TurboModule call. Because drainTurboModuleAggregate() already cleared the map, the recorded call sees wasEmpty === true and fires onFirstRecordAfterEmpty, scheduling another 30s timer — even in a fully idle session with no user activity. This defeats the design intent of arming the timer only on the first record after a drain so silent sessions don't churn timers; instead the SDK perpetually re-flushes its own envelope-send overhead every interval. Consider adding RNSentry to the default ignoreTurboModules set, or suppressing the re-arm when the only recorded activity originates from the flush's own capture path.
Evidence
flushPeriodicAggregate(turboModuleContext.ts) callsdrainTurboModuleAggregate()(clearing the map) thenclient.captureEvent.wrapTurboModulemutates the live module in place (target[key] = wrapper), andwrapper.ts:234sends envelopes through that sameRNSentry.captureEnvelopeinstance, so the call is tracked.captureEnvelopeis absent fromRNSENTRY_SKIP(turboModuleContext.ts) andRNSentryis not in the defaultignoreTurboModules, sorecordTurboModuleCallruns for it.- At turboModuleAggregator.ts:120
wasEmpty = aggregates.size === 0is true right after the drain, so the guard at line 153 firesonFirstRecordAfterEmpty, re-arming the timer (turboModuleContext.tssetOnFirstTurboModuleRecord). - The existing test stubs
captureEventasjest.fn(), so the native send/record cycle never occurs and the self-re-arming behavior is not exercised.
Identified by Warden code-review · 2Q2-JQP
1 participant
📢 Type of change
📜 Description
Adds per-
(module, method, kind)aggregation for TurboModule invocations on top of the existingwrapTurboModuleinstrumentation. Aggregates flush at two points:turbo_modules.aggregatechild span on the event carries the per-method breakdown in span attributes; headline measurements (turbo_modules.call_count,.error_count,.total_ms,.top_module_ms) land on the root span for the standard Measurements panel.info-level event taggedevent.kind=turbo_modules.aggregateso idle sessions without transactions still produce a signal. Armed only on the first record after a drain, so silent sessions don't churn timers.New
turboModuleContextIntegrationoptions:enableAggregateStats(defaulttrue),aggregateFlushIntervalMs(default30000,0disables),ignoreTurboModules(excludes modules from counters; still wrapped for crash attribution).O(1) per call: counters + 6-bucket histogram (
<1ms, <5ms, <20ms, <100ms, <500ms, >=500ms). Failures inside the aggregator never break the wrapped TurboModule call.💡 Motivation and Context
Closes #6164. Per-call spans for every TurboModule invocation would explode span counts on hot async paths; aggregated counters are the right tradeoff.
💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps