Skip to content

Enable APM for 30% of requests, add cleanup cron, rename admin URL, and send user ID - #600

Open
cycomachead wants to merge 1 commit into
mainfrom
cycomachead/268-update-apm-admin-url-user-id/1
Open

cycomachead wants to merge 1 commit into
mainfrom
cycomachead/268-update-apm-admin-url-user-id/1

Conversation

@cycomachead

Copy link
Copy Markdown
Contributor

General Info

  • Pivotal Tracker Story
  • Breaking?

Changes

Configures Faultline per the ticket: renames the dashboard URL, enables sampled APM, adds cleanup for retained data, and ensures a basic user ID is sent with every error.

  • Dashboard URL: mounted at /admin/faultline (was /admin/errors); ignore lists and route helpers updated accordingly.
  • APM enabled at 30% sample rate with 30-day trace retention. APM ignore paths now skip the load balancer health check (/status/health_check) and the dashboard itself.
  • FaultlineCleanupJob (renamed from an APM-only job): Faultline records retention_days/apm_retention_days but never enforces them itself. This job now nightly:
    • Deletes APM traces and attached CPU profiles past apm_retention_days.
    • Deletes error occurrences (and their context rows) past retention_days, recomputes the occurrences_count counter cache on affected groups, and removes groups left empty — except groups marked ignored, so a recurrence doesn't silently open a fresh alerting group.
    • Runs via the existing GoodJob cron (config/application.rb, key faultline_cleanup) nightly at 3:30 AM Pacific.
  • User ID: added a custom_context that attaches the Canvas uid from the session to every occurrence, in addition to Faultline's existing current_user tracking, so a user is identifiable even when no controller instance is available.
  • Bug fix found along the way: unhandled request exceptions were being recorded twice — once by Faultline's middleware (with request/user context) and again by Rails' executor re-reporting to the error subscriber (source application.action_dispatch, no user/URL context). Added a before_track hook to drop that duplicate report. Job errors and explicit Rails.error calls are unaffected.

Testing

  • spec/config/faultline_spec.rb: pins mount path, ignore lists, custom context, dedupe hook, APM settings and retention values.
  • spec/config/good_job_cron_spec.rb: verifies the faultline_cleanup cron schedule.
  • spec/jobs/faultline_cleanup_job_spec.rb: covers stale vs. fresh traces/profiles, occurrence + context deletion, counter cache recompute, empty-group removal, the ignored-group exception, and retention_days: nil (keep forever).
  • spec/requests/faultline_user_identification_spec.rb: signs in through the real OmniAuth flow, forces a controller exception, and asserts exactly one occurrence is recorded with the correct user ID, email, and Canvas uid — plus the anonymous case.
  • Full RSpec suite and rubocop pass; also smoke-tested the new /admin/faultline URL (401 for anonymous, both error and performance pages render for admins).

Documentation

Updated docs/developers.md: added faultline_cleanup to the cron table and documented what the job enforces and why (Faultline stores retention settings but doesn't act on them).

Checklist

  • Name of branch corresponds to story

Superconductor Ticket Implementation | App Preview | Guided Review

- Mount Faultline dashboard at /admin/faultline
- Enable APM at 30% sample rate with 30-day retention
- Add FaultlineCleanupJob for nightly APM and error data cleanup
- Attach Canvas user ID from session context to error occurrences
- Add deduplication hook to prevent double-reporting of request errors
- Ignore health check endpoint and dashboard routes in APM/error tracking

https://www.superconductor.com/tickets/nFgQfHNQTKkj/implementations/KnNWkdJ8GtQN?message_id=JrGG8cF7qBjw#message_JrGG8cF7qBjw

Co-authored-by: Claude Code <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant