Thanks @gotjosh! PR looks very promising. One question in the comments above, one suggestion (don't do a code move) and one question here:

How much metadata do we have on average per series in our Prometheus servers at Grafana Labs? And how much bandwidth will this use to send? We need to be able to give guidelines here.

Also to discuss:

• config to enable / disable this, and whether this should be enabled by default (I think yes, but only if bandwidth is low enough)
• config to specify the ratelimit / how often the data is sent.

Link to design doc for those that missed this:

I believe this is the first time this document has ever been shared. We should probably open it up for review (and enable comments) before proceeding with implementation.

@tomwilkie re the questions of:

How much metadata do we have on average per series in our Prometheus servers at Grafana Labs?

Apologies beforehand as I'm not sure I follow a part of that question. Within Prometheus, metadata is not kept per series but rather per metric name in the scrape cache (and also we keep a scrape cache per target).

That being said, currently, at Grafana Labs we have about ~7.67mb (and a total of ~139k entries) [1] metadata across all targets. However, that number is mildly deceiving as we only have ~1800 unique entries across those caches [2].

And how much bandwidth will this use to send?

With the current implementation, we're keeping an in-memory buffer that is a set of metadata entries. Only appending to the set if we've not seen that metadata (a combination of help/type/name/unit) before. From there, we only ever send data based on two criteria: a) We've reached the maximum number of entries b) We're at the ticked deadline.

Currently, the maximum number of metadata unique entries (similarly to remote-write samples) to flush is 2500. At an average of 55 bytes per unique entry [1] albeit more realistically between 60 - 90 bytes -- we're sending between 150kb - 225kb w/o compression every few minutes as long as we don't tip over that maximum threshold.

On scenarios where the number of unique metadata > than the maximum, it is not perfect, as metadata will be sent on a FIFO approach. There's a chance some metadata will never get sent but this can be overcomed by removing the limitation or allowing the user to tune the maximum value.

Does that make sense? Have I missed something?

[1]

[2]

I believe this is the first time this document has ever been shared. We should probably open it up for review (and enable comments) before proceeding with implementation.

@brian-brazil I don't think the document is highly relevant at this point as it started a while ago. It is not exclusively for this work but also the work already done as part of #6395.

Could we have the discussion as part of this PR?

Could we have the discussion as part of this PR?

This is a hard problem and per previous discussions I was lead to believe we'd have a design doc first. Lets see...

The PR description and doc don't provide any detail on how this works. From a quick look at the code this is re-adding the coupling that we went to some effort to remove between scraping and remote write, plus removing the property that remote write is resilient to restarts, and adding the requirement that a remote write endpoint be stateful.

Given the breadth of these changes I think that a design doc is in order, as these would be non-trivial architectural and semantic changes to both Prometheus in general and the remote write protocol more specifically.

I'm also seeing nothing here that couldn't be done in a sidecar, this smells a lot like the StackDriver use case.

plus removing the property that remote write is resilient to restarts, and adding the requirement that a remote write endpoint be stateful.

Isn't it slightly different? Even within Prometheus, metadata is not resilient to restarts. Don't see why we should make it so within remote-write. The goal here is to make it a best-effort approach similar to what we already do within Prometheus.

Isn't it slightly different? Even within Prometheus, metadata is not resilient to restarts.

Were talking about remote write here. If you we add a feature to remote write, it should maintain the properties of remote write.

I look forward to your design doc, I've been hoping to see the novel ideas for this for a while now.

For those following the conversation along, I've made a separate design doc that includes the decision and thinking for only this part of the feature. I can't make it editable by default so I'm assuming those who wish to comment can request to do so.

Can you make it world commentable? That's how we usually do things.

I had to create a different copy for that. All the previous links should be up to date.

Just to make sure those following along, didn't miss my last message. The design doc is now commentable and open to the world.

I was just getting to reviewing that. The developers list is a better place to share such things, rather than buried in a PR.

Thanks for the tip @brian-brazil, noted!

I would like to include this in v2.23. I guess only the metric names are yet to be finalised. Looking at it now.

I have fixed the metrics and the doc comments. I will merge it in some time before starting the release process.

👍

I will merge it in some time before starting the release process.

I think we need to wait for @cstyan here, as we can't reasonably infer that the changes you just made are good with him.

Merging it as tests are passing. Build will take forever.