Skip to content

Conversation

pedro-stanaka
Copy link
Contributor

@pedro-stanaka pedro-stanaka commented Oct 21, 2024

If a querier sees a "stats" parameter in the query request, it will attach important information about the query execution to the response.
But currently, even if an user sets this value, the Query Frontend will lose this value in its middleware/roundtrippers.

This PR fixes this problem by properly encoding/decoding the requests in QFE. Moreover, this PR also fixes the problem where Thanos does not know about the field "peakSamples" in the stats response from Prometheus. This field was added a while ago, but the protobuf on thanos side was never changed.

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

Verification

Updated tests, and tested locally:

image

@pedro-stanaka pedro-stanaka marked this pull request as ready for review October 21, 2024 15:51
fpetkovski
fpetkovski previously approved these changes Oct 21, 2024
If a querier sees a "stats" parameter in the query request, it will attach important information about the query execution to the response.
But currently, even if an user sets this value, the Query Frontend will lose this value in its middleware/roundtrippers.

This PR fixes this problem by properly encoding/decoding the requests in QFE.

Signed-off-by: Pedro Tanaka <[email protected]>
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.

LGTM!

@saswatamcode saswatamcode merged commit 1bdcc65 into thanos-io:main Oct 22, 2024
22 checks passed
Reimirno pushed a commit to Reimirno/thanos that referenced this pull request Oct 22, 2024
If a querier sees a "stats" parameter in the query request, it will attach important information about the query execution to the response.
But currently, even if an user sets this value, the Query Frontend will lose this value in its middleware/roundtrippers.

This PR fixes this problem by properly encoding/decoding the requests in QFE.

Signed-off-by: Pedro Tanaka <[email protected]>
Signed-off-by: Yu Long <[email protected]>
saswatamcode pushed a commit that referenced this pull request Oct 23, 2024
* UI: Select time range with mouse drag

Signed-off-by: Yu Long <[email protected]>

* QueryFrontend: pass "stats" parameter forward (#7852)

If a querier sees a "stats" parameter in the query request, it will attach important information about the query execution to the response.
But currently, even if an user sets this value, the Query Frontend will lose this value in its middleware/roundtrippers.

This PR fixes this problem by properly encoding/decoding the requests in QFE.

Signed-off-by: Pedro Tanaka <[email protected]>
Signed-off-by: Yu Long <[email protected]>

* build(deps): bump go.opentelemetry.io/otel/bridge/opentracing (#7851)

Bumps [go.opentelemetry.io/otel/bridge/opentracing](https://github.com/open-telemetry/opentelemetry-go) from 1.29.0 to 1.31.0.
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.29.0...v1.31.0)

---
updated-dependencies:
- dependency-name: go.opentelemetry.io/otel/bridge/opentracing
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Yu Long <[email protected]>

* Update CHANGELOG

Signed-off-by: Yu Long <[email protected]>

* Apply fix to linter error (from orig prom PR)

Signed-off-by: Yu Long <[email protected]>

* Fix not-null assertion bug from orig PR

Signed-off-by: Yu Long <[email protected]>

* Commit generated files

Signed-off-by: Yu Long <[email protected]>

* Fix unit test

Signed-off-by: Yu Long <[email protected]>

---------

Signed-off-by: Yu Long <[email protected]>
Signed-off-by: Pedro Tanaka <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Yu Long <[email protected]>
Co-authored-by: Pedro Tanaka <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.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.

3 participants