[management] add context cancel monitoring

[pascal-fischer]

Describe your changes

Issue ticket number and link

Stack

Checklist

• Is it a bug fix
• Is a typo/documentation fix
• Is a feature enhancement
• It is a refactor
• Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

• I added/updated documentation for this change
• Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

• Refactor

  • Adjusted internal database query execution to change how request contexts are applied, improving reliability of DB operations.

• Chores

  • Added cancellation-aware request logging to surface when handlers are canceled mid-request with additional diagnostic details.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a798087c-fe33-43c9-8b71-d0f3dedf4d55

📥 Commits

Reviewing files that changed from the base of the PR and between cde7d85 and 7318769.

📒 Files selected for processing (1)

• management/server/telemetry/http_api_metrics.go

🚧 Files skipped from review as they are similar to previous changes (1)

• management/server/telemetry/http_api_metrics.go

---

📝 Walkthrough

Walkthrough

Removed context propagation from multiple GORM calls in SqlStore so queries/mutations run on the DB handle without WithContext(ctx). Added a cancellation-aware debug log in HTTP API metrics via a completion channel and context.AfterFunc to log when handlers are canceled mid-flight.

Changes

Cohort / File(s)	Summary
Database context removal management/server/store/sql_store.go	Removed WithContext(ctx) from ~16 GORM call sites (e.g., GetCustomDomainsCounts, GetProxyAccessTokenByHashedToken, GetAllProxyAccessTokens, SaveProxyAccessToken, RevokeProxyAccessToken, MarkProxyAccessTokenUsed, GetServicesByCluster*, GetAccountAccessLogs, DeleteOldAccessLogs, SaveProxy, UpdateProxyHeartbeat, GetActiveProxyClusterAddresses, getClusterCapability, CleanupStaleProxies).
Request cancellation logging management/server/telemetry/http_api_metrics.go	Added handlerDone channel and context.AfterFunc(ctx, ...) to emit a debug log when a request context is canceled before the handler finishes; closes handlerDone after ServeHTTP to suppress the log if the handler completed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• [management] Add custom domain counts and service metrics to self-hosted metrics #5414 — Modifies the same GetCustomDomainsCounts method; directly related to the SqlStore query changes.

Poem

🐰 I hop through lines of code so neat,
I loosen contexts, let queries meet.
When handlers cancel mid‑flight’s tune,
I whisper why, beneath the moon.
🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description uses the provided template but lacks critical information: 'Describe your changes' section is empty, no issue ticket is referenced, and no explanation is provided for why documentation is not needed despite significant code changes.	Add a detailed description of the changes in the 'Describe your changes' section, reference the issue ticket number, and explain why documentation is not needed for the context cancellation monitoring feature.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title '[management] add context cancel monitoring' clearly summarizes the main change: adding context cancellation monitoring to the management module, which aligns with the telemetry middleware changes and the commit message focus.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/context-cancel-monitor

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

management/server/store/sql_store.go (1)

5297-5308: Add context to the access-log count and find queries.

Lines 5297 and 5308 build both queries from plain s.db, so a canceled caller no longer interrupts either scan. Since Count() and Find() require context propagation to respect deadlines and cancellation, keep WithContext(ctx) on these queries or use a request-scoped context to allow request cancellation to terminate the operations.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@management/server/store/sql_store.go` around lines 5297 - 5308, The current
access-log queries are built from s.db so they don't propagate the request
context; update the query construction to use the request context by calling
WithContext(ctx) when creating the DB query (e.g., change the creation of
baseQuery used with applyAccessLogFilters and Count to start with
s.db.WithContext(ctx).Model(&accesslogs.AccessLogEntry{}) and likewise ensure
the subsequent query variable (used with Find) is created from
s.db.WithContext(ctx) or reuses the context-aware baseQuery so Count and Find
honor cancellation/deadlines in applyAccessLogFilters, Count, and Find.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@management/server/telemetry/http_api_metrics.go`:
- Around line 186-189: The AfterFunc callback is being registered with
context.AfterFunc and never stopped on the normal return path, causing a warning
to always fire and creating a race by capturing the mutable ctx; change the code
to capture a dedicated ctxForTimer (or similar) when you call context.AfterFunc,
store the returned stop function, and ensure you call that stop function (e.g.,
via defer stop() or explicitly before returning from the middleware/handler) on
the successful path; update the closure to reference ctxForTimer (not the
mutable ctx) and keep the log using r.Method, r.URL.Path, reqID and reqStart so
the timer is properly canceled and no spurious warnings are logged.

---

Nitpick comments:
In `@management/server/store/sql_store.go`:
- Around line 5297-5308: The current access-log queries are built from s.db so
they don't propagate the request context; update the query construction to use
the request context by calling WithContext(ctx) when creating the DB query
(e.g., change the creation of baseQuery used with applyAccessLogFilters and
Count to start with s.db.WithContext(ctx).Model(&accesslogs.AccessLogEntry{})
and likewise ensure the subsequent query variable (used with Find) is created
from s.db.WithContext(ctx) or reuses the context-aware baseQuery so Count and
Find honor cancellation/deadlines in applyAccessLogFilters, Count, and Find.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e4206818-e8cb-4c43-88fd-32e4582b76b9

📥 Commits

Reviewing files that changed from the base of the PR and between 4eed459 and cde7d85.

📒 Files selected for processing (2)

• management/server/store/sql_store.go
• management/server/telemetry/http_api_metrics.go

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud