-
Notifications
You must be signed in to change notification settings - Fork 66
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
WIP statsd metrics eps appointments #20335
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One request, one question open :)
unless response.status == 200 | ||
# log failures | ||
StatsD.increment(STATSD_KEY, tags: ["failures:#{response.body}"]) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the same code is used, could this be extracted to a method like
def track_metric(key, tag)
StatsDMetric.new(key: key).save
StatsD.increment(key, tags: [tag])
end
and then just do:
track_metric(STATSD_KEY, 'get_appointments call completed')
(or whatever the tag is)
StatsD.increment(STATSD_KEY, tags: ['create_draft_appointment call completed']) | ||
unless response.status == 200 | ||
# log failures | ||
StatsD.increment(STATSD_KEY, tags: ["failures:#{response.body}"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure we can log the response body if it had private data in it, but it might be totally fine. It's obviously very useful. Question for @randomsync or @kanchanasuriya
@@ -10,6 +12,13 @@ class AppointmentService < BaseService | |||
def get_appointments | |||
response = perform(:get, "/#{config.base_path}/appointments?patientId=#{patient_id}", | |||
{}, headers) | |||
|
|||
track_metric(STATSD_KEY, 'get_appointments call completed') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please see if we can make use of existing vaos_logging middleware?
Summary
Related issue(s)
Testing done