Propagate metadata from the WAL to remote_write #11640

tpaschalis · 2022-11-29T15:11:01Z

This PR is a follow-up to the work #10972 and #10312 to allow Metadata recorded in the WAL to be propagated through the remote_write.

In those previous PRs we prepared the WAL to be able to record metadata, as well as allowed metadata to be written to the WAL, without touching the MetadataWatcher. The functionality used to be hidden behind a feature flag, but the flag was removed until the remote_write part was wired in, since otherwise it would simply inflate WAL size without being of any use.

This first approach builds on the work of the Native Histograms, by adding another type of seriesType in the queue_manager for metadata as well as a *metadata.Metadata field in the timeSeries struct.

This new field is used to fill in a new slice when a shard is run, is decoded along the other seriesTypes, and passed along to the sendSamples method where it's used to build the write request with a new field in the TimeSeries protobuf definition.

TODO list

Add back the --metadata-storage feature flag and a new send_metadata field in remote_write configs.
Setting both the old MetadataWatcher and this new implementation defaults to using the new one to help with deprecating in the future.
Include Metadata in the TimeSeries protobuf.
Make sure the sharding calculations and buffer size per shard are not affected.

robskillington · 2022-12-06T02:42:53Z

Exciting to see progress on types being natively available in regular Prometheus Write Requesrs from Prom.

With respect to:

Figure out whether we need want to include Metadata in the TimeSeries protobuf, similarly to how Histograms were recently added

I see the current PR does the opposite of this, I think that may not be pragmatic for two major reasons:

Using “metadata” means using “MetricFamilyName” field but what we have from the WAL is metric name (as seen by accessing with lset.Get(model.MetricName)) - so this will not be accurate, data model wise it seems prudent create a Metadata field per TimeSeries so that the Metric Family Name and/or Metric Name doesn’t have to be duplicatively transmitted and/or resolved to some data that is not accessible (it seems impossible to find MetricFamilyName since this metadata is kept per Metric Name in WAL)
Sending it in a separate slice requires duplicatively transmitting the metric name and also allocating an entirely new slice server side rather than reserving space for the extra field (if non-nullable extension is used when adding the field)

Also just wanted to sign off with support and thanks.

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

cstyan

good progress 👍

cstyan · 2023-04-24T19:58:53Z

storage/remote/queue_manager.go

@@ -1347,6 +1420,7 @@ func (s *shards) runShard(ctx context.Context, shardID int, queue *queue) {
 		pBuf = proto.NewBuffer(nil)
 		buf  []byte
 	)
+	// TODO(@tpaschalis) Should we also raise the max if we have WAL metadata?


Maybe, but more likely we should revisit the defaults and recommendations around the buffer size.

For the purposes of this PR we could examine the # of shards that would be used to send samples with the metadata as is, and how many shards the current main branch uses.

I'm not sure what should be done for the purposes of this PR, could you elaborate?

Right now, the metadata would only be sent a) when first seen and b) whenever it changes, which shouldn't be too often, right? Would you expect a difference in the number of shards needed to run?

storage/remote/queue_manager.go

cstyan · 2023-04-24T20:49:41Z

tsdb/wlog/watcher.go

@@ -51,6 +51,7 @@ type WriteTo interface {
 	AppendExemplars([]record.RefExemplar) bool
 	AppendHistograms([]record.RefHistogramSample) bool
 	AppendFloatHistograms([]record.RefFloatHistogramSample) bool
+	AppendWALMetadata([]record.RefMetadata) bool


Can we just name this AppendMetadata since we didn't have a way of passing metadata to the WriteTo interface previously?

The current implementation of metadata uses the same name.

I can rename that to something like AppendWatcherMetadata or AppendFromMetadataWatcher, wdyt?

That's a different interface so it should be okay? but I think we should also ensure the scrape code doesn't get a metadata appender if the WAL metadata is being used, ie if both configs are set we should error on startup, or log a warning and only use the metadata

ie if both configs are set we should error on startup, or log a warning and only use the metadata

I also thought to error on startup, but unfortunately the previous mechanism is enabled by default; erroring out directly could be confusing to user which would have to go and explicilty disable the old one.

ts=2023-04-28T08:29:04.757Z caller=main.go:1229 level=info msg="Loading configuration file" filename=/Users/tpaschalis/prometheus.yml ts=2023-04-28T08:29:04.757Z caller=main.go:1255 level=error msg="Failed to apply configuration" err="the 'send_metadata' and 'metadata_config.send' parameters are mutually exclusive" ... ts=2023-04-28T08:29:04.764Z caller=main.go:1164 level=error err="error loading config from \"/Users/tpaschalis/prometheus.yml\": one or more errors occurred while applying the new configuration (--config.file=\"/Users/tpaschalis/prometheus.yml\")"

I think we should go with your other approach; in 42fc288, if both are set, I'm logging a warning and defaulting to implcitly disabling the old MetadataWatcher instead. WDYT?

As for the method name, there's another MetadataAppender interface which clashes with this name.

I've changed that name to AppendMetadataWatcher and kept the simple AppendMetadata for our own here. Let me know if you don't want this change and would prefer to roll it back.

tsdb/wlog/watcher.go

cstyan · 2023-04-24T20:58:12Z

cmd/prometheus/main.go

+			case "metadata-storage":
+				c.scrape.EnableMetadataStorage = true
+				level.Info(logger).Log("msg", "Experimental in-memory metadata storage enabled")


we should probably check that this is enabled when SendWALMetadata is being used, and if not print an error/warning

Done in 42fc288, after parsing the configuration file. Not sure if there's a better place for the log line that has access to both fields.

I wonder if the "SendWALMetadata" is not the ultimate feature flag for us then? Why bother second flag then if SendWALMetadata could control storage too (both has to be true always)?

cstyan · 2023-04-26T19:48:44Z

storage/remote/queue_manager.go

+func (t *QueueManager) AppendWALMetadata(ms []record.RefMetadata) bool {
+	if !t.sendMetadata {
+		return true
+	}
+
+outer:
+	for _, m := range ms {
+		t.seriesMtx.Lock()
+		lbls, ok := t.seriesLabels[m.Ref]
+		if !ok {
+			t.metrics.droppedMetadataTotal.Inc()
+			// Track dropped metadata in the same EWMA for sharding calc.
+			t.dataDropped.incr(1)
+			if _, ok := t.droppedSeries[m.Ref]; !ok {
+				level.Info(t.logger).Log("msg", "Dropped metadata for series that was not explicitly dropped via relabelling", "ref", m.Ref)
+			}
+			t.seriesMtx.Unlock()
+			continue
+		}
+		t.seriesMtx.Unlock()
+
+		backoff := t.cfg.MinBackoff
+		for {
+			select {
+			case <-t.quit:
+				return false
+			default:
+			}
+			if t.shards.enqueue(m.Ref, timeSeries{
+				seriesLabels: lbls,
+				sType:        tMetadata,
+				metadata: &metadata.Metadata{
+					Type: record.ToTextparseMetricType(m.Type),
+					Help: m.Help,
+					Unit: m.Unit,
+				},
+			}) {
+				continue outer
+			}
+
+			t.metrics.enqueueRetriesTotal.Inc()
+			time.Sleep(time.Duration(backoff))
+			backoff *= 2
+			if backoff > t.cfg.MaxBackoff {
+				backoff = t.cfg.MaxBackoff
+			}
+		}
+	}
+
+	return true
+}


I think actually this should change. If we have metadata records in the WAL and we're going to be sending metadata alongside series labels and samples in the same request, we should be treating metadata records the same as we do series records. That is, we should cache the metadata for a series, check for it whenever we receive a sample that we want to put into the next batch, and also clean up that cache when we clean up the series label cache.

So if I understand correctly, instead of enqueueing a new timeSeries struct every time a WAL metadata record is updated, we'd like to send metadata along with every sample we receive for a series?

Sending them continuously is closer to what we do now (collect and send everything every 1m by default), and the truth is that we'll never send a metadata record without a sample since they only appear in the scrape loop.

I'm just not sure whether that's wasteful or not.

I've went ahead to implement this on bd41c24 if you want to take a look. Is that what you had in mind?
a) Make the TimeSeries proto definition contain a single metadata instance (remove the repeated keyword)
b) I've removed the tMetadata seriesType and the ability to enqueue metadata on its own
c) Use the StoreMetadata method to keep track of metadata similar to the seriesLabels cache and pass that along wherever we create a timeSeries struct (in Append/AppendExemplars/AppendHistograms etc).
d) If sendMetadata is true, we append this cache value to the outgoing timeSeries struct in populateTimeSeries
e) When the series is gone, we clear the cache entry along the series' labels

After an offline sync, I'm reverting this approach (at least for now), to avoid making all requests bigger by default.

I'm keeping the change of each TimeSeries struct containing a single Metadata (not repeated).

You can't just change the remote-write proto definition, it's a standard implemented by many parties.

My initial approach was piggy-backing off the current way metadata is sent over remote_write, but was pretty inefficient and didn't take great advantage of our previous work.

This approach adds a new field, which should be backwards compatible and also similar to how Histograms were also recently added in d9d51c5. Also, like histograms, this is also something opt-in. With these in mind, are there still concerns about changing the proto definition? If then, what would be your suggestion as the next steps, to start a wider discussion?

Broadly, write a Proposal and get it accepted. See for example #10539
Metadata is an existing feature, unlike Native Histograms, so the impact of changing the way they work is much greater.

As far as I had understood how the change was being proposed was that current "metadata sync" flow would stay the same but that inline metadata would be added (so that it can be identified the exact type of a metric at push time for metric pipeline processing, rather than at sync time which requires a foreign join on metric type which cannot be 100% accurate at push time per series if the type of the metric changes for instance).

@tpaschalis is that how you had been approaching this or had you also been intending perhaps to remove the current metadata sync exchange (which is sent at 1min interval by default)?

I think @bboreham what @tpaschalis is pointing out that sending the metadata inline (which the whole point of this is as I understand is to make certain attributes available at push time and guaranteed to be available) has far lower network transfer than sending it in the "metadata" field that exists in protobuf today (since then labels for each series are written once, rather than twice, once for timeseries message and once for metadata message).

As far as I understand though, instead of changing any current metadata sync requests that are made or removing the metadata field in protobuf, it could be simply an additive change to also make metadata available inline on the timeseries message (and hence reduce the overall network effect made by this change).

Here's the original discussion on this approach:
#7771

Related design doc:
https://docs.google.com/document/d/1LY8Im8UyIBn8e3LJ2jB-MoajXkfAqW2eKzY735aYxqo/edit#heading=h.bik9uwphqy3g

Related mailing list discussion:
https://groups.google.com/g/prometheus-developers/c/w-eotGenBGg/m/UZWdxzcBBwAJ

Related stats (more on that PR link):

Here's the increase with a default config, the difference is relatively less measurable - likely to do with the previous config sending larger batches since the send deadline was longer with a max samples per send of 5,000:

steady state ~177kb/sec without this change

steady state ~210kb/sec with this change

roughly 20% increase

I apologise, I was reacting to bd41c24, which includes a change to a field:
bd41c24#diff-bdaf80ebc5fa26365f45db53435b960ce623ea6f86747fb8870ad1abc355f64fR150

Now I see that field was only added in the context of this PR; it wouldn't itself be a breaking change.

I'm sorry I haven't read all the history but I don't understand why we are discussing metadata on a per-series level.
Prometheus doesn't ingest or store metadata that way; it's per-name or "metric family".

Elsewhere, people have pointed me at a conversation where metadata on a per-series level was discussed.
Please ignore me, I don't have the context to get involved.

prompb/types.proto

…r to StoreSeries Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

…a similar to StoreSeries" This reverts commit bd41c24.

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

…defaulting to the new one Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

…tadata Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

…ueueManager Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

tpaschalis · 2023-04-28T11:02:35Z

storage/remote/queue_manager.go

+	// The current MetadataWatcher implementation is mutually exclusive
+	// with the new approach, which stores metadata as WAL records and
+	// ships them alongside series. If both mechanisms are set, the new one
+	// takes precedence by implicitly disabling the older one.
+	if t.mcfg.Send && t.sendMetadata {
+		level.Warn(logger).Log("msg", "the 'send_metadata' and 'metadata_config.send' parameters are mutually exclusive; defaulting to use 'send_metadata'")
+		t.mcfg.Send = false
+	}


Initially I put this snippet into storage/remote/write.go and the ApplyConfig method but mutating the config there meant that the config hash was no longer deterministic, tripping up some tests and possibly users.

While the tests would be easy to fix, I felt that putting them one level deeper here might be better.

jaredjenkins · 2023-05-02T02:22:01Z

Hi @tpaschalis thanks for driving this feature! We're excited to see this land as well. I was wondering what the status of this feature is? Is there anything that I can help with to unblock this?

I haven't been following the code closely, but I did notice that you were inlining the metadata into the TimeSeries. While this does introduce some additional overhead it makes discovering type data much easier. Having to essentially guess at the possible metric family from the series can problematic for a number of reasons that I am sure you are already aware of.

robskillington · 2023-05-02T20:47:18Z

storage/remote/queue_manager.go

+		case tMetadata:
+			pendingData[nPending].Metadata = prompb.Metadata{
+				Type: metricTypeToProtoEquivalent(d.metadata.Type),
+				Help: d.metadata.Help,
+				Unit: d.metadata.Unit,
+			}
+			nPendingMetadata++


Hm, so this wouldn't make metadata available inline for series with samples/exemplars/histograms/etc. It would be sent entirely separately in the array of timeseries sent and the labels would be duplicated inside a request where samples/other were sent for a timeseries.

I thought this was a:
(1) A non-starter network transfer wise since would create another entry in the array with the same set of labels as an entry with samples or exemplars or histograms.
(B) Wouldn't guarantee metadata available when processing an entry with timeseries in any consumer of remote write due to batching/etc separating what is sent that's in the queue. I also believed that making this available as part of the same request for any given timeseries with samples/values sent was the primary reason to propagate it from the WAL rather than sync it every 1min (otherwise it would not be part of the request processing the timeseries with samples/values, similar to today's situation where it's not present since it comes during an entirely separate set of request/responses through an async 1min metadata sync push).

Currently it looks like the metadata record is enqueued separately and therefore would never have samples/exemplars/histograms/etc set for this entry in pendingData[nPending]:

prometheus/storage/remote/queue_manager.go

Lines 817 to 828 in 208922b

if t.shards.enqueue(m.Ref, timeSeries{

seriesLabels: lbls,

sType: tMetadata,

metadata: &metadata.Metadata{

Type: record.ToTextparseMetricType(m.Type),

Help: m.Help,

Unit: m.Unit,

},

}) {

continue outer

}

Previously with this working (in PR #7771) end to end instead we kept the metadata available then attached it to samples as they were enqueued:

prometheus/storage/remote/queue_manager.go

Lines 341 to 367 in b6e5bd1

meta, _ := t.seriesMetadata[s.Ref]

t.seriesMtx.Unlock()

// This will only loop if the queues are being resharded.

backoff := t.cfg.MinBackoff

for {

select {

case <-t.quit:

return false

default:

}

if t.shards.enqueue(s.Ref, sample{

labels: lbls,

meta: meta,

t: s.T,

v: s.V,

}) {

continue outer

}

t.metrics.enqueueRetriesTotal.Inc()

time.Sleep(time.Duration(backoff))

backoff = backoff * 2

if backoff > t.cfg.MaxBackoff {

backoff = t.cfg.MaxBackoff

}

}

Then this was set inline when the timeseries with samples for the entry was sent:

prometheus/storage/remote/queue_manager.go

Lines 864 to 866 in b6e5bd1

pendingSamples[nPending].Type = metricTypeToMetricTypeProto(sample.meta.Type)

pendingSamples[nPending].Unit = sample.meta.Unit

pendingSamples[nPending].Help = sample.meta.Help

cstyan · 2023-05-04T18:52:33Z

Sorry, haven't had time this week/next to follow up on my review due to on-call.

bwplotka

Thanks for this!

I think, given this PR is trying to modify metadata handling in remote write, it's time to finalize our spec for Remote Write v1.1 in regards to metadata in some proposal (gdoc, or PR to https://github.com/prometheus/proposals) before we merge any changes to remote write proto.

If we would like to scope down this PR, we could just use WAL data for existing remote write protocol.

Otherwise, let's collab on some Remote Write spec's final decision. I loved the google group discussion Rob's started, as well as Rob's design document. On the different note the OTLP metrics is now on the way to be 1.0 and simply ignores (maybe for a good reason?) majority of edge cases we discussed (metadata different per series, efficiency considerations etc). Would love to research their point of view too, maybe we are overcomplicating things? 🤔 Or perhaps it's fair to match the storage edge cases and keep the metadata near series (or even samples?)

bwplotka · 2023-06-06T12:53:28Z

cmd/prometheus/main.go

@@ -181,6 +181,9 @@ func (c *flagConfig) setFeatureListOptions(logger log.Logger) error {
 			case "extra-scrape-metrics":
 				c.scrape.ExtraMetrics = true
 				level.Info(logger).Log("msg", "Experimental additional scrape metrics enabled")
+			case "metadata-storage":


Note for reviewers: This was removed on purpose with #11351

bwplotka · 2023-06-06T12:55:30Z

cmd/prometheus/main.go

+			case "metadata-storage":
+				c.scrape.EnableMetadataStorage = true
+				level.Info(logger).Log("msg", "Experimental in-memory metadata storage enabled")


I wonder if the "SendWALMetadata" is not the ultimate feature flag for us then? Why bother second flag then if SendWALMetadata could control storage too (both has to be true always)?

bwplotka · 2023-06-06T16:39:12Z

config/config.go

@@ -857,6 +857,7 @@ type RemoteWriteConfig struct {
 	Name                 string            `yaml:"name,omitempty"`
 	SendExemplars        bool              `yaml:"send_exemplars,omitempty"`
 	SendNativeHistograms bool              `yaml:"send_native_histograms,omitempty"`
+	SendWALMetadata      bool              `yaml:"send_metadata,omitempty"` // TODO(@tpaschalis) Adding an extra field to enable us to remove the `metadata_config` struct in the future.


What's the rationale for removal of MetadataConfig? Any design doc/link to the discussion? It feels we might want to give more config options to metadata in future, depending on v1.1 spec we will came out with.

The idea behind this is that if we treat metadata as just another new type of datapoint in the WAL (like exemplars or native histograms) we wouldn't need any more specific configuration (so we wouldn't have a SendInterval or MaxPerSend option).

Do you have any specific ideas for config options we might need? We can kick off the v1.1 spec discussion and see if something comes up there.

bwplotka · 2023-06-06T16:40:32Z

prompb/types.proto

@@ -127,6 +147,7 @@ message TimeSeries {
  repeated Sample samples       = 2 [(gogoproto.nullable) = false];
  repeated Exemplar exemplars   = 3 [(gogoproto.nullable) = false];
  repeated Histogram histograms = 4 [(gogoproto.nullable) = false];
+  Metadata metadata             = 5 [(gogoproto.nullable) = false];


I think we need design document with a team agreement on what's remote write v1.1 before we start modifying this 🤔

~~Initially I thought we wouldn't have to explicitly change the protocol since it was an extra optional signal (again like exemplars and histograms), if we think that's the way to go I'm up for it!~~

@cstyan also had some ideas about things that could go into a future v1.1 or v1.2 version of the protocol so we can discuss if we want to also bring any more changes.

Update: Yeah, I'm also convinced this is the way to go! What would be the process to start things for drafting v1.1?

Getting consensus on the Google doc proposal and then opening a PR to the docs? Sending to the mailing list?

bwplotka

Thanks for this!

I think, given this PR is trying to modify metadata handling in remote write, it's time to finalize our spec for Remote Write v1.1 in regards to metadata in some proposal (gdoc, or PR to https://github.com/prometheus/proposals) before we merge any changes to remote write proto.

If we would like to scope down this PR, we could just use WAL data for existing remote write protocol.

Otherwise, let's collab on some Remote Write spec's final decision. I loved the google group discussion Rob's started, as well as Rob's design document. On the different note the OTLP metrics is now on the way to be 1.0 and simply ignores (maybe for a good reason?) majority of edge cases we discussed (metadata different per series, efficiency considerations etc). Would love to research their point of view too, maybe we are overcomplicating things? 🤔 Or perhaps it's fair to match the storage edge cases and keep the metadata near series (or even samples?)

tpaschalis · 2023-06-15T15:01:56Z

@bwplotka @cstyan @robskillington

Thanks for your feedback. Apologies for the radio silence, I think we're ready for a push to try and get this over the line. I've tried to summarise the most pressing points in a Google document; it might help with opening a wider discussion and helping other people highlight alternatives.

I think there are two main questions to address here.
First, whether we need to make this into a new remote write protocol version, or if we can proceed as-is.
While the idea of a new protocol version does sound interesting, I'm not sure how much friction it would add. We'd not only have to set up a precedent/process for this, we'd also need to go into designing a protocol negotiation thing and making sure that downstream databases/consumers are aware of it, maybe also get their feedback etc.

Second, how and how often metadata will be sent. The google doc outlines our options, but after some thought, Rob's comments and @beorn7's view in the original discussion, I'm leaning heavily into the proposal that we always send metadata along every sample. This is the most resource-intensive option, but it does guarantee that up-to-date metadata is available for any consumer, and is a step into making metadata a first-class citizen.

I'll revert some recent code changes to go with that approach, and maybe we could gauge its impact with a prombench run.

tpaschalis · 2023-06-15T15:02:25Z

Apologies, hit Close instead of Comment by accident 🤦

…t alongside any sample Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

bwplotka · 2023-06-19T20:51:13Z

Thanks for proposal. I think we need agreement on spec and to me it's similar amount of work to agreeing officially in this PR (it's network protocol after all). If we have even slight agreement on doc, we can start with experimental implementation first with some negotiation.

.. and I love your proposal: Per series & send always is an amazing starting point. Feel free to propagate this document on prometheus-developer list.

beorn7 · 2023-12-12T11:22:47Z

Looking at this during the bug scrub.

@tpaschalis With the recent progress WRT remote write 2.0, is this worth revisiting?

tpaschalis · 2023-12-14T08:54:02Z

Hey @beorn7. I'll haven't followed up with the latest progress; I'll check what happened.

This was put on ice until the interning table was implemented so that we don't blow up people's egress bills; changing to use the interned format should be easy once it's there.

cstyan · 2023-12-18T00:28:45Z

Yep the intention is still to finish this off with the new interning format.

thakerb · 2024-02-04T22:46:48Z

@tpaschalis @cstyan Any update on the expected/approximate time-frame for completion of this PR please?

cstyan · 2024-02-06T16:51:45Z

closing since we merged this work into the remote write 2.0 feature branch

nirgilboa · 2024-02-06T17:05:28Z

What's the best way to follow the remote write 2.0 development?

cstyan · 2024-02-06T19:01:15Z

At the moment we're tracking remaining work here #6333

bboreham mentioned this pull request Apr 12, 2023

Manually stored metrics do not have metadata grafana/alloy#547

Open

tpaschalis added 3 commits April 24, 2023 13:08

Approach bundling metadata along with samples and exemplars

ac83b3a

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Add first test; rebase with main

49cccc6

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Alternative approach: bundle metadata in TimeSeries protobuf

0093563

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

tpaschalis force-pushed the metadata-remote-write-wiring-2 branch from 9b1b000 to 0093563 Compare April 24, 2023 13:49

tpaschalis added 6 commits April 24, 2023 17:11

Fix linting errors

403385f

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Fix linter error

514b6a3

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Fix sendMetadata field usage

3cc7a7f

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Yet another linter fix

32cf14c

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Add a few more tests; remove unused helpers

5227af2

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Use labels.FromStrings helper to circumvent linter error

3f967e1

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

cstyan reviewed Apr 24, 2023

View reviewed changes

cstyan reviewed Apr 26, 2023

View reviewed changes

jaredjenkins reviewed Apr 27, 2023

View reviewed changes

prompb/types.proto Outdated Show resolved Hide resolved

tpaschalis added 5 commits April 27, 2023 18:32

Change logic around AppendWALMetadata; introduce StoreMetadata simila…

bd41c24

…r to StoreSeries Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Revert "Change logic around AppendWALMetadata; introduce StoreMetadat…

3acfa1b

…a similar to StoreSeries" This reverts commit bd41c24.

Remove repeated keyword from TimeSeries' metadata field

7ee3ae6

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Add mechanism for choosing between the two metadata-sending methods, …

42fc288

…defaulting to the new one Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Rename old interface method to AppendWatcherMetadata; new to AppendMe…

6f05973

…tadata Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

tpaschalis requested a review from cstyan April 28, 2023 09:19

tpaschalis added 2 commits April 28, 2023 13:57

Move logic for implicitly choosing one of the two mechanism into NewQ…

a5c1def

…ueueManager Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Mutate the correct field when choosing metadata mechanism

208922b

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

tpaschalis commented Apr 28, 2023

View reviewed changes

robskillington reviewed May 2, 2023

View reviewed changes

bwplotka reviewed Jun 6, 2023

View reviewed changes

tpaschalis closed this Jun 15, 2023

tpaschalis reopened this Jun 15, 2023

tpaschalis added 2 commits June 19, 2023 11:45

Re-introduce the StoreMetadata method; make metadata to always be sen…

04a08ef

…t alongside any sample Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

Fix test helper; remove unnecessary conversion

bc87310

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>

davidgit mentioned this pull request Sep 19, 2023

NewRelic Prometheus Configurator Not Assigning Correct Metadata Type By Reading The Metrics File newrelic/newrelic-prometheus-configurator#235

Open

This was referenced Oct 30, 2023

remote write 2.0: new proto format with string interning #13052

Merged

Discussion: Support OTLP sending in addition to Remote Write #12633

Closed

npazosmendez mentioned this pull request Dec 28, 2023

Rework remote write metadata propagation to use new 2.0 protocol tpaschalis/prometheus#142

Closed

cstyan mentioned this pull request Jan 3, 2024

remote write 2.0 - metadata #13367

Closed

cstyan mentioned this pull request Jan 12, 2024

Remote write v2: metadata support in every write request #13394

Merged

cstyan closed this Feb 6, 2024

	if t.shards.enqueue(m.Ref, timeSeries{
	seriesLabels: lbls,
	sType: tMetadata,
	metadata: &metadata.Metadata{
	Type: record.ToTextparseMetricType(m.Type),
	Help: m.Help,
	Unit: m.Unit,
	},
	}) {
	continue outer
	}

	meta, _ := t.seriesMetadata[s.Ref]
	t.seriesMtx.Unlock()
	// This will only loop if the queues are being resharded.
	backoff := t.cfg.MinBackoff
	for {
	select {
	case <-t.quit:
	return false
	default:
	}

	if t.shards.enqueue(s.Ref, sample{
	labels: lbls,
	meta: meta,
	t: s.T,
	v: s.V,
	}) {
	continue outer
	}

	t.metrics.enqueueRetriesTotal.Inc()
	time.Sleep(time.Duration(backoff))
	backoff = backoff * 2
	if backoff > t.cfg.MaxBackoff {
	backoff = t.cfg.MaxBackoff
	}
	}

	pendingSamples[nPending].Type = metricTypeToMetricTypeProto(sample.meta.Type)
	pendingSamples[nPending].Unit = sample.meta.Unit
	pendingSamples[nPending].Help = sample.meta.Help

Propagate metadata from the WAL to remote_write #11640

Propagate metadata from the WAL to remote_write #11640

Conversation

tpaschalis commented Nov 29, 2022 • edited Loading

TODO list

robskillington commented Dec 6, 2022 • edited Loading

cstyan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robskillington May 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaredjenkins commented May 2, 2023 • edited Loading

robskillington May 2, 2023 • edited Loading

Choose a reason for hiding this comment

cstyan commented May 4, 2023

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpaschalis Jun 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

tpaschalis commented Jun 15, 2023

tpaschalis commented Jun 15, 2023

bwplotka commented Jun 19, 2023

beorn7 commented Dec 12, 2023

tpaschalis commented Dec 14, 2023

cstyan commented Dec 18, 2023

thakerb commented Feb 4, 2024

cstyan commented Feb 6, 2024

nirgilboa commented Feb 6, 2024

cstyan commented Feb 6, 2024

tpaschalis commented Nov 29, 2022 •

edited

Loading

robskillington commented Dec 6, 2022 •

edited

Loading

robskillington May 2, 2023 •

edited

Loading

jaredjenkins commented May 2, 2023 •

edited

Loading

robskillington May 2, 2023 •

edited

Loading

tpaschalis Jun 14, 2023 •

edited

Loading