From 38f9837f144dd503623caecc4048ee6371e5a79e Mon Sep 17 00:00:00 2001 From: ZverGuy Date: Thu, 5 Nov 2026 01:38:47 +0700 Subject: [PATCH] fix(observability-proxy): cap the hits groups when fields_limit is absent The fields_limit cap applied only when the client sent the parameter. vlselect returns every group when it is absent, as it does for 0, so leaving it out bypassed the cap. The capped value is now always sent. Assisted-by: LLM Signed-off-by: ZverGuy --- internal/observabilityproxy/logsparams.go | 12 ++++++------ internal/observabilityproxy/logsparams_test.go | 7 ++++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/internal/observabilityproxy/logsparams.go b/internal/observabilityproxy/logsparams.go index 67462c989..93e1bd9a4 100644 --- a/internal/observabilityproxy/logsparams.go +++ b/internal/observabilityproxy/logsparams.go @@ -103,13 +103,13 @@ func BuildLogsParams(endpoint string, in url.Values, namespace string, now time. out.Add("field", f) } } - if in.Has("fields_limit") { - // setLimit reads an empty value as the default, which here is 0. - if in.Get("fields_limit") == "" { - return nil, errorf(http.StatusBadRequest, "invalid fields_limit \"\"") - } - herr = setLimit(out, in, "fields_limit", 0, lim.MaxLimit) + // vlselect returns every group without fields_limit or with 0, so + // the cap is sent either way; an empty value is malformed rather + // than a request for the default. + if in.Has("fields_limit") && in.Get("fields_limit") == "" { + return nil, errorf(http.StatusBadRequest, "invalid fields_limit \"\"") } + herr = setLimit(out, in, "fields_limit", lim.MaxLimit, lim.MaxLimit) case "stats_query_range": herr = setStep(out, in, start, end, lim) case "field_values", "stream_field_values": diff --git a/internal/observabilityproxy/logsparams_test.go b/internal/observabilityproxy/logsparams_test.go index 62fdc26ac..f39cc8846 100644 --- a/internal/observabilityproxy/logsparams_test.go +++ b/internal/observabilityproxy/logsparams_test.go @@ -116,7 +116,7 @@ func TestBuildLogsParamsRejects(t *testing.T) { code int }{ {"unknown endpoint", "tenant_ids", url.Values{}, http.StatusNotFound}, - // An empty value used to reach vlselect as fields_limit=0. + // vlselect reads fields_limit=0 as no limit on the groups. {"empty fields_limit", "hits", url.Values{"field": {"level"}, "fields_limit": {""}}, http.StatusBadRequest}, {"stats_query is not proxied", "stats_query", url.Values{}, http.StatusNotFound}, {"ignore_global_time_filter", "query", url.Values{"query": {`* options(ignore_global_time_filter=true)`}}, http.StatusBadRequest}, @@ -264,6 +264,11 @@ func TestBuildLogsParamsStepAndLimit(t *testing.T) { if facets.Get("max_values_per_field") != "5000" || facets.Get("max_value_len") != "5000" { t.Errorf("facets caps not clamped: %v", facets) } + // Without fields_limit vlselect returns every group, so the cap is + // sent even when the client sends none. + if got := build(t, "hits", url.Values{"field": {"level"}}).Get("fields_limit"); got != "5000" { + t.Errorf("hits without fields_limit forwards %q, want the 5000 cap", got) + } hits := build(t, "hits", url.Values{"field": {"b", "a"}, "fields_limit": {"99999"}}) fields := hits["field"] sort.Strings(fields)