department-of-veterans-affairs / abd-vro

To get Veterans benefits in minutes, VRO software uses health evidence data to help fast track disability claims.
Other
19 stars 6 forks source link

Introduces a metric_logger ruby implementation for use in the bgs sevice #3075

Closed nelsestu closed 3 months ago

nelsestu commented 3 months ago

Adds a request count metric for requests started, and requests completed, and a request latency metric for calls out to the BGS service.

What was the problem?

We had extremely limited visibility into the goings on with the external calls that the VRO svc-bgs-api client service makes.

Associated tickets or Slack threads:

How does this fix it?[^1]

With this PR, we'll be able to see request start, complete counts as well as tracking the request latency.

How to test this PR

[^1]: Pull-Requests guidelines. If PR is significant, update Current Software State wiki page. [^secrel]: To check if a PR will succeed in the SecRel workflow, test PRs in the SecRel pipeline.

github-actions[bot] commented 3 months ago

Test Results

116 tests  ±0   116 :white_check_mark: ±0   35s :stopwatch: -3s  34 suites ±0     0 :zzz: ±0   34 files   ±0     0 :x: ±0 

Results for commit 413942a8. ± Comparison against base commit 9e235bdc.

:recycle: This comment has been updated with latest results.

github-actions[bot] commented 3 months ago

JaCoCo Test Coverage

Overall Project 66.8% :x:

There is no coverage information present for the Files changed

Ponnia-M commented 3 months ago

@lisac Have these changes been tested locally and they were successful?

lisac commented 3 months ago

@lisac Have these changes been tested locally and they were successful?

Yes, these were tested locally with unit tests. Unit tests were successful.

lisac commented 3 months ago

reviewers: please hold. i managed to introduce buggy behavior, i suspect tracing back to reconciling merges around ruby gems. thank you to Ponnia for flagging.

lisac commented 3 months ago

@Ponnia-M : thank you for flagging the issue with the spike in failed GH checks. I think it's resolved - would you please re-review?

It turns out the Gemfile.lock got out of sync when I merged in other changes, and that led to failures starting up docker containers. With my latest update (eb00f4a), I see the GH checks are now passing, with the exception of one; however, that one is a known failure that's also present in the default branch.