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

feat: move detected field logic to query frontend #14248

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

loki-gh-app[bot]
Copy link
Contributor

@loki-gh-app loki-gh-app bot commented Sep 24, 2024

Backport 36ace66 from #14212


What this PR does / why we need it:

/detected_fields queries in ops are failing over bigger time ranges in Explore Logs, whereas logs queries for the same time period are not. I think this is because of all the optimizations/protections we have in place for logs queries (result cache, sharding, limits, etc.) that have not been implemented for /detected_fields. However, since /detected_fields is just doing a SelectLogs under the hood in the querier, if we move the detected fields logic up to the Query Frontend we can use the existing limited and filter tripperwares to get the underlying logs. Due to the limits already in place in those tripperwares, they will never return too many lines for the detected fields logic to reasonably handle in the Query Frontend, which allows us to move this logic there without fear of knocking over QFs.

This replaces #14210 as it works much better. I have tested in in Ops and it solves the problem without overloading QFs.

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

Sorry, something went wrong.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
(cherry picked from commit 36ace66)
@trevorwhitney trevorwhitney changed the title chore: [k221] feat: move detected field logic to query frontend feat: move detected field logic to query frontend Sep 24, 2024
@trevorwhitney trevorwhitney merged commit 4e1d2ea into k221 Sep 24, 2024
68 checks passed
@trevorwhitney trevorwhitney deleted the backport-14212-to-k221 branch September 24, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant