Repository navigation
cherry-pick: fix(analytics): report aggregate maxDepth as the sum of per-thread peaks (#3017 → v5.3) - #3125
Merged
Conversation
Aggregation folded every numeric measure as a running mean, so the aggregate maxDepth of the transaction-queue-depth gauges was a sum of per-thread means of per-sample peaks: neither a peak nor a bound. Peak-named measures (/^max[A-Z]/) now fold with Math.max within a thread across the period's samples. Across threads the per-thread peaks are still summed. Dispatch-Task: harper-analytics-maxdepth-aggregation Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmFNiYmUGvY7X6mvX3PUgb
… test The cross-thread comment now states when summed peaks bound concurrent depth. The aggregation test disables live recording before seeding so the main thread's pending report cannot land inside its windows. Dispatch-Task: harper-analytics-maxdepth-aggregation Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmFNiYmUGvY7X6mvX3PUgb
Waiting one period and running a cycle after disabling recording lets an in-flight flush finish and consumes any report it left above the cursor, so no live main-thread report lands in the fixture windows. The probe's additive-named measure is maxCount, so the test no longer codifies a summed latency. Dispatch-Task: harper-analytics-maxdepth-aggregation Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CmFNiYmUGvY7X6mvX3PUgb
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the analytics aggregation cycle in resources/analytics/write.ts to aggregate peak-named measures (matching /^max[A-Z]/) using Math.max rather than a weighted average. It also introduces helper functions and a comprehensive unit test in unitTests/resources/analytics/aggregationCycle.test.js to validate that peak-named measures correctly sum per-thread peaks across threads. No review comments were provided for this pull request, and we have no further feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick of fix(analytics): report aggregate maxDepth as the sum of per-thread peaks onto
v5.3for 5.3.2.The release-cherry-pick bot built
cherry-pick/v5.3/pr-3017and its integration tests passed (run), but the branch was never fast-forwarded intov5.3. It merges cleanly onto the currentv5.3and changes onlyresources/analytics/write.tsand its unit test (+77/−1).Merged at the release owner's direction during the v5.3.2 cut.
🤖 Generated with Claude Code