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

collector collisions #128

Closed acpana closed 3 years ago

acpana commented 3 years ago

Overview

IIUC these lines https://github.com/armon/go-metrics/blob/master/prometheus/prometheus.go#L129-L134 will create a collision for the Collector type if we attempt to register more than one PrometheusSink.

Repro

One repro scenario is described here https://github.com/hashicorp/consul/issues/11273 .

Also, I have a test can repro this and will comment below

Proposed Fixes

acpana commented 3 years ago

Test to repro:


func TestNewPrometheusSink(t *testing.T) {

    gaugeDef := GaugeDefinition{
        Name: []string{"my", "test", "gauge"},
        Help: "A gauge for testing? How helpful!",
    }
    // PrometheusSink config w/ definitions for each metric type
    cfg := PrometheusOpts{
        Expiration:         5 * time.Second,
        GaugeDefinitions:   append([]GaugeDefinition{}, gaugeDef),
        SummaryDefinitions: append([]SummaryDefinition{}),
        CounterDefinitions: append([]CounterDefinition{}),
    }

    sink1, err := NewPrometheusSinkFrom(cfg)

    reg := prometheus.DefaultRegisterer

    if err != nil {
        t.Fatalf("err = %v, want nil", err)
    }

    gaugeDef2 := GaugeDefinition{
        Name: []string{"my2", "test", "gauge"},
        Help: "A gauge for testing? How helpful!",
    }

    cfg2 := PrometheusOpts{
        Expiration:         15 * time.Second,
        GaugeDefinitions:   append([]GaugeDefinition{}, gaugeDef2),
        SummaryDefinitions: append([]SummaryDefinition{}),
        CounterDefinitions: append([]CounterDefinition{}),
    }

    sink2, err := NewPrometheusSinkFrom(cfg2) // errors out
        ...
}