cmd/scollector: Record the current_source as a tag rather than a separate metric. #1782
Conversation
| if fl == "*" { | ||
| current_source = 1 | ||
| tags = tags.Merge(opentsdb.TagSet{"current_source": "1"}) |
There was a problem hiding this comment.
Conditional tags are generally a bad idea. It will trigger unknowns when you alert on metric{foo=*,current_source=*} when things switch, because that tagset will have "disappeared".
So it would be better to always have the tagkey present and have a value of "true" / "false".
Even with that change, I'm not sure we need an additional metric so would like to see what you are trying to do.
If we do go ahead with make it an additional tag, I would add a new metric most likely so the behavior doesn't change if other people are alerting on the metric.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Currently, things like ntp.offset are of limited use by themselves. You can't tell which remote the metric is measured against without cross-referencing against ntp.current_source, which seems a bit silly.
This change will allow us to alert things like ntp.offset properly with much more ease, as we can simply filter it using the tag rather than cross referencing. The ntp.current_source metric is currently entirely unused in our config.
👓 @kylebrandt / @captncraig