Validate low cardinality keys

[jonatan-ivanov]

Observations with the same name should have the same set of low cardinality keys.
Example 1:

observation{name=test, lowCardinalityKeyValues=[color=red]}
observation{name=test, lowCardinalityKeyValues=[color=green]}
observation{name=test, lowCardinalityKeyValues=[color=blue]}

Example 1 is valid since all the observations with the same name ("test") has the same set of low cardinality keys ("color").

Example 2:

observation{name=test, lowCardinalityKeyValues=[color=red]}
observation{name=test, lowCardinalityKeyValues=[]}
observation{name=test, lowCardinalityKeyValues=[status=ok]}

Example 2 is invalid since the second observation is missing a key the first one has ("color") and the third observation is not only missing a key the first one has ("color") but it also has an extra key the first one does not have ("status").

If the validator detects this, it will give you an error like this:

Metrics backends may require that all observations with the same name have the same set of low cardinality keys.
There is already an existing observation named 'test' containing keys [color].
The observation you are attempting to register has keys [status].

In order to fix it, you should always add the same set of keys to the Observation. If some data is missing, you can signal it with a "none"/"unknown"/etc. value:

observation{name=test, lowCardinalityKeyValues=[color=red, status=unknown]}
observation{name=test, lowCardinalityKeyValues=[color=unknown, status=unknown]}
observation{name=test, lowCardinalityKeyValues=[color=unknown, status=ok]}

In your codebase, this means if you attach keyvalues conditionally:

if (color != null) {
    observation.lowCardinalityKeyValue("color", color);
}

you should only apply the condition to the value only:

observation.lowCardinalityKeyValue("color", color != null ? color : "unknown");

If you want to turn this validation off (not recommended), you can do it this way:

TestObservationRegistry registry = TestObservationRegistry.builder()
        .validateObservationsWithTheSameNameHavingTheSameSetOfLowCardinalityKeys(false)
        .build();

[shakuzen]

I'm not sure I'd validate this by default. Observations are multiple layers abstracted away from Prometheus, which is the only system I'm aware of that has this limitation, and even then, the limitation is only in the Prometheus Java client. I guess we're trying to help people writing instrumentation where they don't know what ObservationHandler might be in use at runtime. Logging a warning probably isn't very helpful when running tests because it's likely to go unnoticed. Given that, I'm not sure what alternative there is. Maybe this is the best we can do.

[jonatan-ivanov]

I added a component to be able to turn features on/off.
Right now I disabled this validation by default but now that I just pushed the changes I think maybe it should be enabled by default and if users have issues they can manually disable it by passing an empty array to the create method:

TestObservationRegistry.create(new TestObservationRegistry.Capability[]{});

or maybe adding an EMPTY (or ALL_OFF?) capability or array constant so that:

TestObservationRegistry.create(EMPTY);

[shakuzen]

I'd be fine with it being checked by default if there's a straightforward way to disable it. These kinds of configurations options tend to get complicated easily, though.

[jonatan-ivanov]

I enabled by default and added a constant to disable it easily:

TestObservationRegistry.create(ALL_CAPABILITIES_DISABLED);

I guess another alternative to make these things configurable is boolean flags with setters or a builder which I think they can get complicated the same way as the enums, not sure what better options we have.

[shakuzen]

A builder is most future-proof, I think, but we can try with this for now and switch to a builder later if it makes more sense.

[jonatan-ivanov]

Let's use a builder from the beginning since it can prevent making the enum public.
I still kept the enum as an implementation detail to prevent using a bunch of booleans in the future. By default the feature is still enabled but using a builder you can disable it, see the tests.

[shakuzen]

LGTM