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

Update Prometheus to v0.40.1 #5896

Merged
merged 16 commits into from
Dec 16, 2022
Merged

Conversation

rabenhorst
Copy link
Contributor

@rabenhorst rabenhorst commented Nov 15, 2022

Update Prometheus to v0.40.1 without implementing native histograms.

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

Changes

  • Updated prometheus to v0.40.1
  • Updated functions of iterators so that they implement the new chunkenc.Iterator interface and use chunkenc.ValueType
  • Adjusted querier tests to match expected results that slightly changed because of native histograms
  • Small changes to http server to make it work with latest web toolkit version
  • Update weaveworks common to prometheus v0.40.1

Verification

  • Existing unit and e2e tests

@rabenhorst rabenhorst changed the title Update to prometheus v0.40.1 Update Prometheus to v0.40.1 Nov 15, 2022
@rabenhorst rabenhorst force-pushed the prometheus-v0.40.x branch 2 times, most recently from d3e2175 to 0e4556c Compare November 15, 2022 21:38
@rabenhorst rabenhorst marked this pull request as ready for review November 15, 2022 22:16
pkg/dedup/iter.go Outdated Show resolved Hide resolved
{T: 1587693300000, V: 14.23859649122807}, {T: 1587693400000, V: 15.407017543859647}, {T: 1587693500000, V: 15.915789473684208}, {T: 1587693600000, V: 15.712280701754386},
{T: 1587690300000, V: 13.652631578947368}, {T: 1587690400000, V: 14.049122807017543}, {T: 1587690500000, V: 13.961403508771928}, {T: 1587690600000, V: 13.617543859649121}, {T: 1587690700000, V: 14.568421052631578}, {T: 1587690800000, V: 14.989473684210525},
{T: 1587690900000, V: 16.2}, {T: 1587691000000, V: 16.052631578947366}, {T: 1587691100000, V: 15.831578947368419}, {T: 1587691200000, V: 15.659649122807016}, {T: 1587691300000, V: 14.842105263157894}, {T: 1587691400000, V: 14.003508771929823},
{T: 1587691500000, V: 13.782456140350876}, {T: 1587691600000, V: 13.86315789473684}, {T: 1587691700000, V: 15.270282598474376}, {T: 1587691800000, V: 14.343859649122805}, {T: 1587691900000, V: 13.975438596491227}, {T: 1587692000000, V: 13.399999999999999},
Copy link
Member

@GiedriusS GiedriusS Nov 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting that the values have changed here 🤔

Adjusted querier tests to match expected results that slightly changed because of native histograms

Maybe you know why they have changed because of native histograms?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember having to update the extrapolatedRate function in the Thanos engine when updating Prometheus to 2.40.1. I was getting small differences related to floating point calculation since they changed the order of divisions/multiplications. It seems like these changes here are also on the last decimal points which could be the reason why the numbers need slight tweaks.

Copy link
Contributor

@fpetkovski fpetkovski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. I wonder if we want to add support for native histograms in the vendored Cortex codebase. AFAIK we only use the query-frontend from Cortex, so maybe adding support there is not high priority.

saswatamcode
saswatamcode previously approved these changes Nov 16, 2022
Copy link
Member

@saswatamcode saswatamcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for awesome work! 🌟

One small question.

@@ -75,7 +75,14 @@ func (s *Server) ListenAndServe() error {
if err != nil {
return errors.Wrap(err, "server could not be started")
}
return errors.Wrap(toolkit_web.ListenAndServe(s.srv, s.opts.tlsConfigPath, s.logger), "serve HTTP and metrics")

flags := &toolkit_web.FlagConfig{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should just import flags from exporter-toolkit and pass those options. There is https://pkg.go.dev/github.com/prometheus/exporter-toolkit@v0.8.1/web/kingpinflag#AddFlags

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would introduce more changes. I can do that in a separate followup PR. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, follow-up sounds good!

@rabenhorst
Copy link
Contributor Author

rabenhorst commented Dec 13, 2022

@GiedriusS I added a test for both querying native histograms through sidecar and writing native histograms to receiver, to make sure there is no unexpected behavior (error for querying and noop for writing): rabenhorst#1

How I understand it: For querying through the sidecar we will get Error executing query: invalid chunk encoding "<unknown>" because the chunk encoding for histograms in Prometheus types is 2 and it is translated to 1 in Thanos which is invalid in Thanos and Prometheus returns error.
Regarding remote writes (to receivers) histogram does not exist in Thano's prompb type and is ignored.

rabenhorst and others added 9 commits December 13, 2022 20:51
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Fixed pkg

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

Fixed internal

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

Updated weaveworks/common

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

Fixed tests

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

Fixed some querier tests and commented out log spam

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

Fixed querier test

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

Reverted tls integration test

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

Fixed TestChunkQueryable

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

Implemented pushdownSeriesIterator AtT fn

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

Fixed cloudtrace import

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

Ran go mod tidy

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

trigger tests

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
GiedriusS
GiedriusS previously approved these changes Dec 15, 2022
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tests and thorough explanation, I think you are correct 🙇 let's work on native histograms now and further improvements 💪 awesome work! Just a few last nits before merging this. Also, in the CHANGELOG I think we should add an item. Please explicitly note that native histograms are not supported and queries using them will fail with that error just like in the tests 😄

test/e2e/native_histograms_test.go Outdated Show resolved Hide resolved
test/e2e/native_histograms_test.go Outdated Show resolved Hide resolved
test/e2e/native_histograms_test.go Outdated Show resolved Hide resolved
…s to v0.40.7

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
@GiedriusS
Copy link
Member

GiedriusS commented Dec 15, 2022

Damn, unlucky - Prometheus busybox sha256 changed 😄 Maybe you can update to the new hash https://quay.io/repository/prometheus/busybox/manifest/sha256:4ee0a8dced91db2c03eb2a69f20e95a41ad9602ebf3e6892b2666c02cf99cc78 so that e2e tests could run? We have a cronjob that periodically updates these but we can be faster in this PR 😄

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
@rabenhorst
Copy link
Contributor Author

Damn, unlucky - Prometheus busybox sha256 changed 😄 Maybe you can update to the new hash https://quay.io/repository/prometheus/busybox/manifest/sha256:4ee0a8dced91db2c03eb2a69f20e95a41ad9602ebf3e6892b2666c02cf99cc78 so that e2e tests could run? We have a cronjob that periodically updates these but we can be faster in this PR 😄

Done :D

Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪 thanks for your contribution!

@GiedriusS GiedriusS merged commit 789046a into thanos-io:main Dec 16, 2022
fpetkovski pushed a commit to fpetkovski/thanos that referenced this pull request Jan 13, 2023
* Updated prometheus to v0.40.1

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

Fixed pkg

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

Fixed internal

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

Updated weaveworks/common

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

Fixed tests

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

Fixed some querier tests and commented out log spam

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

Fixed querier test

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

Reverted tls integration test

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

Fixed TestChunkQueryable

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

Implemented pushdownSeriesIterator AtT fn

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

Fixed cloudtrace import

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

Ran go mod tidy

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

trigger tests

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

* Uncommented log line

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

* Removed TODOs and fixed test message

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

* Updated prom engine

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

* Added missing nil check for delSeriesIterator and comment

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

* Fixed comment

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

* Started working on e2e tests

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

* Added native histograms e2e tests

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

* Updated

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

* Added license header to native histogram tests

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

* Added retry for TestQueryNativeHistograms

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

* Improved native histogram tests, added changelog and update prometheus to v0.40.7

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

* Fixed changelog

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

* Removed trailing space from changelog

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

* Ran busy box updater

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

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
ngraham20 pushed a commit to ngraham20/thanos that referenced this pull request May 18, 2023
* Updated prometheus to v0.40.1

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

Fixed pkg

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

Fixed internal

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

Updated weaveworks/common

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

Fixed tests

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

Fixed some querier tests and commented out log spam

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

Fixed querier test

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

Reverted tls integration test

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

Fixed TestChunkQueryable

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

Implemented pushdownSeriesIterator AtT fn

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

Fixed cloudtrace import

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

Ran go mod tidy

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

trigger tests

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

* Uncommented log line

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

* Removed TODOs and fixed test message

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

* Updated prom engine

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

* Added missing nil check for delSeriesIterator and comment

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

* Fixed comment

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

* Started working on e2e tests

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

* Added native histograms e2e tests

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

* Updated

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

* Added license header to native histogram tests

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

* Added retry for TestQueryNativeHistograms

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

* Improved native histogram tests, added changelog and update prometheus to v0.40.7

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

* Fixed changelog

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

* Removed trailing space from changelog

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

* Ran busy box updater

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

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants