Repository navigation
Conversation
… accept keywords in any case Loki 3.7.7 has no group aggregation and no label_join: its lexer makes a function token only of the names in functionTokens, so any other name is an IDENTIFIER the grammar cannot place and the query is a 400 parse error at the column of the name. The proxy kept every unknown name(...) as an opaque call and answered it. The parser now keeps opaque only label_replace, vector and approx_topk and reports every other name the way Loki does, also inside a label_replace argument. Loki lower-cases a word before it looks it up, so TOPK(3, X), SUM BY (a) (X), RATE(...) and OFFSET are the keywords they spell, while a label called Rate or Count stays a label. The scanner reads keywords in any case and the query parameter is written with lower-case keywords before a handler reads it.
…e rewriting Loki has neither, and the validator now rejects both before the translator runs, so the marker suffixes, their parsers and the proxy-side rewrites of a matrix response were unreachable.
… and document the change The e2e suite runs the queries Loki rejects against Loki and every Loki-compatible proxy on both query endpoints and compares status and text, and runs uppercase keyword queries and compares the series. group() and label_join() move from the parity and known-gap lists to the error parity list.
…pare the proxy with itself in the e2e The multi-tenant fan-out re-reads the query from the URL, so the lower-case rewrite covers the URL query as well as the parsed form. The proxy tests now send each uppercase query as a GET, a form-encoded POST and a multi-tenant GET. The e2e compares the upper-case answer of the proxy with its own lower-case answer; Loki only has to answer 200 for both spellings.
github-actions
Bot
added
size/XL
Contributor
⚪ Performance A/B: no change beyond noise125 shape×range — 125 ⚪ same · results vs Loki: 105 same, 0 new difference(s), 20 pre-existing Every shape is within noise of the base build (±30% and 100 ms). 125 unchanged shape×range (within noise)
Cold (first-run) timings — what a user opening a panel sees
Why these shapes
base |
Contributor
PR Quality ReportCompared against base branch Coverage and tests
Compatibility
Performance smokeLower CPU cost (
State
|
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
This adds a new gap row,
semantics/proxy-accepts-operators-not-in-loki. The project rule is that the proxy accepts exactly the LogQL Loki accepts.What Loki does. Loki 3.7.7 turns a name into a function token only if it appears in
functionTokens(pkg/logql/syntax/lex.go:95-137). Any other name is an IDENTIFIER the grammar cannot place. Loki then answersparse error at line 1, col N: syntax error: unexpected IDENTIFIER.What the proxy did.
name(...)as an opaque expression, and implementedgroup(...)andlabel_join(...)as extensions.group,label_join,abs,sgn,clamp*,round,ceil,floor,sqrt,exp,ln,log2,log10,sin,cos,scalar,timestamp,hour,quantile,present_over_time,increase,delta,irate,absentandhistogram_quantile.time(),day_of_week()andcount_values()it answered 400 with its own text.lex.go:226-252), soTOPK(3, RATE(...)),SUM BY,ON,BOOLand| JSON | DROPare valid in Loki. The proxy rejected them or sent different backend queries;GROUP(...)even answered a 200streamsresult.Fix.
RateorCountstays a label.label_replace,vectorandapprox_topkopaque. Any other name gets Loki's exact error text and column, before any backend call.group()andlabel_join()translation and response rewriting are removed: about 340 lines.Tests
TestValidateLogQL_NamesLokiDoesNotKnowAreRejectedandTestValidateLogQL_KeywordsAreCaseInsensitive. Their expected texts come from running Loki's ownsyntax.ParseExpr, not from hand-typing. 77 subtests failed before the fix.TestLowercaseKeywords.TestUnknownLogQLNamesRejectedLikeLoki: 11 queries on/queryand/query_rangethrough the mux. It checks a 400 with Loki's text and no backend call.TestUppercaseKeywordsSendTheLowercaseBackendRequests: GET, form POST and multi-tenant.TestCompat_OperatorsNotInLokiRejectedandTestCompat_UppercaseKeywordsMatchLoki.Behaviour change
A selector label spelled like a keyword in any case (
{By="x"},{On="x"}) is now rejected, as Loki does.Merge order
Merge after #735, #744 and #741. Those PRs still carry
label_joinandgrouphandling, and this PR removes it when it is rebased.Pending before ready
TestLogQL_Exhaustive_*.LowercaseKeywords, which runs on every routed request whose query has an uppercase letter.approx_topkis answered as a log query, while Loki evaluates it;-group, keyword-named selector labels,unwrapin a log query,sum({...}), andavg_over_timewithout unwrap.semantics/count-values-unsupported: Loki 3.7.7 has nocount_values, so that row's premise is false.