client_golang: updating client_model dependency to 0.4.0

[Daniel Swarbrick]

Hi folks,

Recently whilst updating a bunch of Prometheus packages in Debian I stumbled across the fact that a whole heap of tests in client_golang fail if it is built against client_model 0.4.0. I have already discussed this a little with Ben Kochie after noticing that he had started to refactor some of the tests.

In a nutshell, virtually all of the metric / protobuf related tests in client_golang are testing against text-marshaled golden tests, which is no longer viable with the protobuf APIv2 used by client_model 0.4.0. Not only has the text marshal format (subtly) changed, but it is also now non-deterministic, seemingly to discourage this style of test. The text marshal format uses variable whitespace, which is immaterial when unmarshaling, but will obviously break brittle tests that compare against a golden text.

My initial approach to solving this was to write a generic function to unmarshal the "want" metric, and compare this to the "got" metric with proto.Equal(). The use of reflect.DeepEqual() is not advised, but on that same page, a useful example of using cmp.Diff() is shown. IMHO, this is perhaps the most convenient way to refactor these tests, not least because the new protobuf library includes a whole package of helpers for use with the cmp package.

But now we get to the ugly stuff. There are several testable examples in the client_golang package, which compare the example code's text marshaled metrics output to the golden "// Output:" text. As you've probably guessed, these now all fail. Even if the golden text were to be updated to the new text marshal format, they would still fail intermittently, since the format is non-deterministic. Some of these tests can be fixed by ensuring that the example outputs "compacted" text, using a function such as the compact() function in the legacy protobuf library. Other tests are trickier, because they are comparing entire HTTP response bodies, albeit with a protobuf error message part way through (which are also no longer deterministic).

I've done some initial work in https://github.com/dswarbrick/client_golang/tree/client_model-0.4.0, but would appreciate if anybody has some bright ideas how to handle the testable examples.

Best,
Daniel

[Bjoern Rabenstein]

On 17.07.23 16:47, Daniel Swarbrick wrote:
>
> But now we get to the ugly stuff. There are several testable examples in
> the client_golang package, which compare the example code's text marshaled
> metrics output to the golden "// Output:" text. As you've probably guessed,
> these now all fail. Even if the golden text were to be updated to the new

> text marshal format, they would still fail *intermittently*, since the

> format is non-deterministic. Some of these tests can be fixed by ensuring
> that the example outputs "compacted" text, using a function such as the
> compact()

> <https://github.com/golang/protobuf/blob/master/proto/text_test.go>

> function in the legacy protobuf library. Other tests are trickier, because
> they are comparing entire HTTP response bodies, albeit with a protobuf

> error message part way through (which are *also* no longer deterministic).

>
> I've done some initial work in
> https://github.com/dswarbrick/client_golang/tree/client_model-0.4.0, but
> would appreciate if anybody has some bright ideas how to handle the
> testable examples.

Hi Daniel, I think your analysis is spot on. For classical text, we
can do stuff do compare in different ways, but for the Go example
tests, we need text output, so we have to do something pretty
involved. (The current way of writing those example tests were really
driven by pragmatism – use the easiest way to got a more or less
readable and deterministic output, and the latter doesn't work
anymore.) Unfortunately, I couldn't come up with any bright ideas for
an easy way out.

Thanks for tackling this.
--
Björn Rabenstein
[PGP-ID] 0x851C3DA17D748D03
[email] bjo...@rabenste.in