Allow creating constant histogram and summary metrics with a created timestamp

[swar8080]

Closes #1535

Describe your PR

Currently there's no way to set created timestamp when creating constant histogram and summar metrics. This adds some new functions using the same naming as the existing NewConstMetricWithCreatedTimestamp

What type of PR is this?

/kind enhancement

Changelog Entry

api: Allow setting created timestamp on constant histogram and summary metrics using the new functions `NewConstHistogramWithCreatedTimestamp` and `NewConstSumamryWithCreatedTimestamp`

Testing

Verified locally that zero-injection happens for the _bucket, _sum, and _count series. An example:

[ArthurSens]

Just wanted to let you know that this is on my list! Hopefully I'll get to it by the end of the week

[ArthurSens]

Ok, I've spent more time reading our codebase today and I think I've found some inconsistencies.

I noticed that ConstHistogram doesn't add exemplars to the histogram, but I knew that somehow it was possible. After reading a bit I've found NewMetricWithExemplars, that adds exemplars to existing metrics and it doesn't matter if it's a Counter or Histograms.

Now for CreatedTimestamps, we took a different turn 😬. Instead of creating a similar function, we created NewConstMetricWithCreatedTimestamp, that only works with Counters.

I'm not sure what's the best solution here, but I'm more inclined to deprecating NewConstMetricWithCreatedTimestamp and following the same strategy we had with the exemplars 🤔

What do you think @swar8080 @bwplotka @kakkoyun ?

[swar8080]

Hey @ArthurSens, thanks for looking at this

As a user I remember expecting NewConstMetricWithCreatedTimestamp to work with constant histograms and then had to dig into the code to figure out why it didn't

For non-const/stateful metrics, like regular counters, it looks like created timestamp is managed internally and not provided by the caller. So not sure if NewMetricWithCreatedTimestamp would be needed for those

[bwplotka]

Silly question, but can we make NewConstMetricWithCreatedTimestamp work with const histograms without breaking change?

Otherwise it might be time for different v2 API here with some builder pattern or something for those side methods, it's getting confusing (:

Happy with whatever is easier for downstream users here.

[ArthurSens]

Silly question, but can we make NewConstMetricWithCreatedTimestamp work with const histograms without breaking change?

I don't think we can, because histograms need different information, i.e. count, sum and bucket boundaries. NewConstMetric would need them as extra arguments and that's a breaking change

Otherwise it might be time for different v2 API here with some builder pattern or something for those side methods, it's getting confusing (:

Happy with whatever is easier for downstream users here.

Yeah, let's iterate on this later. Although we don't have concrete plans for v2, I think we have enough to start experimenting

[ArthurSens]

Alright, I'm happy to continue with NewConstHistogramWithCreatedTimestamp, we can experiment with a cleaner API in the v2 package.

Could you also add a small test similar to this one, but for histograms?

Would you like to add the same thing for summaries as well? (Can be done in another PR if you prefer)

[swar8080]

@ArthurSens I added the extra histogram test and the same change for summaries

[ArthurSens]

LGTM, thanks!

Just two small nits that I'll add in a separate commit

[ArthurSens]

Also, I took the liberty to change you PR description to /kind enhancement, since we're not changing any existing behavior :)