feat(dashboard): add connection monitoring grouped by user, client, and database - #391
Open
dpage wants to merge 3 commits into
Open
feat(dashboard): add connection monitoring grouped by user, client, and database#391dpage wants to merge 3 commits into
dpage wants to merge 3 commits into
Conversation
…nd database The server dashboard previously exposed only a scalar active-connection count per database, taken from pg_stat_database.numbackends, with no breakdown of who was connected or from where. The collector has always stored one row per backend from pg_stat_activity, so the grouped view could be built by aggregating rows that were already being collected; no new collection was required. This adds a Connections section to the server dashboard, presenting the counts in three tabs (By User, By Client, By Database) with a per-group breakdown into total, active, idle, idle in transaction, and other backend states. A new GET /api/v1/metrics/connection-groups endpoint serves the section. The counts come from the single most recent snapshot inside the selected time range; the range decides only which snapshot is treated as the latest, and the figures are neither averaged nor peaked across it, which deliberately differs from the charted metrics elsewhere on the dashboard. Only rows with backend_type = 'client backend' are counted, so the background workers the probe also stores do not appear. Backends without a client address are grouped under 'local', and those without a role name or database under '(unknown)' and '(none)' respectively. The grouping expressions come from a hardcoded whitelist, and the connection ID and both window bounds are bound as parameters, so no caller-supplied value reaches the query text. Both CTEs carry the window bounds explicitly: whilst runtime pruning already avoided scanning irrelevant partitions, the bounds let the planner prune at plan time, which cut the snapshot Append from 90 sub-plans to 2 and mean latency by roughly 23% on a 90-partition fixture. The response is capped at 200 groups, ordered by total descending, so truncation drops only the smallest groups. Closes #346
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughAdds a server connection-groups metrics endpoint that aggregates the latest snapshot by user, client, or database. The dashboard adds a Connections section with tabs, counts, hostnames, timestamps, refresh handling, API contracts, tests, and documentation. ChangesConnection Groups Monitoring
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ConnectionsSection
participant useConnectionGroups
participant PerfSummaryHandler
participant PostgreSQL
User->>ConnectionsSection: select grouping tab
ConnectionsSection->>useConnectionGroups: update groupBy and timeRange
useConnectionGroups->>PerfSummaryHandler: GET connection-groups
PerfSummaryHandler->>PostgreSQL: aggregate latest connection snapshot
PostgreSQL-->>PerfSummaryHandler: grouped rows and collected_at
PerfSummaryHandler-->>useConnectionGroups: ConnectionGroupsResponse
useConnectionGroups-->>ConnectionsSection: render grouped counts
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 374 |
| Duplication | 24 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codacy raised two issues against the new code, and both are worth acting on rather than dismissing. Opengrep's go_sql_rule-concat-sqli fired, at Error level, on the tx.Query call, because the query text reached it from an fmt.Sprintf. Codacy itself scored this at 95% false-positive probability with the correct reasoning, and our own review agreed the code was not vulnerable, so this changes no behaviour. Rather than suppress the rule, the three queries are now composed as compile-time string constants through constant concatenation, so the query passed to tx.Query provably originates in a const and the taint path the rule follows no longer exists. The whitelist of grouping expressions becomes a compiler guarantee instead of a convention, and the format string's %% escaping disappears with it. The generated SQL was captured before the change and compared afterwards: all three groupings, and the defensive fallback, are byte-for-byte identical. A new test asserts the three queries still share one query body, so they cannot quietly diverge into three copies. One small regression: a constant expression cannot format an integer, so LIMIT 200 is now a literal rather than derived from maxConnectionGroups, and an existing test continues to tie the two together. Lizard reported the fetchData callback in useConnectionGroups at a cyclomatic complexity of 14 against a limit of 12. Three module-private helpers now own request-URL assembly, response normalisation, and error message derivation, leaving the hook's lifecycle logic in the callback. Measured with lizard, fetchData drops from 14 to 8, with no helper above 4. The defensive coercions moved rather than being removed, and a test was added for the authenticated-user guard, which the extraction split out into a separately reachable path.
![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
🤖 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 @.claude/golang-expert/metrics-queries.md:
- Around line 273-274: Wrap the long Markdown sentence in metrics-queries.md by
moving “uniformly careful” onto the next line, preserving the wording and
paragraph formatting.
In `@client/src/hooks/useConnectionGroups.ts`:
- Around line 69-117: Update fetchData to track a request generation or sequence
identifier for each invocation, and only apply groups, collectedAt, error, and
loading state changes when the resolving request is still the latest generation.
Keep the existing isMountedRef checks, but ensure superseded responses from
earlier connectionId, groupBy, or timeRange requests cannot overwrite the
current state.
In `@server/src/internal/api/openapi.go`:
- Around line 1401-1413: Update the ConnectionGroupRow schema’s Required list to
include client_hostname while retaining its Nullable: true definition, and apply
the same required-field change to the mirrored OpenAPI schema in the
documentation. Keep the existing response field and nullability behavior
unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7f24f713-0e3d-4828-981a-48998e5f2aa6
📒 Files selected for processing (18)
.claude/golang-expert/metrics-queries.mdclient/src/components/Dashboard/ServerDashboard/ConnectionsSection.tsxclient/src/components/Dashboard/ServerDashboard/__tests__/ConnectionsSection.test.tsxclient/src/components/Dashboard/ServerDashboard/__tests__/ServerDashboard.test.tsxclient/src/components/Dashboard/ServerDashboard/index.tsxclient/src/components/Dashboard/ServerDashboard/types.tsclient/src/hooks/__tests__/useConnectionGroups.test.tsclient/src/hooks/useConnectionGroups.tsdocs/admin-guide/api/openapi.jsondocs/admin-guide/api/reference.mddocs/changelog.mddocs/user-guide/dashboards/server.mdserver/src/internal/api/openapi.goserver/src/internal/api/openapi_test.goserver/src/internal/api/perf_summary_connection_groups.goserver/src/internal/api/perf_summary_connection_groups_sql_test.goserver/src/internal/api/perf_summary_connection_groups_test.goserver/src/internal/api/perf_summary_handlers.go
Guard against a stale response overwriting fresher state in the connection-groups hook. fetchData had no request sequencing, so an in-flight request that resolved after a newer one still committed its result, which meant rapid switching between the By User, By Client and By Database tabs, or a background auto-refresh landing after a tab change, could leave data for one grouping displayed under another. The existing mounted-ref check does not help, because it guards against post-unmount updates rather than out-of-order resolution. A request generation is now claimed at the start of each fetch and checked before committing, on the success path, the error path and the loading update, so a superseded request can neither commit data, nor raise an error over fresher data, nor clear the spinner belonging to the request that superseded it. The accompanying test settles two overlapping requests out of order and fails against the previous implementation. Declare client_hostname as required in the ConnectionGroupRow OpenAPI schema whilst keeping it nullable. The two are orthogonal: the field has no omitempty tag, so the key is always present, either as null or as a string, and omitting it from the required list understated the contract for generated clients. A test now asserts in both directions that the emitted key set and the schema's required list agree, for both schemas, so this cannot drift again. Also reflow the longest line of the golang-expert knowledge base, which was the file's only non-ASCII content, spelling out two constant names that had been abbreviated with ellipses.
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.
Summary
The server dashboard previously showed only a scalar active-connection count
per database, taken from
pg_stat_database.numbackends, with no breakdown ofwho was connected or from where. The collector has always stored one row per
backend from
pg_stat_activity, so this grouped view aggregates rows that werealready being collected; no new collection was required.
Adds a Connections section to the server dashboard, with three tabs (By
User, By Client, By Database), each listing one row per group broken down
into total, active, idle, idle in transaction, and other backend states. On
the By Client tab a reverse-resolved hostname appears beneath the address
where PostgreSQL recorded one, which needs
log_hostnameenabled.Adds
GET /api/v1/metrics/connection-groups, takingconnection_id,group_by(user/client/database) andtime_range, and returning anullable
collected_atalongside agroupsarray.Counts come from the single most recent snapshot within the selected time
range. The range decides only which snapshot counts as the latest; the
figures are neither averaged nor peaked across it, which deliberately differs
from the charted metrics elsewhere on the dashboard.
Only
backend_type = 'client backend'rows are counted, so the backgroundworkers the probe also stores (WAL writer, autovacuum) do not appear.
Backends with no client address are grouped under
local, and those with norole name or database under
(unknown)and(none).A note on one small deviation from the original design:
client_addr::textrenders the netmask on PostgreSQL 18, which would have labelled every client
192.0.2.10/32, so the query useshost(client_addr)instead.Security
Reviewed before pushing; SQL injection, authorisation and client-side XSS all
came back clean. The grouping expressions come from a hardcoded whitelist and
the connection ID and both window bounds are bound parameters, so no
caller-supplied value reaches the query text. Three findings were actioned:
Both CTEs now carry the window bounds explicitly. Runtime pruning already
avoided scanning irrelevant partitions, so the win is at plan time rather
than in I/O: the
snapshotAppenddrops from 90 sub-plans to 2, the plantree from 283 lines to 54, and mean latency by roughly 23% on a 90-partition
fixture. Verified as a semantic no-op across 10 result-set pairs.
The response is capped at 200 groups. Ordering is
total DESC, so truncationdiscards only the smallest groups; documented in the user guide.
Error logs in the new file are consistently sanitised.
Two findings were deliberately not actioned, as both are pre-existing
patterns shared with the sibling metrics handlers and better addressed
separately: a swallowed query error is logged at
DEBUGrather thanWARN(so a persistent failure is invisible to operators whilst the user correctly
sees an empty panel), and there is no per-endpoint rate limiting on
/api/v1/metrics/*.Test plan
the query text and that both partition-pruning bounds are present.
backend_typefilter, the label fallbacks, state bucketing including
idle in transaction (aborted), latest-snapshot selection acrossmultiple
collected_atvalues, out-of-range exclusion, the 200-groupcap, invalid parameters, permission denied, and method not allowed.
server/src/internal/apipasses in full;perf_summary_connection_groups.goat 100% statement coverage (72/72).
on both new files; full client suite green (3521 tests).
gofmtclean,golangci-lint0 issues,go vetclean,npm run lintclean.
deployed to the dev server.
Two pre-existing issues worth knowing about, neither caused by this change: the
whole-
server-tree test run flakes intermittently ininternal/apiwithshared-test-database interference (the package passes reliably on its own, and
runs with these tests excluded still fail), and
npm run typecheckhas a574-error baseline, none of which reference the files added here.
Closes #346
Summary by CodeRabbit
GET /api/v1/metrics/connection-groupswith time-range filtering and a top 200 group cap.