Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POC RW2 to RW1 and new fields #10432

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

POC RW2 to RW1 and new fields #10432

wants to merge 10 commits into from

Conversation

krajorama
Copy link
Contributor

@krajorama krajorama commented Jan 14, 2025

What this PR does

Implements conversion of Remote Write 2.0 into the internal mimirpb.WriteRequest.

Not optimized.

Which issue(s) this PR fixes or relates to

Related to #9072

Depends on for testing:
#10425
prometheus/prometheus#15825 (merged)
prometheus/prometheus#15829 (merged)
prometheus/prometheus#15832

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

The current solution returns 2xx , but doesn't actually ingest the samples.

Prometheus does detect this
prometheus-1  | time=2025-01-13T13:01:35.028Z level=ERROR
source=queue_manager.go:1670 msg="non-recoverable error"
component=remote remote_name=150c10
 url=http://mimir-1:8001/api/v1/push failedSampleCount=2000
failedHistogramCount=0 failedExemplarCount=0
err="sent v2 request with 2000 samples, 0 histograms and 0 exemplars;
 got 2xx, but PRW 2.0 response header statistics indicate 0 samples,
 0 histograms and 0 exemplars were accepted; assumining failure e.g.
 the target only supports PRW 1.0 prometheus.WriteRequest, but does not
 check the Content-Type header correctly"

But we can do better and also start working towards RW2.0 support.

Signed-off-by: György Krajcsovits <[email protected]>
Signed-off-by: György Krajcsovits <[email protected]>
Signed-off-by: György Krajcsovits <[email protected]>
Add the encoded byte size of the metadata per time series and make a map
of used symbol references to know how much of the symbols is because
of metadata.

Signed-off-by: György Krajcsovits <[email protected]>
krajorama added a commit that referenced this pull request Jan 15, 2025
From #10432

Signed-off-by: György Krajcsovits <[email protected]>
krajorama added a commit that referenced this pull request Jan 15, 2025
* fix(rw2.0): reject remote write 2.0 based on content type

The current solution returns 2xx , but doesn't actually ingest the samples.

Prometheus does detect this
prometheus-1  | time=2025-01-13T13:01:35.028Z level=ERROR
source=queue_manager.go:1670 msg="non-recoverable error"
component=remote remote_name=150c10
 url=http://mimir-1:8001/api/v1/push failedSampleCount=2000
failedHistogramCount=0 failedExemplarCount=0
err="sent v2 request with 2000 samples, 0 histograms and 0 exemplars;
 got 2xx, but PRW 2.0 response header statistics indicate 0 samples,
 0 histograms and 0 exemplars were accepted; assumining failure e.g.
 the target only supports PRW 1.0 prometheus.WriteRequest, but does not
 check the Content-Type header correctly"

But we can do better and also start working towards RW2.0 support.

* update changelog
* Copy integration test from POC

From #10432

Signed-off-by: György Krajcsovits <[email protected]>
fix integration test
fix expdiff
better marking of debug stuff in timeseries

Signed-off-by: György Krajcsovits <[email protected]>
krajorama added a commit that referenced this pull request Jan 16, 2025
As far as I can tell we don't cast Prometheus Remote Write 1.0
histogram into mimirpb.Histogram anymore. On the flip-side
this test fails in #10432 because we're going to store RW 2.0
extra field in mimirpb.Histogram.

Related to #9072

Signed-off-by: György Krajcsovits <[email protected]>
As far as I can tell we don't cast Prometheus Remote Write 1.0
histogram into mimirpb.Histogram anymore. On the flip-side
this test fails in #10432 because we're going to store RW 2.0
extra field in mimirpb.Histogram.

Related to #9072

Signed-off-by: György Krajcsovits <[email protected]>
krajorama added a commit that referenced this pull request Jan 17, 2025
As far as I can tell we don't cast Prometheus Remote Write 1.0
histogram into mimirpb.Histogram anymore. On the flip-side
this test fails in #10432 because we're going to store RW 2.0
extra field in mimirpb.Histogram.

Related to #9072

Signed-off-by: György Krajcsovits <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant