Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Travel Pay / Add statsd logging #20805

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Travel Pay / Add statsd logging #20805

wants to merge 9 commits into from

Conversation

liztownd
Copy link
Contributor

@liztownd liztownd commented Feb 14, 2025

Summary

  • This work is behind a feature toggle (flipper): YES/NO No

  • (Summarize the changes that have been made to the platform)

    • Adding statsd logging to our external API calls to measure response time
  • (What is the solution, why is this the solution?)

    • We need a way to measure how long it takes for each response individually to track latency when calling the external API.
    • We are currently only tracking the latency for the entire process, not each step individually (i.e. getting tokens, then getting the claims takes a ms vs. getting the first token takes x ms, the second token takes y ms and actually getting the claims response takes z ms)
  • (Which team do you work for, does your team own the maintenance of this component?)

    • Travel Pay

Related issue(s)

Testing done

  • New code is covered by unit tests
  • Describe what the old behavior was prior to the change
  • We were logging only the total time to make each call at the controller level, now each individual step can be measured independently
  • Describe the steps required to verify your changes are working as expected. Exclusively stating 'Specs run' is NOT acceptable as appropriate testing
  • Update unit tests to check that the correct tags/prefixes were being sent to statsd, but I believe we won't know if it's logging correctly until it's merged and we start seeing metrics show up in datadog

What areas of the site does it impact?

(Describe what parts of the site are impacted andifcode touched other areas)
Travel Pay module only

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

Requested Feedback

I would love feedback as to the statsd prefix and tags we have chosen as we're not 100% sure if this is the best way to go about tracking/sorting/grouping/tagging the different metrics.

Copy link
Contributor

@kjduensing kjduensing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work.

@liztownd liztownd marked this pull request as ready for review February 14, 2025 19:54
@liztownd liztownd requested review from a team as code owners February 14, 2025 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants