Show the real alert total and make dashboard updates visible - #109
Conversation
paolosalvatori
left a comment
There was a problem hiding this comment.
Review — Show the real alert total and make dashboard updates visible
The diagnosis is right and the approach is the correct one: the headline was rendering len(alerts) against a list capped at 25, and reading partition runtime metadata is the right way to get a true count without consuming events or paging. Seven findings, one HIGH, all inline.
The HIGH is worth fixing before merge (app.py:79-80): hub_total()'s except Exception: return 0 makes a failed read look like an empty hub. Since 0 is a real value, data.total_alerts ?? (data.alerts || []).length never falls back, so the headline shows 0 with 25 alerts listed underneath it — and the next good refresh renders a +<entire total> delta badge claiming they all arrived in one interval. That is the same "actively misleading" failure mode this PR set out to remove, re-entered through the error path. The PR description defends the idiom as matching four other helpers, but those return empty collections that render as "no data yet"; partition_snapshot() — same client, same API, same kind of read — deliberately does not catch at all.
Rules applied
~/localstack/localstack-pro/localstack-pro-azure/CLAUDE.md.claude/rules/azure/common/—coding-style.md,hooks.md,patterns.md,security.md,testing.md.claude/rules/azure/python/—coding-style.md,hooks.md,patterns.md,security.md,testing.md.claude/rules/azure/emulator/azure-event-hubs.md(service context)
No *.bicep, *.tf, *.sh or *.bats files are touched, so those rule directories were not required. All expected files were present and readable.
Interpretation applied: every rule above is paths-scoped to localstack-pro-azure/**, and this PR is in localstack-azure-samples, so none attach automatically. I applied the language-agnostic ones (DRY, error handling, hardcoded/rebuilt values, documenting approximate logic, verify-against-Microsoft-Learn) as house style, and did not apply emulator-only rules (ResourceModel/fill(), AccountRegionBundle, AzureError envelopes, @markers.cloud.*), which have no meaning in a Flask sample.
Existing comments
None found — no reviews, inline comments, issue comments, or bot output on this PR.
Findings by severity
| Severity | File | Line | Finding |
|---|---|---|---|
| HIGH | app.py |
79-80 | Failed metadata read reports 0 alerts, then fabricates a +N spike |
| MEDIUM | app.py |
75-78 | Per-partition count duplicated from partition_snapshot() |
| MEDIUM | app.py |
63-68 | Docstring claims a live total; the metric is a high-water mark |
| MEDIUM | index.html |
204-208 | A failed refresh leaves the page reading "updating..." forever |
| MEDIUM | index.html |
73 | "(25 most recent)" hardcodes the Python page size |
| LOW | index.html |
23 | Delta badge renders below the number, not beside it |
| LOW | index.html |
134 | Badge hide timer never cancelled; fresh badge can be hidden early |
Clean files
samples/eventhubs/python/src/dashboard/static/style.css
Verified, not flagged
get_partition_properties()returnsdict[str, Any]withlast_enqueued_sequence_number(int) — the subscript access is correct againstazure-eventhub5.x (_producer_client.py:843,_client_base.py:495).- Empty partition →
last_enqueued_sequence_number == -1, so+1correctly yields 0 rather than a phantom event. Confirmed in the emulator (eventhub/partition_log.py:82-84) and consistent with the SDK's documented-1..Int64.MaxValuerange. element.parentElement.querySelector('.stat-delta')resolves correctly for all three cards, andremove→void offsetWidth→addis the correct animation-restart idiom.previous !== undefinedcorrectly suppresses a flash on first paint.- Using
EventHubProducerClientfor a read-only metadata call on a Listen-first connection string matches whatpartition_snapshot()already does successfully.
Note — emulator divergence surfaced by this change (different repo, not blocking)
localstack-pro's eventhub/partition_log.py:82-84 derives last_enqueued_sequence_number from the retained record list, so it falls back to -1 once retention trims a partition empty. Real Azure keeps the high-water mark and flips IsEmpty instead (PartitionProperties). Against the emulator a long-running demo would therefore see this dashboard's alert total drop back to 0 after a trim, where real Azure holds it. Not caused by this PR and not reachable in a normal demo window, but worth a localstack-pro issue now that this sample depends on that field.
Service parity gaps: not applicable — this PR changes a sample dashboard, not an emulated service's control or data plane.
Verdict
Approve — the diagnosis is right and hub_total() is the correct fix for a headline that was rendering a page size; the HIGH is a contained error-handling flaw in the new helper, not a problem with the approach. Worth fixing before merge so the delta badge cannot invent a spike, but nothing here needs a redesign.
Motivation
Two problems surfaced while demonstrating the Event Hubs sample, both of which make the dashboard actively misleading rather than merely plain.
The fraud alert count sat at 25 and never moved, no matter how many alerts were generated. It was not a stuck counter: the headline number was rendering the length of the recent-alerts list below it, which is capped at 25. So the sample's central claim — that fraud alerts accumulate as suspicious payments arrive — silently stopped being demonstrable exactly when it got interesting.
Second, a refresh that changed something looked identical to one that changed nothing. With the page polling on a timer, there was no way to tell which numbers had moved, or whether the pipeline was doing anything at all.
Changes
hub_total(), which reads the true event count for a hub from partition runtime metadata (summinglast_enqueued_sequence_number + 1across partitions). This does not consume events and is not bounded by any page size — counting by reading the hub would not scale and would reintroduce the same class of bug. The overview payload now carriestotal_alertsfrom it, so the headline figure is the real total.+Ndelta badge for a few seconds, via a smallsetStat()helper and two CSS rules. Cause and effect are now visible during a demo without watching logs.Tests
Verified by hand against the running sample: alerts climb past 25 and keep climbing, the total matches the number of alerts actually published, and the flash plus delta badge fire only on refreshes where a value genuinely moved.
hub_total()follows the sameexcept Exception: return <empty>idiom as the four other helpers inapp.py, so a transient Event Hubs error degrades the affected figure instead of breaking the page.Related
Follow-up to the Event Hubs fraud detection sample (#104).
SMF-790