Add support for tags in metrics

[gsalgado]

InfluxDB 0.9 has support for tags, which are indexed key/value pairs that can be used for fast and efficient queries (https://influxdb.com/docs/v0.9/introduction/overview.html). Also, without tags, you're very limited on the kind of queries you can perform on your data, so it's important to have support for that in go-metrics.

[aaronkavlie-wf]

:+1:

[vgheri]

I agree, tags would be a very useful addition for influx db!

[ansel1]

And for datadog

[trong]

I agree, it's very important feature

[mihasya]

Interest in this feature acknowledged.

Any volunteers to try and design an API and put forth a strawman patch? Requirements that I can think off off the top of my head are:

• Backwards compatible so that people that don't want tags can continue to ignore them (table stakes for this project)
• A consistent interface through which various reporters can access a metric's tags so that various clients (influxdb, graphite, librato, etc) can easily grab them.

[trong]

As I see it:

type Registry interface {
    ...
    AddTags(tags ...string) Registry
    Tags() []string
}

type Timer interface {
    ...
    AddTags(tags ...string) Timer
    Tags() []string
}

// add tags globally for Registry
r := metrics.NewRegistry().AddTags("role:service", "dc:dc1", "host:host1")

// add tags for each metric
d := metrics.NewTimer().AddTags("db:mongodb", "collection:users")
c := metrics.NewTimer().AddTags("cache:redis", "db:6")

// register in tagged Registry
r.Register("db", d)
r.Register("cache", c)

// r.Tags() - returns "role:service", "dc:dc1", "host:host1"
// d.Tags() - returns "db:mongodb", "collection:users"

It would be useful to get a complete list of tags directly from the metric, like this:

// returns "role:service", "dc:dc1", "host:host1", "db:mongodb", "collection:users"
d.Tags(true) // inherited = true
d.InheritedTags()

[ansel1]

Aren't the tags part of the metric's key in the registry? metric "db host:host1" is a different metric from "db host:host2", right? So I'm thinking all the registry functions to get and put a metric, and all the convenience GetOrCreateXXX functions need to take the tags. And the tags should be immutable.

[trong]

Aren't the tags part of the metric's key in the registry?

Not only. There are global metrics like host or service name - for this kind of metrics I offered to add "fluent" method AddTags in Registry.

But there are tags which actual only for some metrics - tag of SQL-query, for example.

[ansel1]

The registry-level AddTags() makes sense to me. But I don't think the metric-level AddTags() makes sense.

We worked around the tags limitation by encoding per-metric tags into the metric name. We encode the metric name like a url, and append tags as query params. Our reporter then decodes the tags from the name. This effectively makes tags part of the name of the metric. In other words, every unique tuple of [name,tag=value,...] is a unique metric in the registry.

[trong]

We worked around the tags limitation by encoding per-metric tags into the metric name. We encode the metric name like a url, and append tags as query params

Yes, it's possible as work-around.

A consistent interface through which various reporters can access a metric's tags so that various clients (influxdb, graphite, librato, etc) can easily grab them.

Parse name of metrics - by my opinion it's not so easily :)

As point to discuss:

• example from InfluxDB documentation:

  cpu_load,host=server01,core=0 value=0.45 1437171724
  cpu_load,host=server01,core=1 value=1.56 1437171724
  cpu_load,host=server02,core=0 value=0.72 1437171724
  cpu_load,host=server02,core=1 value=2.14 1437171724

  cpu_load - name host - registry-level tag core - metric-level tag

• example from Datadog documentation:

  'algorithm.run_time', :tags => ['algorithm:one']
  'algorithm.run_time', :tags => ['algorithm:two']

  Not exactly, but: algorithm - name run_time - registry-level tag [one, two] - metric-level tag

  And as we can see in documentation: https://godoc.org/github.com/DataDog/datadog-go/statsd#Client

  There are client tags:

  type Client struct {
  
  // Namespace to prepend to all statsd calls
  Namespace string
  // Tags are global tags to be added to every statsd call
  Tags []string
  
  sync.Mutex
  // contains filtered or unexported fields
  } 

  And metrics tags: https://godoc.org/github.com/DataDog/datadog-go/statsd#Client.Gauge

  func (c *Client) Gauge(name string, value float64, tags []string, rate float64) error

[JanBerktold]

I am in favour of both registry and metric-level tags. For example, someone might want to report the processing times for a HTTP response and use the return code as a tag for finer details.

We worked around the tags limitation by encoding per-metric tags into the metric name. We encode the metric name like a url, and append tags as query params.

Yeah, but this is only a work-around and why not use the databases's tag feature if they provide one?

[ansel1]

I agree, it would be good to bake it in. I was only pointing out that metric tags need to be part of the immutable name/key of the metric in the registry. Which in turn implies tags need to be added to the registration methods (GetXXX and GetOrRegisterXXX), and can't be added to metrics after registration. On Thu, Dec 24, 2015 at 11:24 AM Jan Berktold notifications@github.com wrote:

I am in favour of both registry and metric-level tags. For example, someone might want to report the processing times for a HTTP response and use the return code as a tag for finer details.

We worked around the tags limitation by encoding per-metric tags into the metric name. We encode the metric name like a url, and append tags as query params.

Yeah, but this is only a work-around and why not use the databases's tag feature if they provide one?

— Reply to this email directly or view it on GitHub https://github.com/rcrowley/go-metrics/issues/135#issuecomment-167086464 .

[trong]

And the tags should be immutable.

@ansel1 Could you explain this a little bit deeper?

[ansel1]

The metric registry is essentially a map. Keys are metric names, values are metric. You can't change the name of a metric after its registered, because you can't change the key that the metric is stored at in the map.

Since tags are effectively parts of the metric's name, the same restrictions apply. The tags are part of the key, so you can't change them once they are in the map.

Changing the tags on an existing metric therefore isn't a valid operation. You'd have to create a new metric. On Sun, Jan 10, 2016 at 1:00 PM trong notifications@github.com wrote:

And the tags should be immutable. @ansel1 https://github.com/ansel1 Could you explain this a little bit deeper?

— Reply to this email directly or view it on GitHub https://github.com/rcrowley/go-metrics/issues/135#issuecomment-170374720 .

[captncraig]

I have encoded metric names such as requests{host=web01,endpoint=foo}. I have a custom influx sender that splits the name and tags and sends appropriate values. A graphite or datadog plugin would need to do slightly different, but similar things.

[lockwobr]

@captncraig like this simple idea, seems that this feature idea is dead, do you have the influx sending code somewhere I can use it? otherwise I might build something do this, can share it back some how. Really if someone would add a flag, saying "using tags" then this lib could take that formatting into place and treat the registry as if it had tags. Or Add a helper to getOrRegister that took not just name but also tags, and it can just format it into the metric name. Seems like a simple way to add it, maybe not the cleanest way to get it done in the long run.

[annismckenzie]

Because I needed it for one of our projects I added preliminary (and limited) support for this on our fork: alfatraining/go-metrics#1. It's just on the counters for now and it's fully backwards-compatible with upstream. If there's a desire by the community to have this I would finish it for the other metric types as well.

[lockwobr]

I need timers and meters support for tags, looking at your code might not be a lot of work, but need this asap. So I might fork you to get this add.

[annismckenzie]

Sure thing, we're still on the fence whether it's worth it to continue or just switch to Prometheus where this is waaaaaayyyyy easier and built-in. Timers and meters is one thing but I actually really really want this for histograms and that's just not possible. Sigh.

[lockwobr]

Interesting I have not looked at Prometheus much maybe I should look at that more today. Would you still us go-metrics to get the data there or just complete different system.

On Wed, May 17, 2017 at 9:15 AM, Daniel Lohse notifications@github.com wrote:

Sure thing, we're still on the fence whether it's worth it to continue or just switch to Prometheus where this is waaaaaayyyyy easier and built-in. Timers and meters is one thing but I actually really really want this for histograms and that's just not possible. Sigh.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rcrowley/go-metrics/issues/135#issuecomment-302141797, or mute the thread https://github.com/notifications/unsubscribe-auth/ABen_kCXtWoU-2K44oT83WObhzkwKFa1ks5r6x03gaJpZM4F2bTr .

-- Brian Lockwood

[annismckenzie]

Judging by https://github.com/go-kit/kit/blob/master/metrics/prometheus/prometheus.go they have their own client so no, I wouldn't use go-metrics anymore (sorry for saying so in this issue and repository).

[f0ster]

I have a fork with this functionality for influx by stuffing the tag data into the name field as json, then unwrapping it before going to influx, https://github.com/f0ster/go-metrics-influxdb

[axpira]

This is a great idea for support metrics go-metrics-wavefront