Add Prometheus metrics instrumentation to WikiStreamService - #68
Add Prometheus metrics instrumentation to WikiStreamService#68GergesShamon wants to merge 3 commits into
Conversation
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds Micrometer instrumentation to the SSE stream service, exposes Prometheus and Actuator metrics, records event-processing measurements, and updates production and test constructor wiring for ChangesSSE metrics instrumentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/qrdlife/wikiconnect/wikimonitor/service/WikiStreamService.java (1)
303-324: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not record queued replay work as send duration.
When
context.replayingis true, this task only queues a pending event and returns, butwiki.broadcast.durationstill records a sample. Move the timer around the actualsendIfMatchedcall or rename the metric description to reflect dispatch/queue processing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/qrdlife/wikiconnect/wikimonitor/service/WikiStreamService.java` around lines 303 - 324, The broadcast duration timer in the event handling block should measure only actual dispatch through sendIfMatched, not events queued during context.replaying. Move Timer.Sample creation to immediately before sendIfMatched, or otherwise stop recording it for the queue-and-return path while preserving existing exception handling and metric behavior for sends.
🧹 Nitpick comments (1)
src/test/java/org/qrdlife/wikiconnect/wikimonitor/service/WikiStreamServiceTest.java (1)
66-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd assertions for the new meters.
This setup only proves that construction accepts a registry; no test verifies metric registration or values. Add focused assertions for counters, replay size, timers, and the active-emitter gauge so instrumentation regressions are detected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/org/qrdlife/wikiconnect/wikimonitor/service/WikiStreamServiceTest.java` around lines 66 - 70, Add focused assertions in WikiStreamServiceTest for the meters registered by WikiStreamService: verify counter registration and values, replay-size metrics, timer presence or recordings, and the active-emitter gauge. Exercise the relevant service behavior as needed, and use the existing meterRegistry to locate metrics by their established names and tags.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/org/qrdlife/wikiconnect/wikimonitor/service/WikiStreamService.java`:
- Around line 448-454: Update the replay handling flow in WikiStreamService so
an empty entries collection records replay size 0 before returning. Preserve the
existing replaySize.record(replayed) behavior for non-empty and processed
replays, ensuring every reconnecting client contributes one metric sample.
- Around line 339-340: Update the matched-event accounting in sendIfMatched so
wiki.events.matched is incremented once per incoming event rather than once per
emitter; alternatively, rename the metric to explicitly represent matched
deliveries. Preserve the existing StreamEventDTO conversion and delivery
behavior.
- Around line 123-125: Update the active-emitter metric near
Gauge.builder("wiki.emitters.active", emitters, Map::size) so its measured value
matches the description: either include anonymous and unresolved-user
subscriptions in the tracked collection before they return, or revise the
description to explicitly state that it counts only authenticated registered
emitters. Keep the chosen metric source and description consistent.
---
Outside diff comments:
In
`@src/main/java/org/qrdlife/wikiconnect/wikimonitor/service/WikiStreamService.java`:
- Around line 303-324: The broadcast duration timer in the event handling block
should measure only actual dispatch through sendIfMatched, not events queued
during context.replaying. Move Timer.Sample creation to immediately before
sendIfMatched, or otherwise stop recording it for the queue-and-return path
while preserving existing exception handling and metric behavior for sends.
---
Nitpick comments:
In
`@src/test/java/org/qrdlife/wikiconnect/wikimonitor/service/WikiStreamServiceTest.java`:
- Around line 66-70: Add focused assertions in WikiStreamServiceTest for the
meters registered by WikiStreamService: verify counter registration and values,
replay-size metrics, timer presence or recordings, and the active-emitter gauge.
Exercise the relevant service behavior as needed, and use the existing
meterRegistry to locate metrics by their established names and tags.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 068b7b21-bb72-49f8-83e3-c2f9f6be6e1e
📒 Files selected for processing (4)
pom.xmlsrc/main/java/org/qrdlife/wikiconnect/wikimonitor/service/WikiStreamService.javasrc/main/resources/application.propertiessrc/test/java/org/qrdlife/wikiconnect/wikimonitor/service/WikiStreamServiceTest.java
| eventsMatched.increment(); | ||
| StreamEventDTO dto = StreamEventDTO.fromRecentChange(rc, matchedFilters); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Count matched events at the intended unit.
sendIfMatched runs once per emitter, so one incoming event matching three clients increments wiki.events.matched three times. That contradicts the event-level description; either rename it to represent matched deliveries or aggregate it once per incoming event.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/org/qrdlife/wikiconnect/wikimonitor/service/WikiStreamService.java`
around lines 339 - 340, Update the matched-event accounting in sendIfMatched so
wiki.events.matched is incremented once per incoming event rather than once per
emitter; alternatively, rename the metric to explicitly represent matched
deliveries. Preserve the existing StreamEventDTO conversion and delivery
behavior.
…r WikiStreamService performance monitoring
9c3a9c5 to
3d0f680
Compare
…avior - Update description of wiki.emitters.active gauge to explicitly state that it counts authenticated registered emitters. - Rename wiki.events.matched metric to wiki.events.matched.deliveries to reflect per-emitter delivery accounting. - Record replay size sample of 0 when client reconnects and event cache is empty in replayMissedEvents. - Add unit test verifying empty replay metric recording.
1 participant
Summary
Adds Micrometer/Prometheus metrics instrumentation to
WikiStreamServiceto enable monitoring and diagnostics of the SSE event pipeline (stream ingestion, caching, and broadcast stages).Changes
micrometer-registry-prometheusdependency topom.xmlMeterRegistryintoWikiStreamServiceand registered the following metrics:wiki.events.received(Counter) — total events received from the Wikimedia streamwiki.events.matched(Counter) — events matching at least one user filterwiki.event.process.duration(Timer) — time from parse to broadcast dispatchwiki.cache.write.duration(Timer) — Redis cache write latencywiki.broadcast.duration(Timer) — per-emitter SSE send durationwiki.stream.reconnects(Counter) — stream reconnect attemptswiki.replay.size(DistributionSummary) — number of events replayed per reconnecting clientwiki.emitters.active(Gauge) — currently connected SSE clientsprometheus,health, andmetricsactuator endpoints inapplication.propertiesevent.process,broadcast,cache.write) to support bottleneck diagnosisWikiStreamServiceTestto inject aSimpleMeterRegistryfor unit testsNotes
Testing
SimpleMeterRegistry./actuator/prometheusexposes the new metrics.Summary by CodeRabbit