Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FanoutSink key modified across sinks when DogStatsdSink is enabled #154

Closed
Achooo opened this issue Jun 6, 2023 · 0 comments · Fixed by #156
Closed

FanoutSink key modified across sinks when DogStatsdSink is enabled #154

Achooo opened this issue Jun 6, 2023 · 0 comments · Fixed by #156

Comments

@Achooo
Copy link

Achooo commented Jun 6, 2023

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:

func (s *DogStatsdSink) parseKey(key []string) ([]string, []metrics.Label) {
// Since DogStatsd supports dimensionality via tags on metric keys, this sink's approach is to splice the hostname out of the key in favor of a `host` tag
// The `host` tag is either forced here, or set downstream by the DogStatsd server
var labels []metrics.Label
hostName := s.hostName
// Splice the hostname out of the key
for i, el := range key {
if el == hostName {
key = append(key[:i], key[i+1:]...)
break
}
}

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant