Relax necessity of declaring labelset in advance

[dt-rush]

There is no inherent restriction in prometheus for this. Labels can appear or disappear from the timeseries a node exports. This restriction in this JS client appears to be a consequence of choices made in the design of the internal hashmap property used in the 4 metrics classes, the utils.setValue() function, and possibly the utils.getLabels() function. This issue is a placeholder for further investigation.

[zbjornson]

Not opposed to relaxing the requirement, but also note that #299 (comment) made it so Typescript/ts-lint users will get a warning if they try to use a label that wasn't pre-declared.

[dt-rush]

Thanks for pointing that out @zbjornson.

I'm working on a PR and tests for this atm.

For further justification for this change, I cite the robustperception.io blog post on predeclaring metrics:

The first reason relates to what was previously covered in Existential issues with metrics, metrics that only some times exist are difficult to deal with in PromQL. By declaring the metric before your application uses it, the client library can initialise it to 0. This applies even if the Counter is never subsequently incremented. However this doesn't work for metrics with labels: the client library doesn't know what labels might come up, so you have to do it yourself.

(emphasis added)

[dt-rush]

@zbjornson as mentioned in the discussion in #299, that'll be reverted as it adds to the unnecessary rigidity of using this client.

[dt-rush]

That is all sorted nicely, however, there is one more issue to bring up:

In the Writing client libaries # Labels docs, we read:

Client libraries MUST NOT under any circumstances allow users to have different label names for the same metric for Gauge/Counter/Summary/Histogram or any other Collector offered by the library.

The wording of this is not exactly easy to interpret, since its only possible meaning seems a contradiction with what is said (and observed in practice) elsewhere, that the appearance of a new labelset will create a new timeseries (that is, there's no problem having different label names for the same metric). There is no problem even in the current system of pre-declaring a labelset, {a, b, c, d} and then at various times exporting metrics labelled with subsets: {a, b}, {c, d},{c}, {b, d}, etc... that is, "different label names for the same metric".

This is brought up before in this SO question where the user similarly brings up this contradiction.

There is really no difference, if exporting

some_counter {f="1"} 1 
some_counter {f="1", g="1"} 1 
some_counter {g="1"} 1 
some_counter {g="1", h="1"} 1 
some_counter {f="1", h="1"} 1 

between

new Counter({name: "some_counter", labelNames: ["f", "g", "h"]})

and

new Counter({name: "some_counter"});

from the standpoint of the exported metrics, in cases like these, if we simply allow labels to appear whenever they occur.

Implicitly, the "MUST NOT" amounts to a statement (can't be correct, can it?) that the only metric permitted is one which uses its full labelspace all of the time. But that's nonsense, as shown above.

[dt-rush]

Branch pushed here: https://github.com/dt-rush/prom-client/tree/relax-label-predeclaration

[dt-rush]

Note: that branch can be merged, but it still won't work for typescript clients unless #299 https://github.com/siimon/prom-client/pull/299/files is reverted

[dt-rush]

@siimon I wonder if you'd be able to give any insight here, it seems this can go ahead, but not sure

[dt-rush]

I see other client libraries also (unnecessarily?) restricting label names in this way, and yet as explained above, this doesn't seem to be at odds with how exporters already work even in the case of predeclared labels. There's literally no difference to the prometheus process running the scrape; say we declared labelNames {a, b, c, ... , y, z} but only exported a through m for a year, then suddenly we export a metric with a through r. The "added" labels were "part of the labelset" from the very beginning, but they "just now appeared" from the perspective of the scrape.

[dt-rush]

bump @zbjornson, perhaps you can weigh in on this?

[zbjornson]

Sorry for the delay. Hoping to have time to look this weekend.

[siimon]

Sorry for being late on the ball here, thanks for your suggestion and input. Me personally would love to drop this too since it just adds complexity.

My main concern though is to keep it aligned to other prometheus clients in other languages, specifically the offically supported ones. Have they dropped this requirement too? If not then I probably would wait until they have