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

Per tenant max search duration #1421

Merged
merged 10 commits into from
May 5, 2022

Conversation

joe-elliott
Copy link
Member

What this PR does:
Adds the ability to have a per tenant max search duration. By default, if there is no per tenant configuration, the original config value is used. This PR also updates the jsonnet under ./operations to mount the overrides yaml onto the frontend.

Additional changes

  • Documentation as well as catching the missing block_retention override
  • Updated the jsonnet example to use full backend search

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Copy link
Contributor

@knylander-grafana knylander-grafana left a comment

Choose a reason for hiding this comment

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

Reviewed the doc files and have a few minor suggestions. "Config" was changed to configuration so it's spelled out. Front-end component is a compound adjective so should be hyphenated.

docs/tempo/website/configuration/_index.md Outdated Show resolved Hide resolved
Copy link
Contributor

@knylander-grafana knylander-grafana left a comment

Choose a reason for hiding this comment

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

Thank you for making the suggested changes. I'll use Comment instead next time.

Signed-off-by: Joe Elliott <[email protected]>
@@ -70,6 +70,9 @@ type Limits struct {
// Querier enforced limits.
MaxBytesPerTagValuesQuery int `yaml:"max_bytes_per_tag_values_query" json:"max_bytes_per_tag_values_query"`

// QueryFrontend enforced limits
MaxSearchDuration model.Duration `yaml:"max_search_duration" json:"max_search_duration"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for model.Duration? Just comparing to MetricsGeneratorCollectionInterval time.Duration above.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we use model.Duration b/c it supports marshalling to and from JSON as well as YAML whereas time.Duration seems to only support YAML. There are 3 model.Duration fields and only 1 time.Duration.

Copy link
Contributor

@mdisibio mdisibio left a comment

Choose a reason for hiding this comment

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

LGTM just one question.

@joe-elliott joe-elliott merged commit e5c40e5 into grafana:main May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants