From 5fd84496fd5c696a22e976f2ca35f80ec7473491 Mon Sep 17 00:00:00 2001 From: szibis Date: Mon, 25 May 2026 07:35:27 +0200 Subject: [PATCH 1/5] =?UTF-8?q?test(e2e):=20un-skip=20label=5Freplace=20an?= =?UTF-8?q?d=20|>=20pattern=20include=20filter=20=E2=80=94=20implementatio?= =?UTF-8?q?ns=20already=20exist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/e2e-compat/missing_ops_compat_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/e2e-compat/missing_ops_compat_test.go b/test/e2e-compat/missing_ops_compat_test.go index 8b3207c6..551e5782 100644 --- a/test/e2e-compat/missing_ops_compat_test.go +++ b/test/e2e-compat/missing_ops_compat_test.go @@ -200,8 +200,7 @@ func TestMissingOps_PatternMatchLineFilter(t *testing.T) { query string skip bool }{ - // include_pattern: proxy does not translate |> pattern match filter (TODO: implement). - {name: "include_pattern", query: `{app="pattern-filter-test"} |> "user=<_> action=login"`, skip: true}, + {name: "include_pattern", query: `{app="pattern-filter-test"} |> "user=<_> action=login"`}, {name: "exclude_pattern", query: `{app="pattern-filter-test"} !> "result=failure"`}, } @@ -313,7 +312,6 @@ func TestMissingOps_UnwrapBytesModifier(t *testing.T) { // --------------------------------------------------------------------------- func TestMissingOps_LabelReplace(t *testing.T) { - t.Skip("known proxy gap: label_replace() not implemented in translator (TODO: implement)") ensureMissingOpsData(t) now := time.Now().UTC().Truncate(time.Minute) From 7ffbf82bcdc21bd8fd82dc396cdbf3903dd80509 Mon Sep 17 00:00:00 2001 From: szibis Date: Mon, 25 May 2026 07:42:35 +0200 Subject: [PATCH 2/5] test: clarify unpack label filter gap, add translator unit tests for unpack+filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The proxy gap claim in TestMissingOps_UnpackParser was inaccurate. The translator already correctly emits `| unpack_json | filter method:="GET"` for `| unpack | method="GET"` — the only real reason to skip those cases is that the e2e test data uses plain JSON, not Loki pack() format, so Loki returns empty results. Add three unit tests to translator_test.go confirming correct translation of bare `| unpack`, `| unpack | method="GET"`, and `| unpack | status >= 400`. Update the skip comment in missing_ops_compat_test.go to reflect the actual (sole) reason. --- internal/translator/translator_test.go | 15 +++++++++++++++ test/e2e-compat/missing_ops_compat_test.go | 10 +++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/internal/translator/translator_test.go b/internal/translator/translator_test.go index 38165775..18373209 100644 --- a/internal/translator/translator_test.go +++ b/internal/translator/translator_test.go @@ -97,6 +97,21 @@ func TestTranslateLogQL(t *testing.T) { logql: `{app="nginx"} | logfmt`, want: `app:="nginx" | unpack_logfmt`, }, + { + name: "unpack parser bare", + logql: `{app="nginx"} | unpack`, + want: `app:="nginx" | unpack_json`, + }, + { + name: "unpack then label string filter", + logql: `{app="unpack-test"} | unpack | method="GET"`, + want: `app:="unpack-test" | unpack_json | filter method:="GET"`, + }, + { + name: "unpack then label numeric filter", + logql: `{app="unpack-test"} | unpack | status >= 400`, + want: `app:="unpack-test" | unpack_json | filter status:>=400`, + }, { name: "pattern parser", logql: `{app="nginx"} | pattern " - - <_>"`, diff --git a/test/e2e-compat/missing_ops_compat_test.go b/test/e2e-compat/missing_ops_compat_test.go index 551e5782..a79060ca 100644 --- a/test/e2e-compat/missing_ops_compat_test.go +++ b/test/e2e-compat/missing_ops_compat_test.go @@ -113,9 +113,13 @@ func TestMissingOps_UnpackParser(t *testing.T) { query string skip bool }{ - // unpack_filter and unpack_status_filter are skipped: the test data uses - // plain JSON, not pack-produced format, so Loki returns empty results. - // Proxy-side unpack label filtering is also a known gap (TODO: implement). + // Skipped: the e2e test data for app="unpack-test" uses plain JSON (not + // Loki pack() format). Loki's | unpack only extracts fields from pack()- + // produced log entries, so Loki itself returns zero results for these + // queries. The translator correctly emits `| unpack_json | filter method:="GET"` + // (and `| unpack_json | filter status:>=400`), which works in VL, but + // parity testing against Loki is impossible without pack()-formatted test + // data. There is no proxy-side translation gap — the translation is correct. {name: "unpack_filter", query: `{app="unpack-test"} | unpack | method="GET"`, skip: true}, {name: "unpack_status_filter", query: `{app="unpack-test"} | unpack | status >= 400`, skip: true}, {name: "unpack_no_filter", query: `{app="unpack-test"} | unpack`}, From d9fe0bcc9125a47807c7b2de2ef6027f7bd6817f Mon Sep 17 00:00:00 2001 From: szibis Date: Mon, 25 May 2026 07:53:44 +0200 Subject: [PATCH 3/5] docs(changelog): add entries for un-skipped e2e tests and unpack gap clarification --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a34104d1..8dacf0a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Correct misleading "proxy gap" comment on `unpack_filter`/`unpack_status_filter` e2e tests — the translator correctly emits `| unpack_json | filter ...`; the tests remain skipped only because the e2e test data is plain JSON (not `pack()`-format), which Loki's `| unpack` requires + ## [1.50.0] - 2026-05-26 ### Added @@ -14,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `GET /_cache/peers` diagnostic endpoint: returns the current peer ring as `{"peers":[...],"self":"...","count":N}` — useful for verifying discovery is working correctly without inspecting logs - `srv` peer discovery mode (`-peer-discovery=srv -peer-srv=_service._tcp.domain`): resolves peers from DNS SRV records which embed port numbers. Compatible with Kubernetes StatefulSet headless services, Consul DNS, CoreDNS, and any SRV-capable resolver. Inherits readiness gating from the DNS layer. - `http` peer discovery mode (`-peer-discovery=http -peer-http-url=...`): polls an HTTP endpoint every `DiscoveryInterval` and parses the JSON response into a peer list. Auto-detects four response formats: simple string array, `{"peers":[...]}`, Prometheus HTTP SD (`[{"targets":[...]}]`), and Consul catalog (`[{"ServiceAddress":"...","ServicePort":N}]`). Works with Consul, Nomad, and custom registries outside Kubernetes. +- Translator unit tests for `| unpack`, `| unpack | method="GET"`, and `| unpack | status >= 400` — confirm correct VL translation without a live stack ### Performance - Time-bucketed cache keys align response cache entries with Grafana's `$__interval` rounding and VL's `split_interval` middleware, reducing redundant backend queries on repeated dashboard refreshes by up to 80% @@ -54,6 +58,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [1.42.0] - 2026-05-24 + ### Performance - Remove redundant proxy-side Go CIDR filtering (`ipFilterStreams`) — VL now handles `ip()` filters natively - Hoist 9 per-request `regexp.MustCompile` calls to package-level vars in the translator, eliminating repeated regex compilation on every query From 5ce282808fe9ce73a7ab065f1c63dd4e4a79ab03 Mon Sep 17 00:00:00 2001 From: szibis Date: Tue, 26 May 2026 14:14:45 +0200 Subject: [PATCH 4/5] test: unit tests for ParseAllLabelReplaceMarkers and json alias-rewrite path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add TestParseAllLabelReplaceMarkers covering single marker, chained two-level label_replace(label_replace(...)), and no-marker pass-through. The chained case was previously only validated via e2e; the comment in TestLogQL_Exhaustive_KnownGaps already noted the fix (ParseAllLabelReplaceMarkers handles nested markers) but it had no unit test. Extend TestFieldFilterMigration with the json alias-then-filter case: | json http_code="status" | http_code="200" → | unpack_json | filter status:="200" This is the exact pattern that json_field_alias_then_filter parity was testing e2e; now it has a fast, stack-free unit test too. --- CHANGELOG.md | 2 + internal/translator/translator_test.go | 65 ++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8dacf0a5..48843fda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `srv` peer discovery mode (`-peer-discovery=srv -peer-srv=_service._tcp.domain`): resolves peers from DNS SRV records which embed port numbers. Compatible with Kubernetes StatefulSet headless services, Consul DNS, CoreDNS, and any SRV-capable resolver. Inherits readiness gating from the DNS layer. - `http` peer discovery mode (`-peer-discovery=http -peer-http-url=...`): polls an HTTP endpoint every `DiscoveryInterval` and parses the JSON response into a peer list. Auto-detects four response formats: simple string array, `{"peers":[...]}`, Prometheus HTTP SD (`[{"targets":[...]}]`), and Consul catalog (`[{"ServiceAddress":"...","ServicePort":N}]`). Works with Consul, Nomad, and custom registries outside Kubernetes. - Translator unit tests for `| unpack`, `| unpack | method="GET"`, and `| unpack | status >= 400` — confirm correct VL translation without a live stack +- `TestParseAllLabelReplaceMarkers` unit tests covering single marker, chained two-level `label_replace(label_replace(...))`, and no-marker pass-through — the chained case was previously only validated via e2e; now covered as a unit test +- `TestFieldFilterMigration` extended with `json alias then filter` case: `| json http_code="status" | http_code="200"` → `| unpack_json | filter status:="200"` verifying the proxy's alias-rewrite path that fixed `json_field_alias_then_filter` parity ### Performance - Time-bucketed cache keys align response cache entries with Grafana's `$__interval` rounding and VL's `split_interval` middleware, reducing redundant backend queries on repeated dashboard refreshes by up to 80% diff --git a/internal/translator/translator_test.go b/internal/translator/translator_test.go index 18373209..16cd94e1 100644 --- a/internal/translator/translator_test.go +++ b/internal/translator/translator_test.go @@ -847,6 +847,63 @@ func TestLabelReplaceTranslation(t *testing.T) { } } +func TestParseAllLabelReplaceMarkers(t *testing.T) { + t.Run("single marker", func(t *testing.T) { + q := `label_replace(rate({app="api"}[5m]), "host", "$1", "instance", "(.*):.+")` + result, err := TranslateLogQL(q) + if err != nil { + t.Fatalf("TranslateLogQL: %v", err) + } + clean, specs := ParseAllLabelReplaceMarkers(result) + if len(specs) != 1 { + t.Fatalf("want 1 spec, got %d", len(specs)) + } + if specs[0].DstLabel != "host" || specs[0].SrcLabel != "instance" { + t.Errorf("spec mismatch: %+v", specs[0]) + } + if strings.Contains(clean, "__lvp_lr:") { + t.Errorf("marker not stripped from clean query: %q", clean) + } + }) + + t.Run("chained two markers — inner applied first", func(t *testing.T) { + // label_replace(label_replace(inner, "service","$1","app","(.*)"), "short","$1","service","^([^-]+)") + // Translator embeds one marker per nesting level; ParseAllLabelReplaceMarkers must return + // both in left-to-right (inner→outer) order so callers apply them in sequence. + q := `label_replace(label_replace(sum by(app)(count_over_time({env="production"}[5m])), "service", "$1", "app", "(.*)"), "short", "$1", "service", "^([^-]+)")` + result, err := TranslateLogQL(q) + if err != nil { + t.Fatalf("TranslateLogQL: %v", err) + } + clean, specs := ParseAllLabelReplaceMarkers(result) + if len(specs) != 2 { + t.Fatalf("want 2 specs (chained), got %d; raw=%q", len(specs), result) + } + // Inner marker: dst=service src=app + if specs[0].DstLabel != "service" || specs[0].SrcLabel != "app" { + t.Errorf("inner spec mismatch: %+v", specs[0]) + } + // Outer marker: dst=short src=service + if specs[1].DstLabel != "short" || specs[1].SrcLabel != "service" { + t.Errorf("outer spec mismatch: %+v", specs[1]) + } + if strings.Contains(clean, "__lvp_lr:") { + t.Errorf("markers not fully stripped from clean query: %q", clean) + } + }) + + t.Run("no markers — returns original", func(t *testing.T) { + q := `app:="nginx"` + clean, specs := ParseAllLabelReplaceMarkers(q) + if len(specs) != 0 { + t.Errorf("want 0 specs, got %d", len(specs)) + } + if clean != q { + t.Errorf("no-marker query was modified: %q", clean) + } + }) +} + func TestLabelJoinTranslation(t *testing.T) { cases := []struct { in string @@ -1070,6 +1127,14 @@ func TestFieldFilterMigration(t *testing.T) { logql: `{app="api"} | json | service.name = "bar"`, want: `app:="api" | unpack_json | filter "service.name":="bar"`, }, + { + // json_field_alias_then_filter: the proxy must rewrite a filter on the + // alias name back to the original field name so VL sees the correct field. + // Fixed: TranslateLogQL maps aliased filter targets to their source fields. + name: "json alias then filter on alias — rewrites to original field", + logql: `{app="api-gateway"} | json http_code="status" | http_code="200"`, + want: `app:="api-gateway" | unpack_json | filter status:="200"`, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { From fd71e84051d91b0f618b5a2b212698ce6e2ca8d2 Mon Sep 17 00:00:00 2001 From: szibis Date: Tue, 26 May 2026 21:01:47 +0200 Subject: [PATCH 5/5] =?UTF-8?q?test:=20re-skip=20|>=20include=5Fpattern=20?= =?UTF-8?q?and=20label=5Freplace=5Fsum=20=E2=80=94=20still=20known=20gaps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit |> include pattern returns mismatched line count (proxy=6 loki=0). label_replace nested inside sum causes VL parse error (400). --- test/e2e-compat/missing_ops_compat_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/e2e-compat/missing_ops_compat_test.go b/test/e2e-compat/missing_ops_compat_test.go index a79060ca..902fce7b 100644 --- a/test/e2e-compat/missing_ops_compat_test.go +++ b/test/e2e-compat/missing_ops_compat_test.go @@ -204,7 +204,7 @@ func TestMissingOps_PatternMatchLineFilter(t *testing.T) { query string skip bool }{ - {name: "include_pattern", query: `{app="pattern-filter-test"} |> "user=<_> action=login"`}, + {name: "include_pattern", query: `{app="pattern-filter-test"} |> "user=<_> action=login"`, skip: true}, {name: "exclude_pattern", query: `{app="pattern-filter-test"} !> "result=failure"`}, } @@ -325,13 +325,17 @@ func TestMissingOps_LabelReplace(t *testing.T) { tests := []struct { name string query string + skip bool }{ {name: "label_replace_basic", query: `label_replace(rate({app="api-gateway"}[5m]), "app_short", "$1", "app", "(.*)-.*")`}, - {name: "label_replace_sum", query: `sum by (app_short) (label_replace(rate({app="api-gateway"}[5m]), "app_short", "$1", "app", "(.*)-.*"))`}, + {name: "label_replace_sum", query: `sum by (app_short) (label_replace(rate({app="api-gateway"}[5m]), "app_short", "$1", "app", "(.*)-.*"))`, skip: true}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + if tc.skip { + t.Skip("known proxy gap — skipped") + } lokiResult := queryRangeResult(t, lokiURL, tc.query, start, end, "60") proxyResult := queryRangeResult(t, proxyURL, tc.query, start, end, "60")