-
Notifications
You must be signed in to change notification settings - Fork 521
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
Correctly cancel GRPC context beneath the HTTP server #3443
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joe-elliott
requested review from
annanay25,
mdisibio,
mapno,
yvrhdn,
zalegrala,
electron0zero,
ie-pham and
stoewer
as code owners
February 29, 2024 14:08
electron0zero
approved these changes
Mar 1, 2024
Signed-off-by: Joe Elliott <number101010@gmail.com>
joe-elliott
force-pushed
the
cancel-grpc-context
branch
from
March 7, 2024 21:08
d251d7f
to
b0346e5
Compare
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
zalegrala
approved these changes
Mar 12, 2024
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.
This looks reasonable to me. Nice use of the interceptor.
joe-elliott
added a commit
to joe-elliott/tempo
that referenced
this pull request
Mar 19, 2024
* cancel context Signed-off-by: Joe Elliott <number101010@gmail.com> * update dskit Signed-off-by: Joe Elliott <number101010@gmail.com> * focused timeouts Signed-off-by: Joe Elliott <number101010@gmail.com> * docs Signed-off-by: Joe Elliott <number101010@gmail.com> * lint N docs Signed-off-by: Joe Elliott <number101010@gmail.com> * more lint Signed-off-by: Joe Elliott <number101010@gmail.com> * make update-mod Signed-off-by: Joe Elliott <number101010@gmail.com> --------- Signed-off-by: Joe Elliott <number101010@gmail.com>
joe-elliott
added a commit
that referenced
this pull request
Mar 20, 2024
* log request Signed-off-by: Joe Elliott <number101010@gmail.com> * move stuff a bit Signed-off-by: Joe Elliott <number101010@gmail.com> * oh my. e2e tests pass Signed-off-by: Joe Elliott <number101010@gmail.com> * add handlers Signed-off-by: Joe Elliott <number101010@gmail.com> * streaming tags Signed-off-by: Joe Elliott <number101010@gmail.com> * add cli support Signed-off-by: Joe Elliott <number101010@gmail.com> * improve logging Signed-off-by: Joe Elliott <number101010@gmail.com> * fix Signed-off-by: Joe Elliott <number101010@gmail.com> * docs Signed-off-by: Joe Elliott <number101010@gmail.com> * pipe overrides Signed-off-by: Joe Elliott <number101010@gmail.com> * cleanup Signed-off-by: Joe Elliott <number101010@gmail.com> * cleanup Signed-off-by: Joe Elliott <number101010@gmail.com> * support limits Signed-off-by: Joe Elliott <number101010@gmail.com> * docs Signed-off-by: Joe Elliott <number101010@gmail.com> * e2e tests and caching Signed-off-by: Joe Elliott <number101010@gmail.com> * key prefixes Signed-off-by: Joe Elliott <number101010@gmail.com> * cache keys Signed-off-by: Joe Elliott <number101010@gmail.com> * Fixed distinct collection in combiners Signed-off-by: Joe Elliott <number101010@gmail.com> * fixed combiner bugs and revived tests Signed-off-by: Joe Elliott <number101010@gmail.com> * restored all tests Signed-off-by: Joe Elliott <number101010@gmail.com> * lint Signed-off-by: Joe Elliott <number101010@gmail.com> * made search handler utilities generic Signed-off-by: Joe Elliott <number101010@gmail.com> * Added handler tests for tags Signed-off-by: Joe Elliott <number101010@gmail.com> * add diff support Signed-off-by: Joe Elliott <number101010@gmail.com> * lint Signed-off-by: Joe Elliott <number101010@gmail.com> * add distinct value collector tests Signed-off-by: Joe Elliott <number101010@gmail.com> * fix integration tests Signed-off-by: Joe Elliott <number101010@gmail.com> * diff tests Signed-off-by: Joe Elliott <number101010@gmail.com> * swapped query for the more robust ExtractMatchers(query) Signed-off-by: Joe Elliott <number101010@gmail.com> * tests Signed-off-by: Joe Elliott <number101010@gmail.com> * moved e2e tests to a more sensible place Signed-off-by: Joe Elliott <number101010@gmail.com> * fix non-deterministic test Signed-off-by: Joe Elliott <number101010@gmail.com> * changelog Signed-off-by: Joe Elliott <number101010@gmail.com> * fix tests for 429 handling Signed-off-by: Joe Elliott <number101010@gmail.com> * Update docs/sources/tempo/operations/tempo_cli.md Co-authored-by: Mario <mariorvinas@gmail.com> * review Signed-off-by: Joe Elliott <number101010@gmail.com> * Update docs/sources/tempo/api_docs/_index.md Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com> * Correctly cancel GRPC context beneath the HTTP server (#3443) * cancel context Signed-off-by: Joe Elliott <number101010@gmail.com> * update dskit Signed-off-by: Joe Elliott <number101010@gmail.com> * focused timeouts Signed-off-by: Joe Elliott <number101010@gmail.com> * docs Signed-off-by: Joe Elliott <number101010@gmail.com> * lint N docs Signed-off-by: Joe Elliott <number101010@gmail.com> * more lint Signed-off-by: Joe Elliott <number101010@gmail.com> * make update-mod Signed-off-by: Joe Elliott <number101010@gmail.com> --------- Signed-off-by: Joe Elliott <number101010@gmail.com> * Bump anchore/sbom-action from 0.15.8 to 0.15.9 (#3476) Bumps [anchore/sbom-action](https://github.com/anchore/sbom-action) from 0.15.8 to 0.15.9. - [Release notes](https://github.com/anchore/sbom-action/releases) - [Commits](anchore/sbom-action@v0.15.8...v0.15.9) --- updated-dependencies: - dependency-name: anchore/sbom-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Doc update (#3482) * doc: remove reference to previously purged script * doc: correct label for docs updates * [TraceQL Metrics] Use new per-tenant max_metrics_duration and fix duration check (#3484) * Use new per-tenant max_metrics_duration, and fix duration timestamp handling * Update docs and defaults * Handle prefixes when listing blocks from S3 and GCS (#3466) * Handle prefixes when listing blocks from S3 fixes #3465 * Handle prefixes when listing blocks from GCS * Add test for prefixes when listing blocks from Azure * Update unit tests to check for actual block IDs instead of just length of the slices Cleanup unit tests * Further refine S3/GCS backend for ListBlocks Brings logic more in line with Azure object parsing. Also has the benefit of handling prefixes without a trailing slash. * Update poller integration test to exercise prefixes * Update e2e test to exercise prefixes * Fix format check error * Fix failing e2e tests * Remove unnecessary prefix permutations from e2e test * Remove unnecessary test config file copy * Ignore lint --------- Co-authored-by: Zach Leslie <zach.leslie@grafana.com> * Update doc-validator.yml (#3483) Updates the doc-validator to the latest version. Note that this changes the reference format to use the full URL (https://....) instead of /docs/blah * [DOC] Document Tempo Operator Monolithic mode (#3474) * [DOC] Document Tempo Operator Monolithic mode Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com> * clarify supported storages Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com> * fix case of title Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com> * Apply suggestions from code review * Apply suggestions from code review --------- Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com> Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com> * [DOC] document Grafana data source setup using Grafana and Tempo operators (#3473) * [docs] document Grafana data source setup using Grafana and Tempo operators * move the Grafana data source setup page to the operator folder (this page is only relevant for the operator) * document Grafana data source setup using Grafana and Tempo operators Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com> * Apply suggestions from code review --------- Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com> Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com> * [DOC] fix typo in setup/operator/monolithic.md (#3496) Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com> * Bump github.com/prometheus/client_golang from 1.18.0 to 1.19.0 (#3455) * Bump github.com/prometheus/client_golang from 1.18.0 to 1.19.0 Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.18.0 to 1.19.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.18.0...v1.19.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Update serverless gomod --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: grafanabot <bot@grafana.com> * Add support for dashes, quotes and spaces in attribute names (#3458) * Add support for dashes, quotes and spaces in attribute names * chlog * [TraceQL Metrics] Step align query_range time range (#3490) * Step align query_range time range * Time range error: improve message and fix format for prom format. * oops remove printlns * lint * changelog * 2.4.1 changelog (#3503) Signed-off-by: Joe Elliott <number101010@gmail.com> * [DOC] Add 2.4.1 release notes (#3504) * fix tests due to interface changing Signed-off-by: Joe Elliott <number101010@gmail.com> * Pass context Signed-off-by: Joe Elliott <number101010@gmail.com> --------- Signed-off-by: Joe Elliott <number101010@gmail.com> Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com> Co-authored-by: Mario <mariorvinas@gmail.com> Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Matt Simonsen <matt.simonsen@gmail.com> Co-authored-by: Martin Disibio <martin.disibio@grafana.com> Co-authored-by: Ben Foster <bpfoster@gmail.com> Co-authored-by: Zach Leslie <zach.leslie@grafana.com> Co-authored-by: Andreas Gerstmayr <agerstmayr@redhat.com> Co-authored-by: grafanabot <bot@grafana.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does:
Currently if a GRPC streaming query or HTTP query exceeds the HTTP write timeout when passing through the HTTP server it continues until completion. i.e. context is not correctly cancelled. In addition this unhelpful error message is presented in Grafana:
This PR correctly cancels context for both the gRPC and HTTP frontend endpoints. It updates our dskit dependency to access a new http timeout middleware and uses gRPC interceptor to cancel calls to the streaming querier.
Concerns:
api_timeout
. Any suggestions?Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]