Add support for tags in metrics about go-metrics HOT 23 OPEN

Because I needed it for one of our projects I added preliminary (and limited) support for this on our fork: alfatraining#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.

from go-metrics.

👍

from go-metrics.

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

from go-metrics.

And for datadog

from go-metrics.

I agree, it's very important feature

from go-metrics.

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.

from go-metrics.

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()

from go-metrics.

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.

from go-metrics.

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.

from go-metrics.

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.

from go-metrics.

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

from go-metrics.

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?

from go-metrics.

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 [email protected]
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
#135 (comment)
.

from go-metrics.

And the tags should be immutable.

@ansel1 Could you explain this a little bit deeper?

from go-metrics.

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 [email protected] 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
#135 (comment)
.

from go-metrics.

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.

from go-metrics.

@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.

from go-metrics.

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.

from go-metrics.

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.

from go-metrics.

from go-metrics.

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).

from go-metrics.

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

from go-metrics.

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

from go-metrics.