Skip to content

refactor: security and rate limit metrics come from spans, and SecurityMetrics and RateLimitMetrics are gone - #404

Merged
HardMax71 merged 1 commit into
mainfrom
refactor/security-rate-limit-metrics-from-spans
Oct 19, 2026
Merged

HardMax71 merged 1 commit into
mainfrom
refactor/security-rate-limit-metrics-from-spans

Conversation

@HardMax71

@HardMax71 HardMax71 commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Summary

Part of #389, the security and rate limit area. The 15 instruments in SecurityMetrics and RateLimitMetrics now come from spans, and every label survives as a span attribute, as decided for #389 (keep the labels instead of switching to HTTP route metrics).

Changes

AuthService.login runs in an auth.login span with method and success. Token and CSRF rejections, admin route checks, lockouts, logins on locked accounts, weak passwords and admin password resets each leave an auth.* span where the metric used to be recorded. RateLimitService leaves a rate_limit.request span per check and a rate_limit.decision span when a rule applies. The bypass counter comes from requests whose algorithm is bypassed. The collector's signaltometrics connector makes the same 15 series, and the two dashboards use the new label names (auth_reason, rate_limit_endpoint, ...). The services take trace.Tracer.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • CI/CD or tooling changes

Test Plan

  • Backend unit tests pass (cd backend && uv run pytest tests/unit)
  • Frontend unit tests pass (cd frontend && npm run test)
  • Backend E2E tests pass (docker compose exec -T backend sh -c 'uv sync --group test --no-dev --frozen --no-install-project --no-build && pytest tests/e2e')
  • Frontend E2E tests pass (cd frontend && npm run test:e2e)
  • API spec and client regenerated if routes or schemas changed (./deploy.sh openapi, then ./deploy.sh types), never edited by hand
  • Manual testing performed

Validation: unit 570 passed, contract 3 passed, e2e 488 passed and 1 skipped. Driving the services from the API container (a bad token, a failed and a good login, a too-short password, five rate limit checks) gave exactly those counts at the collector.

Checklist

  • My code follows the project's style guidelines
  • I have run pre-commit run --all-files and all checks pass
  • I have added tests that prove my fix/feature works
  • I have updated documentation if needed
  • My changes don't introduce new warnings

Summary by cubic

Replaces the SecurityMetrics and RateLimitMetrics instrument classes with spans, so all 15 security and rate limit metrics are now derived from traces.

  • Logins, token and CSRF rejections, admin checks, lockouts, weak passwords, and password resets each leave an auth.* span; rate limit checks leave rate_limit.request and rate_limit.decision spans.
  • The collector's signaltometrics connector produces the same series, and every label survives as a span attribute.
  • Services now take a trace.Tracer instead of the metric classes; the dashboards and metrics reference use the new label names (auth_reason, rate_limit_endpoint, ...); the bypass counter now comes from requests whose algorithm is bypassed.

Written for commit 1947c75. Summary will update on new commits.

View guided diff Turn on auto-fix

…tyMetrics and RateLimitMetrics are gone

Each login runs in an auth.login span, and token and CSRF rejections, admin checks, lockouts, weak passwords and password resets leave an auth.* span where the metric used to be recorded. RateLimitService leaves rate_limit.request and rate_limit.decision spans. The collector's signaltometrics connector makes the same 15 series from them, and every label survives as a span attribute (decision 3 on #389). The bypass counter now comes from requests whose algorithm is bypassed. Part of #389.
@HardMax71
HardMax71 enabled auto-merge (squash) October 19, 2026 00:06
@coderabbitai

coderabbitai Bot commented Oct 19, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d3bdacf7-fcd8-4b54-bfa0-9d0c1e79276d
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added documentation Improvements or additions to documentation area: api FastAPI routes, auth, rate limiting, settings area: observability Metrics, dashboards, alerts, logs, traces, OTel collector labels Oct 19, 2026
@sonarqubecloud

Copy link
Copy Markdown

@codecov-commenter

codecov-commenter commented Oct 19, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
backend/app/core/security.py 90.00% 1 Missing ⚠️
backend/app/services/auth_service.py 90.90% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Flag Coverage Δ
backend-e2e 82.83% <79.48%> (-0.11%) ⬇️
backend-unit 74.46% <64.10%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
backend/app/core/metrics/__init__.py 100.00% <ø> (ø)
backend/app/core/providers.py 95.33% <100.00%> (-0.11%) ⬇️
backend/app/services/admin/admin_user_service.py 100.00% <100.00%> (ø)
backend/app/services/rate_limit_service.py 95.97% <100.00%> (+0.04%) ⬆️
backend/app/core/security.py 96.90% <90.00%> (ø)
backend/app/services/auth_service.py 95.41% <90.90%> (+0.17%) ⬆️
Components Coverage Δ
Backend 92.15% <94.87%> (-0.05%) ⬇️
Frontend 92.60% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@HardMax71
HardMax71 merged commit 3c57366 into main Oct 19, 2026
21 checks passed
@HardMax71
HardMax71 deleted the refactor/security-rate-limit-metrics-from-spans branch October 19, 2026 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: api FastAPI routes, auth, rate limiting, settings area: observability Metrics, dashboards, alerts, logs, traces, OTel collector documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants