sidecar, query, receiver: Native histogram support by rabenhorst · Pull Request #6032 · thanos-io/thanos

rabenhorst · 2023-01-10T11:26:46Z

Add native histogram support for sidecar, query, receiver. It does not include necessary changes for query UI and query frontend. The plan is to do this separate PRs. Relates to #5907.

I added CHANGELOG entry for this change.
Change is not relevant to the end user.

Changes

Added native histogram to prompb and storepb types (including helper/conversion functions)
Implemented native histogram appending in receiver writer
Implemented native histogram support for related iterators

Verification

Native histogram e2e tests to test query (including dedup) and write through sidecar and receiver
- Exposed raw json message in QueryInstant of promclient as workaround (until Add histogram vectors to model prometheus/common#417 is available)
Added test and benchmark for native histogram for receiver write

pkg/dedup/iter.go

docs/proposals-accepted/202301-distributed-query-execution.md

cmd/thanos/receive.go

pkg/store/prometheus.go

fpetkovski

Looks good, thanks. Just one comment from my side

test/e2e/native_histograms_test.go

yeya24 · 2023-01-13T06:03:49Z

pkg/query/querier_test.go

 }

-// TODO(rabenhorst): Needs to be implemented for native histogram support.
 func (s *mockedSeriesIterator) AtHistogram() (int64, *histogram.Histogram) {


I see we still panic in this method but removed the comment. Do we still need to implement anything?

I will restore the comment. Rn mockedSeriesIterator does not support histograms and if we need it it needs to be implemented.

cmd/thanos/receive.go

rabenhorst · 2023-01-13T20:15:24Z

I updated prometheus/common so that it includes prometheus/common#417, which is not tagged yet but helps to improve tests (see last commit).

fpetkovski

Thanks, this looks good now 👍

GiedriusS

LGTM, just a nit

pkg/query/iter.go

GiedriusS · 2023-01-24T10:20:21Z

pkg/dedup/iter.go

 	lastUseA := it.useA
 	defer func() {
-		if it.useA != lastUseA {
+		if it.useA != lastUseA && isFloatVal {


adjustAtValue seems to already be protected against doing something with non-float values but this makes code clearer 👍

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Added comment Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Cleanup native histogram tests Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Made native hist test ha and replace usage of at with att Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Fix deduplication with native histograms Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Enabled native histogram writing Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Committed missing files for write Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Added benchmark for histogram writing Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Fixed chunkSeriesIterator Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Enabled replication for native histogram write test Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Lint native histogram test imports Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Removed ToDo comments Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Cleanup Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

* Added native histogram support for sidecar, query, receiver. Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Added comment Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Cleanup native histogram tests Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Made native hist test ha and replace usage of at with att Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Fix deduplication with native histograms Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Enabled native histogram writing Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Committed missing files for write Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Added benchmark for histogram writing Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Fixed chunkSeriesIterator Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Enabled replication for native histogram write test Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Lint native histogram test imports Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Removed ToDo comments Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Cleanup Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> * Renamed lastValue of dedup iter and added TODO Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> * Fixed typo Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> * Added hidden native histogram flag for receive tsdb Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> * merge Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> * Comments and reverted change to qfe Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> * Go mod tidy Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> * Dedup iter nit Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com> Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

pull-request-size bot added the size/XL label Jan 10, 2023

rabenhorst force-pushed the native-histograms branch 2 times, most recently from 831ee0c to 67a4190 Compare January 10, 2023 11:37

fpetkovski reviewed Jan 10, 2023

View reviewed changes

pkg/dedup/iter.go Show resolved Hide resolved

rabenhorst marked this pull request as ready for review January 10, 2023 14:25

rabenhorst force-pushed the native-histograms branch from 5e05fb2 to 29fdb83 Compare January 10, 2023 17:09

yeya24 reviewed Jan 11, 2023

View reviewed changes

docs/proposals-accepted/202301-distributed-query-execution.md Show resolved Hide resolved

yeya24 mentioned this pull request Jan 11, 2023

Add Native Histogram Support #5907

Closed

7 tasks

rabenhorst force-pushed the native-histograms branch from 29fdb83 to 0d35f4a Compare January 11, 2023 07:32

rabenhorst commented Jan 11, 2023

View reviewed changes

cmd/thanos/receive.go Outdated Show resolved Hide resolved

rabenhorst force-pushed the native-histograms branch 2 times, most recently from 57c9771 to 8edbf72 Compare January 12, 2023 13:57

fpetkovski reviewed Jan 12, 2023

View reviewed changes

pkg/store/prometheus.go Outdated Show resolved Hide resolved

fpetkovski reviewed Jan 12, 2023

View reviewed changes

yeya24 reviewed Jan 13, 2023

View reviewed changes

rabenhorst force-pushed the native-histograms branch 3 times, most recently from c938a32 to ca26c93 Compare January 16, 2023 16:07

fpetkovski previously approved these changes Jan 17, 2023

View reviewed changes

rabenhorst dismissed fpetkovski’s stale review via d61405a January 17, 2023 13:53

rabenhorst force-pushed the native-histograms branch from ca26c93 to d61405a Compare January 17, 2023 13:53

fpetkovski mentioned this pull request Jan 19, 2023

Support native histograms in store-gateway #6056

Merged

2 tasks

rabenhorst force-pushed the native-histograms branch 2 times, most recently from e3347d6 to dc81061 Compare January 19, 2023 12:10

fpetkovski previously approved these changes Jan 19, 2023

View reviewed changes

GiedriusS reviewed Jan 24, 2023

View reviewed changes

rabenhorst dismissed fpetkovski’s stale review via 4a5d074 January 24, 2023 11:01

rabenhorst force-pushed the native-histograms branch from 204b20d to 4109b40 Compare January 24, 2023 11:02

rabenhorst added 7 commits January 24, 2023 13:02

Renamed lastValue of dedup iter and added TODO

a67e913

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Fixed typo

0f28067

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Added hidden native histogram flag for receive tsdb

edfc9e6

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

merge

25e26aa

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Comments and reverted change to qfe

44617d8

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Go mod tidy

908330e

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Dedup iter nit

6bc4626

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

rabenhorst force-pushed the native-histograms branch from 4b432ea to 6bc4626 Compare January 24, 2023 12:06

GiedriusS approved these changes Jan 24, 2023

View reviewed changes

GiedriusS merged commit 071b827 into thanos-io:main Jan 24, 2023

rabenhorst mentioned this pull request Jan 24, 2023

query frontend, query UI: Native histogram support #6071

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sidecar, query, receiver: Native histogram support#6032

sidecar, query, receiver: Native histogram support#6032
GiedriusS merged 8 commits intothanos-io:mainfrom
rabenhorst:native-histograms

rabenhorst commented Jan 10, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fpetkovski left a comment

Uh oh!

Uh oh!

yeya24 Jan 13, 2023

Uh oh!

rabenhorst Jan 16, 2023 •

edited

Loading

Uh oh!

Uh oh!

rabenhorst commented Jan 13, 2023

Uh oh!

fpetkovski left a comment

Uh oh!

GiedriusS left a comment

Uh oh!

Uh oh!

GiedriusS Jan 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

rabenhorst commented Jan 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Verification

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fpetkovski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yeya24 Jan 13, 2023

Choose a reason for hiding this comment

Uh oh!

rabenhorst Jan 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rabenhorst commented Jan 13, 2023

Uh oh!

fpetkovski left a comment

Choose a reason for hiding this comment

Uh oh!

GiedriusS left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GiedriusS Jan 24, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rabenhorst commented Jan 10, 2023 •

edited

Loading

rabenhorst Jan 16, 2023 •

edited

Loading