Don't zero out Gauges.

[jdmarshall]

This PR started as exploratory work for a different issue - there's been a TODO that I added to metric.js that fights the work needed for #812. I had already decided that doing #812 all in a single PR would probably get a little bit on the long side, so I was looking for some stopping point in the middle to split it up.

I noticed that if #622 were fixed, then a default reset() implementation would be used by 2 of the 4 builtin metric types.

This PR initializes the store in metrics.js, and then retains the LabelMap object across reset() calls, and removes the zeroing logic from Gauge.reset(), thereby fixing #622.

[krajorama]

I think it makes sense to keep the current behavior to be in line with what client_golang and client_java does. It could be the basis of introducing constant labels later. See comment inside.

[jdmarshall]

If I understand the code correctly, this client only has dynamic labels , meaning you cannot set a value for a label at the constructor, you always have to define it when measuring?

I'm trying to change that behavior at the moment (#812, #814), as a side effect of fixing three other issues. Some of the code in this PR is in service of that work, but could be split out.

[krajorama]

Thinking about this more: in general it seems the issue is that there's no separation between constructing a constant label metric versus dynamic label metric like in Go (e.g. NewCounter vs NewCounterVec).

Interestingly Java is in the same state as client_js right now: https://prometheus.github.io/client_java/getting-started/labels/#initializing-label-values . @zeitlinger is it causing issues for Java that metrics without labels are automatically set to 0 and are exposed? Is there plans to change it?

BTW, isn't simply removing the current zeroing out in this PR a breaking change? Someone might rightfully depend on it.

IMHO the absolute cleanest solution would be to introduce NewCounter/NewCounterVec factory functions that cleanly separate the two use cases.

[jdmarshall]

Oh it's definitely a breaking change, which is why #622 is marked as semver-major. But it's been complained about by a few different people. Gauges getting sampled as zeros is no bueno when you're using min/max or average aggregation.

As for the factory function, that's one of the goals of #812, but it's a fair point that I should write that down since it's all in my head at the moment. In fact I might need to make an epic ('project') for this work.

[jdmarshall]

Also, I still don't see any evidence in the Go code of Gauges being initialized to zeros. You gave Counter examples, which absolutely do need to be initialized to 0.

[jdmarshall]

@krajorama This change has no affect on Counter, which is intended to be initialized to zero. This is gauge, where no initial value is correct or can be. We don't know what's being measured. CPU%, req/s, heap usage.

[krajorama]

Also, I still don't see any evidence in the Go code of Gauges being initialized to zeros. You gave Counter examples, which absolutely do need to be initialized to 0.

It's relatively easy to reproduce the Gauge behavior with client_golang, I have a dumb little testbed that I use and tweak all the time. The constant labels version does init to 0. Whereas the dynamic labels one doesn't: https://github.com/krajorama/promtestsource/blob/dynamic-gauge/promtestsource.go#L208 .

From the code it seems like client_java does the same, but I have not tested it directly.

Oh it's definitely a breaking change, which is why #622 is marked as semver-major. But it's been complained about by a few different people. Gauges getting sampled as zeros is no bueno when you're using min/max or average aggregation.

Ok, that's fair.

This is gauge, where no initial value is correct or can be. We don't know what's being measured. CPU%, req/s, heap usage.

That's also fair, interestingly the specification still calls for starting at 0 (and may offer a way to init to non-zero): https://prometheus.io/docs/instrumenting/writing_clientlibs/#gauge. But the spec doesn't say if this has to exposed or not. Which would be very impossible for dynamic labels anyway.

Summary:

• Technically no spec says that you must expose the zero.
• Go and Java clients both expose the zero if there are no const/dynamic labels for the Gauge
• I've now also checked python and that behaves the same when there's no labels specified. However it only supports dynamic labels beyond that, there's no constant labels, so I think it's pretty much the same as this client now.

I think a way forward could be to do this breaking change, but also add a new API that allows for explicitly setting constant labels (even zero) so that you can switch to using that if you're depending on the current behavior. The new API would allow 0..n constant labels (label name+value). What do you think?

[krajorama]

Actually you may not need a new API at all , just document that you need to observe a value to make it appear right away, right?

[jdmarshall]

@krajorama I have captured your research as a requirement of #812 which sounds reasonable to me.

I still think it's wrong but probably consistency matters here. What I'd like to do is converse with the other implementers and ask why we are collectively doing it this way and whether my reasons for suggesting we all stop are more compelling for their reasons for doing it this way. But that would have required me to register for the convention and I hadn't been aware it existed until after the cutoff date.

[krajorama]

Approved as agreed, acknowledging it's a breaking change. Will have to be explained in the release notes.

Added some Claude findings as comments, could be follow up as well.

[krajorama]

claude claims this is no-op, should be something like

expect((await instance.get()).values).toEqual([]);

[krajorama]

claude claims this is no-op, should be something like

expect((await instance.get()).values).toEqual([]);