ESS-4327: Added option to set metric name when using _timer decorator - #156
Conversation
There was a problem hiding this comment.
Pull request overview
Adds configurability to the internal latency-metric decorator so the client can emit distinct Application Insights metric names for different read paths, improving observability of egress usage.
Changes:
- Refactors
_timerinto a decorator factory that accepts ametric_name. - Updates
Client.get()andClient.get_samples_aggregate()to emit distinct metric names.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| "elapsed": elapsed_time, | ||
| "number-of-samples": number_of_samples, | ||
| } | ||
| metric().info(metric_name, extra=properties) |
There was a problem hiding this comment.
NIT: another suggestion - put the function name to custom properties. So that the trace name in App Insights will be the same, but the exact function could be extracted from the properties.
There was a problem hiding this comment.
Yeah it's a valid point, I'd definitely go that route if there were other "Timer"s but since it's just these two anyway, I'm inclined to keep it as is for now :)
No user facing changes, but this allows us to track egress metrics better by being able to differentiate between .get() and .get_samples_aggregate() methods in Application Insights.