Update docs to include multiple tag support #4797
Conversation
|
It wasn't mentioned in the original issue (#1781), but did negation make it in to this addition? If so, what's the syntax for negation. |
|
@nugend This change specifically addressed the multiple tags so it didn't add support for negation. Could you please file a separate issue for it if you're needing it? |
This addresses the potential proliferation of metrics if a query of "?tag=foo&tag=bar" is treated differently than "?tag=bar&tag=foo". Now, tags are always sorted before being recorded, making these two emit the same metric.
|
@banks Ready for another review when you get a chance. |
|
Note: After a little more investigation, it looks like the metrics endpoint (/v1/agent/metrics) will display incomplete information about metrics with multiple tags (see this issue), even though the metrics are outputted correctly and completely for external consumers. I've added a note to this effect in the docs. |
|
@adilyse commented on your go-metrics issue but I'm pretty sure that at least some of the popular stats sinks will not support multiple labels/tags with same key. I suggest we either:
2 makes most sense to me now but curious if you can think of any reason that's a bad plan? While I think this is fine now, I was interested to see this warning while trying to find in the Prometheus docs whether they support multiple lables with same key: https://prometheus.io/docs/practices/naming/#labels
Emphasis mine. Technically tags is an unbounded value as is service name which is why they make me itchy but in practice these are shouldn't be used as unbounded in practice e.g. registering with random tags or unique tags for every new instance etc. So I still think it's OK - operators can always filter metrics that are not useful/too expensive for them. |
|
After talking it through with @pearkes, I think we should leave it as-is for now. It provides additional value to the metrics systems that support multiple labels with the same key and is easily excluded for anyone who starts having trouble with it. |
* Update docs to include multiple tag support * Sort tags before using them in metrics This addresses the potential proliferation of metrics if a query of "?tag=foo&tag=bar" is treated differently than "?tag=bar&tag=foo". Now, tags are always sorted before being recorded, making these two emit the same metric. * Add caveat about multiple tags returned by the metrics endpoint
This adds notes in the documentation about the support of multiple tags for service and health queries in the api, as well as for the additional telemetry created using this feature (#1781).
See the (now merged) PR for the related implementation.