-
Notifications
You must be signed in to change notification settings - Fork 638
update prometheus no client golang at grafana/mimir-prometheus@fdf902d #9156
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
Conversation
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
…l` flag is enabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mimir code changes lgtm. I haven't looked closely at Prom code changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
had a look at pkg/querier, ruler, and mimirtool. pkg/distributor seems copied from upstream and I hope someone reviewed it there.
return rdResp.Results[0], nil | ||
|
||
res := rdResp.Results[0] | ||
return remote.FromQueryResult(sortSeries, res), nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why sortSeries
is a parameter now. Did the remote-read API also change? Previously they were supposed to be returned sorted by the server. Are we supposed to pass sortSeries
to the server?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the server side:
https://github.com/prometheus/prometheus/blob/28a830ed9f331e71549c24c2ac3b441033201e8f/storage/remote/read_handler.go#L83
The protocol does not have this option
https://github.com/prometheus/prometheus/blob/28a830ed9f331e71549c24c2ac3b441033201e8f/prompb/remote.proto#L31
https://github.com/prometheus/prometheus/blob/28a830ed9f331e71549c24c2ac3b441033201e8f/prompb/remote.proto#L67
https://github.com/prometheus/prometheus/blob/28a830ed9f331e71549c24c2ac3b441033201e8f/prompb/types.proto#L154
So this is pure client side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strange
Add changelog. Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
#9156) * Update mimir-prometheus Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * DefaultChunkedReadLimit was moved from storage to config Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Adopt to new remote read client interface Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Update otlp generate code Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * OTLP: ignore annotations for now Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Remote read Content-Type unset in tests Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Use client_golang 1.19.1 Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Update engine test cases from upstream Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Mark unsupported engine expressions Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Disable test that assumes the newly added `promql-delayed-name-removal` flag is enabled * Re-enable running MQE test cases against Prometheus' engine now that prometheus/prometheus#14611 has been merged * Updates due to review Add changelog. Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> --------- Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> Co-authored-by: Charles Korn <charles.korn@grafana.com> (cherry picked from commit 61634ee)
#9156) (#9176) * Update mimir-prometheus Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * DefaultChunkedReadLimit was moved from storage to config Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Adopt to new remote read client interface Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Update otlp generate code Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * OTLP: ignore annotations for now Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Remote read Content-Type unset in tests Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Use client_golang 1.19.1 Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Update engine test cases from upstream Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Mark unsupported engine expressions Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> * Disable test that assumes the newly added `promql-delayed-name-removal` flag is enabled * Re-enable running MQE test cases against Prometheus' engine now that prometheus/prometheus#14611 has been merged * Updates due to review Add changelog. Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> --------- Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> Co-authored-by: Charles Korn <charles.korn@grafana.com> (cherry picked from commit 61634ee) Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
Since #9156 probably, all the remote read commands return no data I added a missing error return using `timeseries.Err()` and got this error: `mimirtool: error: chunkedReader: message size exceeded the limit 0 bytes; got: 361 bytes, try --help` which lead me to find that we need to pass in `ChunkedReadLimit` on the client I set it at a default 1 MiB and configurable through a flag
* fix(mimirtool): Make remote-read actually return data Since #9156 probably, all the remote read commands return no data I added a missing error return using `timeseries.Err()` and got this error: `mimirtool: error: chunkedReader: message size exceeded the limit 0 bytes; got: 361 bytes, try --help` which lead me to find that we need to pass in `ChunkedReadLimit` on the client I set it at a default 1 MiB and configurable through a flag * CHANGELOG * Add other missing error * Update default read limit * Update pkg/mimirtool/commands/remote_read.go Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> --------- Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
What this PR does
Update mimir-prometheus at grafana/mimir-prometheus@fdf902d
The remote read client interface has changed in prometheus/prometheus#11379
I've updated the users and also added native histogram support to backfill.
Keeps client_golang version as it has some bugs.
prometheus/client_golang#1605
prometheus/client_golang#1607