arc/add_instrumentation_to_appoint_a_rep#27585
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Datadog/StatsD instrumentation around creating Power of Attorney (appoint-a-representative) requests in the Accredited Representative Portal, along with unit tests verifying the new metrics are emitted.
Changes:
- Increment a new overall request counter metric when a POA request is created.
- Increment a new “pathway” counter metric distinguishing rep-first vs org-first flows.
- Add service specs that stub
AccreditedRepresentativePortal::Monitoringand assert the counters are tracked.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| modules/accredited_representative_portal/app/services/accredited_representative_portal/power_of_attorney_request_service/create.rb | Emits new StatsD counters for POA request creation + pathway. |
| modules/accredited_representative_portal/spec/services/accredited_representative_portal/power_of_attorney_request_service/create_spec.rb | Adds unit tests to validate the new track_count calls. |
| request.save! | ||
|
|
||
| pathway = @registration_number.present? ? 'rep-first' : 'org-first' | ||
|
|
||
| Monitoring.new.track_count('ar.poa.request.count') | ||
| Monitoring.new.track_count("ar.poa.request.pathway.#{pathway}") | ||
| end |
There was a problem hiding this comment.
track_count is executed inside the DB transaction (after save! but before the transaction commits). If the transaction fails at commit time, these metrics will still be emitted even though the record wasn’t persisted. Consider moving the metric emission outside the ActiveRecord::Base.transaction block (or using an after_commit hook) so counts only reflect committed requests.
|
|
||
| request.save! | ||
|
|
||
| pathway = @registration_number.present? ? 'rep-first' : 'org-first' |
There was a problem hiding this comment.
The pathway metric name uses hyphenated segments (rep-first / org-first). This module’s existing StatsD/Datadog metric names appear to use only dots/underscores (e.g., ar.poa.request.decision.accept, ar.unique_session.count). To avoid introducing an inconsistent naming pattern (and potential metric-name restrictions), consider switching to underscore-separated names or emitting a single ar.poa.request.pathway metric with a pathway:rep_first|org_first tag.
| pathway = @registration_number.present? ? 'rep-first' : 'org-first' | |
| pathway = @registration_number.present? ? 'rep_first' : 'org_first' |
|
Your branch is 100+ commits behind, so I've updated it with the latest. LGTM otherwise! |
Summary
Related issue(s)
Testing done
What areas of the site does it impact?
get-help-from-accredited-representative/appoint-rep/
Acceptance criteria