Expose data from rule component #92
Conversation
| met := grpc_prometheus.NewServerMetrics() | ||
| met.EnableHandlingTimeHistogram( | ||
| grpc_prometheus.WithHistogramBuckets([]float64{ | ||
| 0.001, 0.01, 0.05, 0.1, 0.2, 0.4, 0.8, 1.6, 3.2, 6.4, |
There was a problem hiding this comment.
Can we ensure that all of these are the same across all thanos Grpc clients/servers i.e fetch it from common var
There was a problem hiding this comment.
Do we want that though? Right now they are the same but they are also just guesses at this point.
We might find out different components have different performance characteristics and different buckets could make sense.
cmd/thanos/rule.go
Outdated
| "gopkg.in/alecthomas/kingpin.v2" | ||
| ) | ||
|
|
||
| // registerRule registers a rule command. | ||
| func registerRule(m map[string]setupFunc, app *kingpin.Application, name string) { | ||
| cmd := app.Command(name, "query node exposing PromQL enabled Query API with data retrieved from multiple store nodes") | ||
|
|
||
| lsetStr := cmd.Flag("labels", "labels applying to all generated metrics"). | ||
| PlaceHolder("<labels>").String() |
There was a problem hiding this comment.
Why not sticking to the slicing providing by kingpin (Strings()). Seems not-consistent here to allow for , delimiter suddenly and everywhere else not (e.g known-hosts)
--label="a=1" --label="b=1"
will give use [a=1, b=2] array
pkg/store/promproxy.go
Outdated
| @@ -249,7 +249,7 @@ func encodeChunk(ss []prompb.Sample, mint int64) (storepb.Chunk_Encoding, []byte | |||
|
|
|||
| // translateAndExtendLabels transforms a metrics into a protobuf label set. It additionally | |||
| // attaches the given labels to it, overwriting existing ones on colllision. | |||
| func translateAndExtendLabels(m []prompb.Label, extend labels.Labels) []storepb.Label { | |||
| func (p *PrometheusProxy) translateAndExtendLabels(m []prompb.Label, extend labels.Labels) []storepb.Label { | |||
There was a problem hiding this comment.
Why this and above? no need for that if these function are not using anything from this struct.
There was a problem hiding this comment.
OK, can see now, these methods needs to be different for TSDB Store and you want to stick to the same naming.. Lgtm
There was a problem hiding this comment.
Yea, it's just about namespacing them.
pkg/store/tsdb.go
Outdated
| return status.Error(codes.InvalidArgument, err.Error()) | ||
| } | ||
|
|
||
| // TODO(fabxc): an improvement over this trivial approach would be to directly |
There was a problem hiding this comment.
Also: Why we need that improvement? What is the bottleneck here? (: Sorry.. it is not so straightforward.. We use chunks provided by TSDB right? We just encode it to match our gRPC proto
There was a problem hiding this comment.
Yes, we would need to transform to storepb.Chunk anyway. But the .Data field is just an opaque byte slice and we could just take those bytes as they exist in TSDB since the chunk XOR encoding is the same.
Instead we decode chunks in TSDB and create a new chunk. That's just wasted work really.
But it's easier for now and I don't know how performance relevant it would be anyway.
pkg/store/promproxy.go
Outdated
| @@ -232,7 +232,7 @@ func extLabelsMatches(extLabels labels.Labels, ms []storepb.LabelMatcher) (bool, | |||
|
|
|||
| // encodeChunk translates the sample pairs into a chunk. It takes a minimum timestamp | |||
| // and drops all samples before that one. | |||
| func encodeChunk(ss []prompb.Sample, mint int64) (storepb.Chunk_Encoding, []byte, error) { | |||
| func (p *PrometheusProxy) encodeChunk(ss []prompb.Sample, mint int64) (storepb.Chunk_Encoding, []byte, error) { | |||
There was a problem hiding this comment.
Actually PrometheusProxy could be now PrometheusStore to be consistent with other namings, right? (:
pkg/store/tsdb.go
Outdated
| // Series returns all series for a requested time range and label matcher. The returned data may | ||
| // exceed the requested time bounds. | ||
| // | ||
| // Prometheus's range query API is not suitable to give us all datapoints. We use the |
|
PTAL |
Disable vertical compaction for downsampled blocks
Signed-off-by: Subbarao Meduri <smeduri@redhat.com>
* Revert "upgrade golang/x/net to 0.17.0 (thanos-io#92)" This reverts commit 1a7d05c. Signed-off-by: Subbarao Meduri <smeduri@redhat.com> * Revert "Revert "[release-2.9] Upgrade to thanos 0.32.3 (thanos-io#84)"" This reverts commit c2dccb9. Signed-off-by: Subbarao Meduri <smeduri@redhat.com> * Revert "Revert "upgrade golang/x/net to 0.17.0 (thanos-io#85)"" This reverts commit 8214b23. Signed-off-by: Subbarao Meduri <smeduri@redhat.com> * Revert "Revert "update busybox version (thanos-io#91)"" This reverts commit 485c4f7. Signed-off-by: Subbarao Meduri <smeduri@redhat.com>
@Bplotka
I'm writing an e2e test for the rule<>query interaction now but this code here is good for review already.