Skip to content

Commit

Permalink
feat(api)!: Fail log queries when executed on instant query endpoint (#…
Browse files Browse the repository at this point in the history
…13421)

### What this PR does / why we need it

**Background**

A log selector expression is a LogQL expression that returns logs, in contrast to a sample expressions, which returns metrics (samples). The simplest form of log selector expressions are label matchers, e.g. `{env="prod"}`.

**Change**

This PR changes the behaviour of Loki so that the instant query endpoint `/api/v1/query` does not allow sending a log selector expression as query any more. Instead, it returns a status code 400 (Bad Request) with the error message **"log queries are not supported as an instant query type, please change you query to a range query type"**.

**Why**
Previously this API endpoint allowed these types of log queries, but returned inconsistent results, which where a major cause for confusion. Returning a concise error helps the user understand that they likely selected the wrong query type in Grafana when executing the query.

---

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
  • Loading branch information
chaudum authored Jul 5, 2024
1 parent cf5f52d commit ce71f1c
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 13 deletions.
5 changes: 3 additions & 2 deletions docs/sources/reference/loki-http-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,9 @@ GET /loki/api/v1/query
```

`/loki/api/v1/query` allows for doing queries against a single point in time.
This type of query is often referred to as an instant query. Instant queries are mostly used for metric type LogQL queries.
It accepts the following query parameters in the URL:
This type of query is often referred to as an instant query. Instant queries are only used for metric type LogQL queries
and will return a 400 (Bad Request) in case a log type query is provided.
The endpoint accepts the following query parameters in the URL:

- `query`: The [LogQL]({{< relref "../query" >}}) query to perform. Requests that do not use valid LogQL syntax will return errors.
- `limit`: The max number of entries to return. It defaults to `100`. Only applies to query types which produce a stream (log lines) response.
Expand Down
2 changes: 1 addition & 1 deletion integration/loki_micro_services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,7 @@ func TestCategorizedLabels(t *testing.T) {
expectedEncodingFlags = tc.encodingFlags
}

resp, err := cliQueryFrontend.RunQuery(context.Background(), tc.query, headers...)
resp, err := cliQueryFrontend.RunRangeQuery(context.Background(), tc.query, headers...)
require.NoError(t, err)
assert.Equal(t, "streams", resp.Data.ResultType)

Expand Down
19 changes: 10 additions & 9 deletions pkg/logqlmodel/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@ import (
// Those errors are useful for comparing error returned by the engine.
// e.g. errors.Is(err,logqlmodel.ErrParse) let you know if this is a ast parsing error.
var (
ErrParse = errors.New("failed to parse the log query")
ErrPipeline = errors.New("failed execute pipeline")
ErrLimit = errors.New("limit reached while evaluating the query")
ErrIntervalLimit = errors.New("[interval] value exceeds limit")
ErrBlocked = errors.New("query blocked by policy")
ErrParseMatchers = errors.New("only label matchers are supported")
ErrorLabel = "__error__"
PreserveErrorLabel = "__preserve_error__"
ErrorDetailsLabel = "__error_details__"
ErrParse = errors.New("failed to parse the log query")
ErrPipeline = errors.New("failed execute pipeline")
ErrLimit = errors.New("limit reached while evaluating the query")
ErrIntervalLimit = errors.New("[interval] value exceeds limit")
ErrBlocked = errors.New("query blocked by policy")
ErrParseMatchers = errors.New("only label matchers are supported")
ErrUnsupportedSyntaxForInstantQuery = errors.New("log queries are not supported as an instant query type, please change your query to a range query type")
ErrorLabel = "__error__"
PreserveErrorLabel = "__preserve_error__"
ErrorDetailsLabel = "__error_details__"
)

// ParseError is what is returned when we failed to parse.
Expand Down
5 changes: 5 additions & 0 deletions pkg/querier/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ func (q *QuerierAPI) RangeQueryHandler(ctx context.Context, req *queryrange.Loki

// InstantQueryHandler is a http.HandlerFunc for instant queries.
func (q *QuerierAPI) InstantQueryHandler(ctx context.Context, req *queryrange.LokiInstantRequest) (logqlmodel.Result, error) {
// do not allow log selector expression (aka log query) as instant query
if _, ok := req.Plan.AST.(syntax.SampleExpr); !ok {
return logqlmodel.Result{}, logqlmodel.ErrUnsupportedSyntaxForInstantQuery
}

if err := q.validateMaxEntriesLimits(ctx, req.Plan.AST, req.Limit); err != nil {
return logqlmodel.Result{}, err
}
Expand Down
30 changes: 30 additions & 0 deletions pkg/querier/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/grafana/loki/v3/pkg/loghttp"
"github.com/grafana/loki/v3/pkg/logproto"
"github.com/grafana/loki/v3/pkg/logqlmodel"
"github.com/grafana/loki/v3/pkg/validation"

"github.com/go-kit/log"
Expand All @@ -21,6 +22,35 @@ import (
"github.com/stretchr/testify/require"
)

func TestInstantQueryHandler(t *testing.T) {
defaultLimits := defaultLimitsTestConfig()
limits, err := validation.NewOverrides(defaultLimits, nil)
require.NoError(t, err)

t.Run("log selector expression not allowed for instant queries", func(t *testing.T) {
api := NewQuerierAPI(mockQuerierConfig(), nil, limits, log.NewNopLogger())

ctx := user.InjectOrgID(context.Background(), "user")
req, err := http.NewRequestWithContext(ctx, "GET", `/api/v1/query`, nil)
require.NoError(t, err)

q := req.URL.Query()
q.Add("query", `{app="loki"}`)
req.URL.RawQuery = q.Encode()
err = req.ParseForm()
require.NoError(t, err)

rr := httptest.NewRecorder()

handler := NewQuerierHandler(api)
httpHandler := NewQuerierHTTPHandler(handler)

httpHandler.ServeHTTP(rr, req)
require.Equal(t, http.StatusBadRequest, rr.Code)
require.Equal(t, logqlmodel.ErrUnsupportedSyntaxForInstantQuery.Error(), rr.Body.String())
})
}

func TestTailHandler(t *testing.T) {
defaultLimits := defaultLimitsTestConfig()
limits, err := validation.NewOverrides(defaultLimits, nil)
Expand Down
7 changes: 6 additions & 1 deletion pkg/util/server/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,12 @@ func ClientHTTPStatusAndError(err error) (int, error) {
return http.StatusGatewayTimeout, errors.New(ErrDeadlineExceeded)
case errors.As(err, &queryErr):
return http.StatusBadRequest, err
case errors.Is(err, logqlmodel.ErrLimit) || errors.Is(err, logqlmodel.ErrParse) || errors.Is(err, logqlmodel.ErrPipeline) || errors.Is(err, logqlmodel.ErrBlocked) || errors.Is(err, logqlmodel.ErrParseMatchers):
case errors.Is(err, logqlmodel.ErrLimit) ||
errors.Is(err, logqlmodel.ErrParse) ||
errors.Is(err, logqlmodel.ErrPipeline) ||
errors.Is(err, logqlmodel.ErrBlocked) ||
errors.Is(err, logqlmodel.ErrParseMatchers) ||
errors.Is(err, logqlmodel.ErrUnsupportedSyntaxForInstantQuery):
return http.StatusBadRequest, err
case errors.Is(err, user.ErrNoOrgID):
return http.StatusBadRequest, err
Expand Down

0 comments on commit ce71f1c

Please sign in to comment.