-
Notifications
You must be signed in to change notification settings - Fork 418
Provide Default Values for .add_metric(unit=) and .add_metric(value=) #1180
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
Comments
Thanks for opening your first issue here! We'll come back to you as soon as we can. |
Hey @nsandar That's a great idea for This could be similar to how we've done default dimensions. I'm currently out of bandwidth to implement it until other higher priorities enhancements are done - I can however provide guidance to contribute it if you could help. We'd need a couple of things to implement this:
|
Hey, @heitorlessa, I would like to contribute a pr for this. # metrics.py
def add_metric(self, name: str, value: float, unit: Optional[Union[MetricUnit, str]] = None) -> None:
if unit is None and self.default_metric_unit is None:
raise MetricUnitError("specify a unit")
super().add_metric(name=name, value=value, unit=unit) Note that this also means that |
Hey @florian42 thanks a lot for the help. We're going through a big feature now (Event Handler route splitting) and I can have a look at this with you later this week. On the positioning, yep, a mistake I've made early was not enforcing kwarg only |
We're closing feature requests older than a year that haven't received enough customers +1 or that we were unable to prioritize. For newer customers interested in, please feel free to comment and we can reopen it. |
|
Is your feature request related to a problem? Please describe.
Approximately 2/3rd of my metrics are just posting singletons to a given metric, where
unit=MetricUnit.Count
andvalue=1
. This results in repeating myself frequently.Describe the solution you'd like
Provide default values for
unit
andvalue
, such that I can.add_metric()
by providing just aname=
value.Describe alternatives you've considered
A wrapper function that provides the same functionality might also be useful, but this seems like overkill compared to providing default values.
Additional context
This is the first time I've opened a Feature Request on a public repo... did I provide enough detail? :-)
The text was updated successfully, but these errors were encountered: