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)