metrics
metrics copied to clipboard
VictoriaMetrics
Helper function to build metric's name
Hi, thank you for the lib it's great.
I have a proposition to add a helper function that creates a metric's name by it's name and labels. Current API for Counter(applies to other types too) expects a final name, like foo{bar="baz",aaa="b"}.
Building such names is not that hard but it's repetitive work without sense (had to copy buildName helper 4 or so times between projects).
Proposal: add func BuildName(name string, labels ...string) string that creates a valid Prometheus compatible name. Especially there is no error result, we can just panic if the labels variable isn't even (key-value, huh). However, it may return an error, not so hard too.
Happy to make a PR for that. Thanks.
I'm unsure this helper function is better than fmt.Sprintf() from readability and maintainability points of view. IMHO, fmt.Sprintf("metric_name{label1=%q, ... , labelN=%q}", labelValue1, ..., labelValueN) is easier to read, understand and maintain than metrics.BuildName("metric_name", "label1", labelValue1, ..., "labelN", labelValueN) .
Could you provide a use case, where the metrics.BuildName() is better to use than fmst.Sprintf()?
Sprintf pattern is less discoverable and might be error-prone for the new users, however works in the same way.
Have 0 will to argue further, so closing.
You know what, I've a case where helper/builder func works better: codegen :)
Instead of combining own helper package for 1 consumer or hacking codegen template, it can be as easy as metrics.BuildName(name, labels...).
What do you think?
Yes, as many people have own build name func. I'm prefer to have sorted labels in metrics
Indeed, scanning through the code, I've not found labels sorting, looks like foo{bar=val,baz=ue} isn't the same as foo{baz=ue,bar=val} 👀
Yes, so i'm write own func like this https://github.com/unistack-org/micro-meter-victoriametrics/blob/master/victoriametrics.go#L25 label sorting like this https://github.com/unistack-org/micro/blob/v3.4.9/meter/meter.go#L80
I like usage like metricVec.WithLabelValues("aa","bb").Observe(). In case I have dynamic set of labels (let's say handler name)
How I supposed to handle this in VMmetrics?
I like usage like metricVec.WithLabelValues("aa","bb").Observe(). In case I have dynamic set of labels (let's say handler name)
How I supposed to handle this in VMmetrics?
write own func that get or create needed metric with labels =)
Indeed, scanning through the code, I've not found labels sorting, looks like foo{bar=val,baz=ue} isn't the same as foo{baz=ue,bar=val}
Yes, this looks like a low-priority issue, which may be addressed in the future for all the fuctions accepting fully qualified metric name with labels. There is no need in adding BuildName function for resolving this issue.
I'd propose leaving the BuildName function outside the github.com/VictoriaMetrics/metrics package, since:
- it has higher cognitive complexity compared to fmt.Sprintf due to an extra abstraction level
- it has no clear advantages over fmt.Sprintf (except of a dedicated function name - then it is better making an alias of fmt.Sprintf with BuildName name :) ).
- its functionality is specific to a particular use case. For example, in one case it should sort the provided labels, while in other case the labels' sorting isn't needed.
Yes, this looks like a low-priority issue, which may be addressed in the future for all the fuctions accepting fully qualified metric name with labels. There is no need in adding BuildName function for resolving this issue.
I'd propose leaving the BuildName function outside the github.com/VictoriaMetrics/metrics package, since:
- it has higher cognitive complexity compared to fmt.Sprintf due to an extra abstraction level
- it has no clear advantages over fmt.Sprintf (except of a dedicated function name - then it is better making an alias of fmt.Sprintf with BuildName name :) ).
- its functionality is specific to a particular use case. For example, in one case it should sort the provided labels, while in other case the labels' sorting isn't needed.
Having to write the helper to build the label values is currently one of the reasons why we did not start to migrate to metrics in all projects. Probably 90% of our metrics are using label values, which would all need to build the metrics name, so a helper is needed in every project. Not a big hassle, but enough to hold us back it seems.
But if there is need to use a builder for nearly every project, it could be a useful addition to the metrics package to reduce overhead. So from a usability point of view for the developer I think it would be a welcome addition and could help adoption.
Definitely a low prio issue of course.
