dskit icon indicating copy to clipboard operation
dskit copied to clipboard

Creating multiple KV client within the same process panic

Open cyriltovena opened this issue 3 years ago • 2 comments

Because the metrics registration is done inside each client and there's no way to pass your own metric struct.

Even though it's not known in advance what would be the metric to create since the type of client is decided later during NewClient call.

This seems like a big refactoring, for now I'm discarding all metrics for my client, but I'd like to gather idea on how to solve this.

Example of panic:

	/Users/ctovena/go/src/github.com/grafana/dskit/vendor/github.com/prometheus/client_golang/prometheus/promauto/auto.go:362 +0xf5
github.com/grafana/dskit/kv.newMetricsClient({0x1999d9c, 0x8}, {0x1ac3030, 0xc0002b61b0}, {0x1ab1d20, 0xc0002fc000})
	/Users/ctovena/go/src/github.com/grafana/dskit/kv/metrics.go:51 +0x1d5
github.com/grafana/dskit/kv.createClient({_, _}, {_, _}, {{{0x0, 0x0}, {0x0, 0x0}, 0x0, 0x0, ...}, ...}, ...)
	/Users/ctovena/go/src/github.com/grafana/dskit/kv/client.go:188 +0x6f9
github.com/grafana/dskit/kv.NewClient({{0x1999d9c, 0x8}, {0x0, 0x0}, {{{0x0, 0x0}, {0x0, 0x0}, 0x0, 0x0, ...}, ...}, ...}, ...)
	/Users/ctovena/go/src/github.com/grafana/dskit/kv/client.go:131 +0x13c

But obviously this would happen for all metrics.

cyriltovena avatar Feb 07 '22 16:02 cyriltovena

I've encountered a similar bug when running Tempo in single binary mode (= all components run in a single process). The metrics-generator (a new component) and the compactor both use the BasicLifecycler. Calls causing trouble: ring.NewBasicLifecycler and ring.NewWithStoreClientAndStrategy. Stacktrace: https://github.com/grafana/tempo/issues/1303#issuecomment-1060566677

I'm not sure yet how to avoid this, making the metrics struct a parameter of NewBasicLifecycler would not really improve this as both calls are very spread out over the codebase.

yvrhdn avatar Mar 07 '22 11:03 yvrhdn

Have you tried wrapping the registerer with prometheus.WrapRegistererWith() (doc)? The idea is that you can wrap the registerer adding a constant label to differentiate the component. In other projects we use the component label. For example, we wrap it with prometheus.WrapRegistererWith(prometheus.Labels{"component": "store-gateway"}, reg).

Does it solve your issue?

pracucci avatar Mar 14 '22 16:03 pracucci