Is there any specific reason to not expose now property in the metric options struct?

[myarjunar]

In one of my unit tests, I am comparing the equality of MetricFamily objects, which include a Counter type metric. Prior to the v1.17.0 release, the CreatedTimestamp field did not include a timestamp value at the time of creation. However, with this recent release, the CreatedTimestamp is automatically populated with the current timestamp at the time of metric creation.

I also noticed that the metric options include a now property. Based on the description in the code, it appears that this property is used for testing purposes internally within the package. I would like to suggest making the now property public in the library. This would provide users like me with the ability to mock the now function, enabling us to conduct comprehensive testing externally.

Would love to hear your opinion on this or if there is indeed a specific reason not to do it that way. Thanks in advance!

[ArthurSens]

Hmmmm, good point... when implementing this though, we were afraid that people would override the now() function with some super weird function that would eventually trigger weird behaviors in Prometheus. It was basically to avoid users to shooting their own foots.

I understand that it makes testing a lot harder now. WDYT @bwplotka @kakkoyun ?

[bwplotka]

Thanks! Let's find together a good solution, sorry for breaking your tests.

I am comparing the equality of MetricFamily objects, which include a Counter type metric

How you compare those exactly @myarjunar?

I would love to avoid making test-focused fields public in important APIs and add full compatibility guarantee on those. I think there are ways to avoid it:

1. We could look on your tests. We are adding this field only to protobuf exposition format and protobuf dto. Protobuf is forward & backward compatible so comparing protobuf wouldn't break your test. But users (even client_golang 2w ago or so) was (wrongly in many cases) comparing protobuf string (text) serialized output with expected strings. This is essentially problem explained in encoding: provide canonical output format golang/protobuf#1121 and we updated client_golang to resolve it in Bump client_model #1323 (by comparing proto objects, NOT text format).
2. If above is unavoidable we could move now from Opts to some global var (sketchy)
3. ...or add helpers that "sanitize" the strings for test purposes like we have in https://github.com/prometheus/client_golang/blob/main/prometheus/utils_test.go#L43

[AlexanderYastrebov]

Hello, we've also stumbled upon this.
In our case we'd like to add start timestamp label to the counters and we are testing by verifying HTTP text response from prometheus handler. Ideally we'd like to mock now function to return fixed value to assert against in test.

when implementing this though, we were afraid that people would override the now() function with some super weird function that would eventually trigger weird behaviors in Prometheus. It was basically to avoid users to shooting their own foots.
I would love to avoid making test-focused fields public in important APIs and add full compatibility guarantee on those.

I think Opts.Now could be exported with a clear description what it does.