Open myarjunar opened 1 year ago
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 ?
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:
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.
In one of my unit tests, I am comparing the equality of
MetricFamily
objects, which include aCounter
type metric. Prior to the v1.17.0 release, theCreatedTimestamp
field did not include a timestamp value at the time of creation. However, with this recent release, theCreatedTimestamp
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 thenow
property public in the library. This would provide users like me with the ability to mock thenow
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!