hashicorp / go-metrics

A Golang library for exporting performance and runtime metrics to external metrics systems (i.e. statsite, statsd)
MIT License
1.46k stars 177 forks source link

FanoutSink key modified across sinks when DogStatsdSink is enabled #154

Closed Achooo closed 1 year ago

Achooo commented 1 year ago

Problem

  1. FanoutSink does not copy the key before passing it to different sink.
  2. DogStatsdSink overrides the key slice in Line 73 when the hostname is the same as a key value:

https://github.com/hashicorp/go-metrics/blob/858601465fcd6976fa4b951419becea272d25579/datadog/dogstatsd.go#L63-L77

Reproduction Steps

  1. Create a FanoutSink
  2. Add a DogStatsdSink with a hostname
  3. Add another Sink (e.g. InmemSink)
  4. Call SetGaugeWithLabels() with the hostname as a value in the keys

Example of a test I added in datadog/dogstatsd_test.go :

func TestFanoutSink(t *testing.T) {
    server, _ := setupTestServerAndBuffer(t)
    defer server.Close()

    dog, err := NewDogStatsdSink(DogStatsdAddr, "consul")
    require.NoError(t, err)

    inmem := metrics.NewInmemSink(10*time.Second, 10*time.Second)

    fSink := metrics.FanoutSink{dog, inmem}
    fSink.SetGaugeWithLabels([]string{"consul", "metric"}, 10, []metrics.Label{{Name: "a", Value: "b"}})

    intervals := inmem.Data()
    require.Len(t, intervals, 1)

    if intervals[0].Gauges["consul.metric;a=b"].Value != 10 {
        t.Fatalf("bad val: %v", intervals[0].Gauges)
    }
}

Output

--- FAIL: TestFanoutSink (0.00s)
    /<>/HashiCorp/go-metrics/datadog/dogstatsd_test.go:164: bad val: map[metric.metric;a=b:{metric.metric  10 [{a b}] map[]}]
FAIL
FAIL    github.com/hashicorp/go-metrics/datadog 0.694s
FAIL

Expected Output

I would expect the key in the inmemsink to remain consul.metric but it is modified to metric.metric

Fix

The FanoutSink should ideally copy the key before passing it to sinks to avoid modifications