From fb5049a7479ed28190bd90b4df2de6225fc1296f Mon Sep 17 00:00:00 2001 From: John Coffey Date: Thu, 13 Aug 2026 12:21:42 -0700 Subject: [PATCH] Phase 2: unified query language spanning ClickHouse and Tantivy Replaces the separate SQL-only /query and text-only /search endpoints with one pipe-syntax query language (plus raw SQL escape hatch) that compiles to a single IR and execution plan across both backends, so a query like `message:"connection refused" | stats count by host` runs as one request instead of two disjoint tools. - api/internal/querylang: lexer -> ast -> parser -> ir -> planner -> executor, each layer independently tested. - Execution generalizes Phase 1's proven Tantivy-prefilter pattern into a 4-way routing table (pure ClickHouse / text-only / text + aggregation / raw SQL passthrough). - Unified web query page and `sentryctl query`, both hitting the same POST /query endpoint. - Benchmarked against a real 1,022,000-row dataset (hack/benchmark-fixture); caught and fixed a real bug where the Tantivy prefilter cap (10,000) produced an IN-clause exceeding ClickHouse's default max_query_size -- lowered to 5,000, documented in docs/query-language-design.md and docs/phase-2-runbook.md. - docs/query-language-reference.md: customer-facing syntax reference. --- .gitignore | 1 + CLAUDE.md | 51 +- api/README.md | 81 ++-- api/cmd/api/main.go | 14 +- api/internal/queryapi/handler.go | 81 ++-- api/internal/queryapi/handler_test.go | 162 +++++-- api/internal/queryapi/search.go | 96 ---- api/internal/queryapi/search_test.go | 125 ----- api/internal/queryapi/validate.go | 47 -- api/internal/queryapi/validate_test.go | 38 -- api/internal/querylang/ast/ast.go | 91 ++++ .../executor/chrunner.go} | 26 +- api/internal/querylang/executor/executor.go | 72 +++ .../querylang/executor/executor_test.go | 309 ++++++++++++ api/internal/querylang/executor/sql.go | 228 +++++++++ api/internal/querylang/ir/ir.go | 80 ++++ api/internal/querylang/lexer/lexer.go | 238 +++++++++ api/internal/querylang/lexer/lexer_test.go | 140 ++++++ api/internal/querylang/parser/parser.go | 452 ++++++++++++++++++ api/internal/querylang/parser/parser_test.go | 278 +++++++++++ api/internal/querylang/planner/planner.go | 286 +++++++++++ .../querylang/planner/planner_test.go | 221 +++++++++ api/internal/searchclient/client.go | 6 +- cli/README.md | 23 +- cli/cmd/sentryctl/main.go | 170 ++++++- cli/cmd/sentryctl/main_test.go | 127 +++++ docs/phase-2-runbook.md | 178 +++++++ docs/query-language-design.md | 250 ++++++++++ docs/query-language-reference.md | 322 +++++++++++++ hack/benchmark-fixture/go.mod | 18 + hack/benchmark-fixture/go.sum | 38 ++ hack/benchmark-fixture/main.go | 172 +++++++ web/src/routes/+layout.svelte | 29 -- web/src/routes/+page.svelte | 182 ++++++- web/src/routes/search/+page.svelte | 96 ---- web/src/routes/search/+page.ts | 4 - 36 files changed, 4119 insertions(+), 613 deletions(-) delete mode 100644 api/internal/queryapi/search.go delete mode 100644 api/internal/queryapi/search_test.go delete mode 100644 api/internal/queryapi/validate.go delete mode 100644 api/internal/queryapi/validate_test.go create mode 100644 api/internal/querylang/ast/ast.go rename api/internal/{queryapi/executor.go => querylang/executor/chrunner.go} (62%) create mode 100644 api/internal/querylang/executor/executor.go create mode 100644 api/internal/querylang/executor/executor_test.go create mode 100644 api/internal/querylang/executor/sql.go create mode 100644 api/internal/querylang/ir/ir.go create mode 100644 api/internal/querylang/lexer/lexer.go create mode 100644 api/internal/querylang/lexer/lexer_test.go create mode 100644 api/internal/querylang/parser/parser.go create mode 100644 api/internal/querylang/parser/parser_test.go create mode 100644 api/internal/querylang/planner/planner.go create mode 100644 api/internal/querylang/planner/planner_test.go create mode 100644 docs/phase-2-runbook.md create mode 100644 docs/query-language-design.md create mode 100644 docs/query-language-reference.md create mode 100644 hack/benchmark-fixture/go.mod create mode 100644 hack/benchmark-fixture/go.sum create mode 100644 hack/benchmark-fixture/main.go delete mode 100644 web/src/routes/search/+page.svelte delete mode 100644 web/src/routes/search/+page.ts diff --git a/.gitignore b/.gitignore index b1c737d..feacbb1 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ search/target/ /api/api /cli/sentryctl /hack/windows-fixture/windows-fixture +/hack/benchmark-fixture/benchmark-fixture # Node / SvelteKit (web/ has its own more detailed .gitignore too) web/node_modules/ diff --git a/CLAUDE.md b/CLAUDE.md index 3410aab..d02d8c9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,21 +54,46 @@ dashboards — that discipline held for the whole phase. ## What "done" looks like for Phase 1 -A Windows Event Log entry and a Linux journald entry should both be -queryable via SQL (the ClickHouse path) and via free-text search (the -Tantivy path), from the same UI, within a few seconds of being generated. +**Status: shipped.** A Windows Event Log entry and a Linux journald entry +are both queryable via SQL (the ClickHouse path) and via free-text search +(the Tantivy path), from the same UI, within a few seconds of being +generated. Verified end-to-end on the live stack, including the same +`record_id` coming back from both query paths for the same record — see +`/docs/phase-1-runbook.md`. -Non-goals for this phase (same "resist scope creep" discipline as Phase 0): -no alerting, no dashboards, no SPL-like query layer, no multi-tenancy, and -no unified query experience — two separate boxes on two separate pages is -correct for Phase 1; unifying them is Phase 2's job. +ETW and WEF (Windows Event Forwarding) were *designed* in this phase but +not required to be running for "done": ETW ships behind a feature flag +most environments won't enable (it needs elevated privileges), and WEF's +receiver-side was explicitly deferred rather than built. Only the Event +Log source needed to actually be running end-to-end, and did. The +Windows-specific agent code itself (`EvtSubscribe`, ETW, service +registration) remains unverified on real Windows — no Windows toolchain +existed anywhere in the environment this was built in; flagged +prominently in `/agent/README.md` and the runbook. -ETW and WEF (Windows Event Forwarding) are *designed* in this phase but not -required to be running for "done": ETW ships behind a feature flag most -environments won't enable (it needs elevated privileges), and WEF's -receiver-side is explicitly deferred rather than built now — see -`/docs/phase-1-runbook.md` for both. Only the Event Log source needs to -actually be running end-to-end for this phase to count as done. +## What "done" looks like for Phase 2 + +A single query bar in the web UI and a single `sentryctl query` command +can express filter + free-text + stats in one query (e.g. `service=api | +where status>=500 | stats count by host | sort -count`, or +`message:"connection refused" | stats count by host`), execute correctly +against both ClickHouse and Tantivy in one compiled plan, and return in +well under a second for a 1M-row fixture dataset (rough benchmark, not a +formal SLA — see `/docs/phase-2-runbook.md` for the actual measurement). +Raw ClickHouse SQL remains available as an escape hatch, compiling to the +same execution plan/IR as the pipe syntax so performance doesn't depend +on which syntax a query uses. + +Non-goals for this phase (same "resist scope creep" discipline as every +phase so far): no alerting, no dashboards, no multi-tenancy — this phase +is the query layer only. The two separate placeholder pages/endpoints +from Phase 0/1 (`/query` raw-SQL-only, `/search` free-text-only) are +retired, replaced by one `/query` endpoint and one query page. + +See `/docs/query-language-design.md` for the grammar, IR, and +ClickHouse/Tantivy routing strategy, and +`/docs/query-language-reference.md` for the user-facing syntax reference +once built. ## When in doubt Ask before: changing the pinned stack, adding a new external dependency diff --git a/api/README.md b/api/README.md index c674f36..3980cf1 100644 --- a/api/README.md +++ b/api/README.md @@ -1,39 +1,39 @@ # api -Sentry's query API: two intentionally crude endpoints — raw SQL and -free-text search — that Phase 2's real query layer replaces outright. +Sentry's query API: a single `POST /query` endpoint accepting either the +pipe syntax or raw SQL, compiled and routed across ClickHouse and Tantivy +by `internal/querylang`. Replaces Phase 0/1's two separate placeholder +endpoints (raw-SQL-only `/query`, free-text-only `/search`) — see +`/docs/query-language-design.md` for the grammar, IR, and routing design, +and `/docs/query-language-reference.md` for the user-facing syntax. ## Why plain REST, not gRPC + REST gateway CLAUDE.md pins the control plane to "Go, gRPC + REST gateway." This service is plain `net/http` instead — a deliberate simplification, not a -change to the pinned stack. Wiring up `.proto` services, +change to the pinned stack. Wiring up a `.proto` service, `google.api.http` annotations, and `protoc-gen-grpc-gateway` codegen for -two endpoints that Phase 2 replaces outright with a real SPL-like query -layer would be exactly the kind of premature machinery this project's -conventions warn against. `api` *does* speak gRPC internally though — to -`/search` (see below) — this simplification is specifically about the -public-facing surface, not a blanket avoidance of gRPC. +one endpoint doesn't buy much at this size. `api` *does* speak gRPC +internally — to `/search` — this simplification is about the +public-facing surface only. -## Endpoints +## Endpoint -- `POST /query` — body `{"sql": "SELECT ..."}`, response - `{"columns": [...], "rows": [[...], ...]}` or `{"error": "..."}`. - SELECT-only, single-statement, basic keyword-based injection guarding - (see `internal/queryapi/validate.go` for exactly what that does and - doesn't catch — it's not a SQL parser). -- `POST /search` — body `{"query": "...", "limit": 100}`, same response - shape as `/query`. Calls `/search`'s `SearchService.Search` gRPC RPC to - resolve the free-text query into matching `record_id`s, then joins - those back against ClickHouse (`SELECT * FROM logs WHERE record_id IN - (...)`) to return full rows — so both endpoints return the same - `{columns, rows}` shape and `/web` can reuse one table component for - both. Every `record_id` is validated as a real UUID before being - embedded in the generated SQL (defense in depth: `record_id`s come from - an internal, trusted service, not raw user input, but a value that - fails to parse as a UUID can't contain SQL-breaking characters either - way). -- `GET /healthz` — for docker-compose/k8s liveness checks. +`POST /query` — body `{"query": "...", "language": ""}`, response +`{"columns": [...], "rows": [[...], ...]}` or `{"error": "..."}`. + +- `query` is either pipe syntax (`service=api | where status>=500 | + stats count by host`) or raw SQL (`SELECT ...`). Auto-detected by + whether the query starts with `SELECT` (case-insensitive). +- `language` optionally overrides detection: `"sql"` or `"spl"`. Exists + for the rare case a pipe query legitimately starts with the literal + word "select" as a bare search term. +- Both syntaxes compile to the same `querylang/ir.Plan` and execute + through the same code path — see `internal/querylang/executor` for the + four routing cases (pure ClickHouse; Tantivy prefilter + ClickHouse + rows; Tantivy prefilter + ClickHouse aggregation; raw SQL passthrough). + +`GET /healthz` — for docker-compose/k8s liveness checks. No auth. Not scoped yet — don't expose this beyond a trusted dev/homelab network. @@ -48,7 +48,7 @@ Environment variables (see `internal/config/config.go`): | `CLICKHOUSE_ADDR` | `localhost:9000` | Native protocol port | | `CLICKHOUSE_DATABASE` / `_USERNAME` / `_PASSWORD` | `sentry` / `default` / `` | | | `SEARCH_GRPC_ADDR` | `localhost:50052` | Must match `/search`'s `GRPC_LISTEN_ADDR` | -| `QUERY_TIMEOUT_SECONDS` | `30` | Per-request timeout, both endpoints | +| `QUERY_TIMEOUT_SECONDS` | `30` | Per-request timeout | | `CORS_ALLOWED_ORIGIN` | `*` | Wide open by default since there's no auth yet; tighten together | `searchclient.Dial` connects to `/search` over plain TCP, no TLS — same @@ -71,15 +71,18 @@ docker build -f api/Dockerfile -t sentry-api . ## Testing notes -`internal/queryapi`'s HTTP handlers depend on ClickHouse and `/search` -only through narrow interfaces (`queryExecutor`, `searchClient`), so -routing, validation, JSON encoding, error-status mapping, and the -record_id-to-SQL query building are all unit-tested against fakes — no -live ClickHouse or `/search` instance needed. `Executor` itself (the -reflection-based row scanning against `driver.Rows`) and -`internal/searchclient`'s actual gRPC dial are not unit-tested — the -former because faking ClickHouse's `driver.Rows` interface fully would be -significant test-only scaffolding the driver's own docs say isn't meant -to be implemented by adopters; the latter because it's a thin wrapper -with nothing but wiring to test. Both are exercised end-to-end via the -docker-compose flow in `/docs/phase-1-runbook.md` instead. +`internal/queryapi`'s HTTP handler depends on ClickHouse and `/search` +only through the narrow interfaces `querylang/executor` defines +(`SQLRunner`, `SearchClient`), so routing, compilation, JSON encoding, +and error-status mapping are all unit-tested against fakes — no live +ClickHouse or `/search` instance needed, and the real lexer/parser/ +planner run unmocked in these tests, only the backends are faked. See +`internal/querylang`'s own package docs for how compilation and +execution are tested independently of each other. `executor.ChRunner` +(the reflection-based row scanning against ClickHouse's `driver.Rows`) +and `internal/searchclient`'s actual gRPC dial are not unit-tested — the +former because faking `driver.Rows` fully would be significant +test-only scaffolding the driver's own docs say isn't meant to be +implemented by adopters; the latter because it's a thin wrapper with +nothing but wiring to test. Both are exercised end-to-end via the +docker-compose flow in `/docs/phase-2-runbook.md`. diff --git a/api/cmd/api/main.go b/api/cmd/api/main.go index 04c837d..7d7ce1a 100644 --- a/api/cmd/api/main.go +++ b/api/cmd/api/main.go @@ -1,7 +1,8 @@ -// Command api is Sentry's query API: POST /query (raw SQL, SELECT-only) -// and POST /search (free-text, via the search service). See -// internal/queryapi for why these are plain REST rather than the pinned -// gRPC+gateway pattern. +// Command api is Sentry's query API: a single POST /query endpoint +// accepting either the pipe syntax or raw SQL, compiled and routed +// across ClickHouse and search by internal/querylang. See +// internal/queryapi and /docs/query-language-design.md for why this is +// plain REST rather than the pinned gRPC+gateway pattern. package main import ( @@ -16,6 +17,7 @@ import ( "github.com/ClickHouse/clickhouse-go/v2" "github.com/sentry/sentry/api/internal/config" + "github.com/sentry/sentry/api/internal/querylang/executor" "github.com/sentry/sentry/api/internal/queryapi" "github.com/sentry/sentry/api/internal/searchclient" ) @@ -58,8 +60,8 @@ func main() { } defer search.Close() - exec := queryapi.NewExecutor(conn) - handler := queryapi.NewHandler(logger, exec, search, cfg.QueryTimeout, cfg.CORSAllowedOrigin) + sqlRunner := executor.NewChRunner(conn) + handler := queryapi.NewHandler(logger, sqlRunner, search, cfg.QueryTimeout, cfg.CORSAllowedOrigin) srv := &http.Server{ Addr: cfg.HTTPListenAddr, diff --git a/api/internal/queryapi/handler.go b/api/internal/queryapi/handler.go index 93143cc..462b7f6 100644 --- a/api/internal/queryapi/handler.go +++ b/api/internal/queryapi/handler.go @@ -1,13 +1,14 @@ -// Package queryapi is Sentry's query API: POST /query (Phase 0, a crude -// raw-SQL passthrough allowlisted to SELECT) and POST /search (Phase 1, -// free-text search via the search service, joined back against -// ClickHouse). This is a deliberate simplification of the pinned "gRPC + -// REST gateway" control-plane pattern (see CLAUDE.md's tech stack table): -// plain net/http REST handlers, not a gRPC service transcoded through -// grpc-gateway. That machinery (proto definitions, googleapis -// annotations, gateway codegen) doesn't buy much for two crude endpoints -// that Phase 2's real SPL-like query layer replaces outright. Revisit -// gRPC+gateway once /api's endpoint count and lifespan justify it. +// Package queryapi is Sentry's query API: a single POST /query endpoint +// accepting either the pipe syntax or raw SQL, compiled by +// querylang/planner and executed by querylang/executor. Replaces Phase +// 0/1's two separate placeholder endpoints (raw-SQL-only /query, +// free-text-only /search) -- see /docs/query-language-design.md. +// +// Still plain net/http, not the pinned gRPC+REST-gateway pattern, for +// the same reason as Phase 0/1: this is one endpoint, and the +// proto/annotations/codegen machinery doesn't buy much at that size. +// `/api` does speak gRPC internally (to /search) — this simplification +// is about the public-facing surface only. package queryapi import ( @@ -15,38 +16,34 @@ import ( "encoding/json" "log/slog" "net/http" + "strings" "time" -) -// queryExecutor is the narrow interface handleQuery depends on, so tests -// can substitute a fake without a real ClickHouse connection. *Executor -// satisfies it. -type queryExecutor interface { - Execute(ctx context.Context, sql string) (*QueryResult, error) -} + "github.com/sentry/sentry/api/internal/querylang/executor" + "github.com/sentry/sentry/api/internal/querylang/planner" +) type Handler struct { logger *slog.Logger - exec queryExecutor - search searchClient + sqlRunner executor.SQLRunner + search executor.SearchClient queryTimeout time.Duration allowedOrigin string } -func NewHandler(logger *slog.Logger, exec queryExecutor, search searchClient, queryTimeout time.Duration, allowedOrigin string) *Handler { - return &Handler{logger: logger, exec: exec, search: search, queryTimeout: queryTimeout, allowedOrigin: allowedOrigin} +func NewHandler(logger *slog.Logger, sqlRunner executor.SQLRunner, search executor.SearchClient, queryTimeout time.Duration, allowedOrigin string) *Handler { + return &Handler{logger: logger, sqlRunner: sqlRunner, search: search, queryTimeout: queryTimeout, allowedOrigin: allowedOrigin} } func (h *Handler) Routes() http.Handler { mux := http.NewServeMux() mux.HandleFunc("POST /query", h.handleQuery) - mux.HandleFunc("POST /search", h.handleSearch) mux.HandleFunc("GET /healthz", h.handleHealthz) return h.withCORS(mux) } // withCORS is deliberately permissive by default (see CORSAllowedOrigin in -// internal/config) since Phase 0 has no auth and the SvelteKit dev server +// internal/config) since there's no auth yet and the SvelteKit dev server // runs on a different origin. Tighten alongside adding real auth. func (h *Handler) withCORS(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -66,14 +63,24 @@ func (h *Handler) handleHealthz(w http.ResponseWriter, _ *http.Request) { } type queryRequest struct { - SQL string `json:"sql"` + Query string `json:"query"` + // Language overrides auto-detection ("" / omitted). "sql" or "spl" -- + // see planner.Language and /docs/query-language-design.md's + // "Detection" section for why this exists: the rare case a pipe + // query legitimately starts with the literal word "select". + Language string `json:"language"` +} + +type queryResponse struct { + Columns []string `json:"columns"` + Rows [][]any `json:"rows"` } type errorResponse struct { Error string `json:"error"` } -// maxBodyBytes caps the request body: a raw SQL string has no legitimate +// maxBodyBytes caps the request body: a query string has no legitimate // reason to be larger than this. const maxBodyBytes = 1 << 20 // 1 MiB @@ -85,8 +92,19 @@ func (h *Handler) handleQuery(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "invalid JSON body: "+err.Error()) return } + if strings.TrimSpace(req.Query) == "" { + writeError(w, http.StatusBadRequest, "query must not be empty") + return + } - if err := validateSelectOnly(req.SQL); err != nil { + lang := planner.Language(req.Language) + if lang != planner.Auto && lang != planner.SQL && lang != planner.SPL { + writeError(w, http.StatusBadRequest, `language must be "sql", "spl", or omitted`) + return + } + + plan, err := planner.Compile(req.Query, lang, time.Now()) + if err != nil { writeError(w, http.StatusBadRequest, err.Error()) return } @@ -94,14 +112,19 @@ func (h *Handler) handleQuery(w http.ResponseWriter, r *http.Request) { ctx, cancel := context.WithTimeout(r.Context(), h.queryTimeout) defer cancel() - result, err := h.exec.Execute(ctx, req.SQL) + result, err := executor.Execute(ctx, plan, h.sqlRunner, h.search) if err != nil { - h.logger.Error("query execution failed", "error", err) + h.logger.Error("query execution failed", "query", req.Query, "error", err) writeError(w, http.StatusBadGateway, "query failed: "+err.Error()) return } - writeJSON(w, result) + writeJSON(w, queryResponse{Columns: result.Columns, Rows: result.Rows}) +} + +func writeJSON(w http.ResponseWriter, v any) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(v) } func writeError(w http.ResponseWriter, status int, msg string) { diff --git a/api/internal/queryapi/handler_test.go b/api/internal/queryapi/handler_test.go index 82e487f..b84e23b 100644 --- a/api/internal/queryapi/handler_test.go +++ b/api/internal/queryapi/handler_test.go @@ -11,111 +11,179 @@ import ( "strings" "testing" "time" + + "github.com/sentry/sentry/api/internal/querylang/executor" ) -type fakeExecutor struct { - result *QueryResult +type fakeSQLRunner struct { + result *executor.Result err error gotSQL string } -func (f *fakeExecutor) Execute(_ context.Context, sql string) (*QueryResult, error) { +func (f *fakeSQLRunner) RunSQL(_ context.Context, sql string) (*executor.Result, error) { f.gotSQL = sql if f.err != nil { return nil, f.err } - return f.result, nil + if f.result != nil { + return f.result, nil + } + return &executor.Result{Columns: []string{}, Rows: [][]any{}}, nil } type fakeSearchClient struct { recordIDs []string err error + gotQuery string } -func (f *fakeSearchClient) Search(_ context.Context, _ string, _ uint32) ([]string, error) { +func (f *fakeSearchClient) Search(_ context.Context, query string, _ uint32) ([]string, error) { + f.gotQuery = query if f.err != nil { return nil, f.err } return f.recordIDs, nil } -func newTestHandler(exec queryExecutor) *Handler { - return newTestHandlerWithSearch(exec, &fakeSearchClient{}) +func newTestHandler(sqlRunner *fakeSQLRunner, search *fakeSearchClient) *Handler { + if search == nil { + search = &fakeSearchClient{} + } + return NewHandler(slog.New(slog.NewTextHandler(io.Discard, nil)), sqlRunner, search, time.Second, "*") } -func newTestHandlerWithSearch(exec queryExecutor, search searchClient) *Handler { - return NewHandler(slog.New(slog.NewTextHandler(io.Discard, nil)), exec, search, time.Second, "*") +func postQuery(t *testing.T, h *Handler, body string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/query", strings.NewReader(body)) + rec := httptest.NewRecorder() + h.Routes().ServeHTTP(rec, req) + return rec } -func TestHandleQuerySuccess(t *testing.T) { - fe := &fakeExecutor{result: &QueryResult{ +func TestHandleQuerySQLSuccess(t *testing.T) { + sr := &fakeSQLRunner{result: &executor.Result{ Columns: []string{"host", "count"}, Rows: [][]any{{"h1", 3}}, }} - h := newTestHandler(fe) + h := newTestHandler(sr, nil) - body := strings.NewReader(`{"sql": "SELECT host, count(*) FROM logs GROUP BY host"}`) - req := httptest.NewRequest(http.MethodPost, "/query", body) - rec := httptest.NewRecorder() - - h.Routes().ServeHTTP(rec, req) + rec := postQuery(t, h, `{"query": "SELECT host, count(*) FROM logs GROUP BY host"}`) if rec.Code != http.StatusOK { t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) } - var got QueryResult + var got queryResponse if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { t.Fatalf("decoding response: %v", err) } if len(got.Columns) != 2 || len(got.Rows) != 1 { t.Fatalf("unexpected result: %+v", got) } - if fe.gotSQL != "SELECT host, count(*) FROM logs GROUP BY host" { - t.Fatalf("executor received unexpected SQL: %q", fe.gotSQL) + if sr.gotSQL != "SELECT host, count(*) FROM logs GROUP BY host" { + t.Fatalf("unexpected SQL passed through: %q", sr.gotSQL) } } -func TestHandleQueryRejectsNonSelect(t *testing.T) { - fe := &fakeExecutor{} - h := newTestHandler(fe) +func TestHandleQueryPipeSyntaxSuccess(t *testing.T) { + sr := &fakeSQLRunner{result: &executor.Result{ + Columns: []string{"host"}, + Rows: [][]any{{"api"}}, + }} + h := newTestHandler(sr, nil) - body := strings.NewReader(`{"sql": "DELETE FROM logs"}`) - req := httptest.NewRequest(http.MethodPost, "/query", body) - rec := httptest.NewRecorder() + rec := postQuery(t, h, `{"query": "service=api"}`) - h.Routes().ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + if !strings.Contains(sr.gotSQL, "`service` = 'api'") { + t.Fatalf("expected compiled SQL to filter on service, got: %s", sr.gotSQL) + } +} +func TestHandleQueryTextSearchRoutesThroughSearchClient(t *testing.T) { + sr := &fakeSQLRunner{} + fs := &fakeSearchClient{recordIDs: []string{"id-1"}} + h := newTestHandler(sr, fs) + + rec := postQuery(t, h, `{"query": "message:\"connection refused\""}`) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + if fs.gotQuery != `"connection refused"` { + t.Fatalf("search query = %q", fs.gotQuery) + } + if !strings.Contains(sr.gotSQL, "record_id IN ('id-1')") { + t.Fatalf("expected the search prefilter in the generated SQL, got: %s", sr.gotSQL) + } +} + +func TestHandleQueryRejectsEmptyQuery(t *testing.T) { + h := newTestHandler(&fakeSQLRunner{}, nil) + rec := postQuery(t, h, `{"query": " "}`) if rec.Code != http.StatusBadRequest { t.Fatalf("status = %d, want 400", rec.Code) } - if fe.gotSQL != "" { - t.Fatal("executor should not have been called for a rejected query") - } } func TestHandleQueryRejectsInvalidJSON(t *testing.T) { - h := newTestHandler(&fakeExecutor{}) - - body := strings.NewReader(`not json`) - req := httptest.NewRequest(http.MethodPost, "/query", body) - rec := httptest.NewRecorder() - - h.Routes().ServeHTTP(rec, req) - + h := newTestHandler(&fakeSQLRunner{}, nil) + rec := postQuery(t, h, `not json`) if rec.Code != http.StatusBadRequest { t.Fatalf("status = %d, want 400", rec.Code) } } +func TestHandleQueryRejectsCompileError(t *testing.T) { + h := newTestHandler(&fakeSQLRunner{}, nil) + rec := postQuery(t, h, `{"query": "service=api | bogus"}`) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400; body=%s", rec.Code, rec.Body.String()) + } +} + +func TestHandleQueryRejectsNonSelectSQL(t *testing.T) { + h := newTestHandler(&fakeSQLRunner{}, nil) + rec := postQuery(t, h, `{"query": "DELETE FROM logs", "language": "sql"}`) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } +} + +func TestHandleQueryRejectsInvalidLanguage(t *testing.T) { + h := newTestHandler(&fakeSQLRunner{}, nil) + rec := postQuery(t, h, `{"query": "service=api", "language": "cobol"}`) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } +} + +func TestHandleQueryExplicitLanguageOverridesAutoDetect(t *testing.T) { + sr := &fakeSQLRunner{} + fs := &fakeSearchClient{recordIDs: []string{"id-1"}} + h := newTestHandler(sr, fs) + + // "select" alone would auto-detect as (nonsensical but + // syntactically-valid-looking) SQL without the override -- the + // override forces pipe-syntax parsing instead, where a bare word + // with no comparator is a free-text search term. + rec := postQuery(t, h, `{"query": "select", "language": "spl"}`) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + if fs.gotQuery != "select" { + t.Fatalf("expected 'select' to be treated as a free-text search term, got query=%q", fs.gotQuery) + } +} + func TestHandleQueryExecutorErrorReturnsBadGateway(t *testing.T) { - fe := &fakeExecutor{err: errors.New("boom")} - h := newTestHandler(fe) + sr := &fakeSQLRunner{err: errors.New("boom")} + h := newTestHandler(sr, nil) - body := strings.NewReader(`{"sql": "SELECT 1"}`) - req := httptest.NewRequest(http.MethodPost, "/query", body) - rec := httptest.NewRecorder() - - h.Routes().ServeHTTP(rec, req) + rec := postQuery(t, h, `{"query": "SELECT 1"}`) if rec.Code != http.StatusBadGateway { t.Fatalf("status = %d, want 502", rec.Code) @@ -123,7 +191,7 @@ func TestHandleQueryExecutorErrorReturnsBadGateway(t *testing.T) { } func TestHandleHealthz(t *testing.T) { - h := newTestHandler(&fakeExecutor{}) + h := newTestHandler(&fakeSQLRunner{}, nil) req := httptest.NewRequest(http.MethodGet, "/healthz", nil) rec := httptest.NewRecorder() @@ -135,7 +203,7 @@ func TestHandleHealthz(t *testing.T) { } func TestCORSPreflight(t *testing.T) { - h := newTestHandler(&fakeExecutor{}) + h := newTestHandler(&fakeSQLRunner{}, nil) req := httptest.NewRequest(http.MethodOptions, "/query", nil) rec := httptest.NewRecorder() diff --git a/api/internal/queryapi/search.go b/api/internal/queryapi/search.go deleted file mode 100644 index 40e0d89..0000000 --- a/api/internal/queryapi/search.go +++ /dev/null @@ -1,96 +0,0 @@ -package queryapi - -import ( - "context" - "encoding/json" - "fmt" - "net/http" - "strings" - - "github.com/google/uuid" -) - -// searchClient is the narrow interface handleSearch depends on, so tests -// can substitute a fake without a real search service. A small gRPC -// adapter in cmd/api satisfies this. -type searchClient interface { - Search(ctx context.Context, query string, limit uint32) ([]string, error) -} - -type searchRequest struct { - Query string `json:"query"` - Limit uint32 `json:"limit"` -} - -func (h *Handler) handleSearch(w http.ResponseWriter, r *http.Request) { - r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes) - - var req searchRequest - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - writeError(w, http.StatusBadRequest, "invalid JSON body: "+err.Error()) - return - } - if strings.TrimSpace(req.Query) == "" { - writeError(w, http.StatusBadRequest, "query must not be empty") - return - } - - ctx, cancel := context.WithTimeout(r.Context(), h.queryTimeout) - defer cancel() - - recordIDs, err := h.search.Search(ctx, req.Query, req.Limit) - if err != nil { - h.logger.Error("search failed", "error", err) - writeError(w, http.StatusBadGateway, "search failed: "+err.Error()) - return - } - - if len(recordIDs) == 0 { - writeJSON(w, &QueryResult{Columns: []string{}, Rows: [][]any{}}) - return - } - - sql, err := recordIDsQuery(recordIDs) - if err != nil { - h.logger.Error("building record_id query", "error", err) - writeError(w, http.StatusBadGateway, "search returned unusable results") - return - } - - result, err := h.exec.Execute(ctx, sql) - if err != nil { - h.logger.Error("joining search results against clickhouse failed", "error", err) - writeError(w, http.StatusBadGateway, "query failed: "+err.Error()) - return - } - - writeJSON(w, result) -} - -// recordIDsQuery builds a SELECT ... WHERE record_id IN (...) against the -// IDs the search service returned. Every ID is validated as a real UUID -// before being embedded in the query string -- record_ids come from an -// internal, trusted service (not raw user input), but a UUID that fails -// to parse can't contain SQL-breaking characters either way, so this is -// defense in depth, not a response to a specific threat. -func recordIDsQuery(recordIDs []string) (string, error) { - quoted := make([]string, 0, len(recordIDs)) - for _, id := range recordIDs { - if _, err := uuid.Parse(id); err != nil { - continue // skip anything not a valid UUID rather than failing the whole query - } - quoted = append(quoted, "'"+id+"'") - } - if len(quoted) == 0 { - return "", fmt.Errorf("no valid record_ids in search response") - } - return fmt.Sprintf( - "SELECT * FROM logs WHERE record_id IN (%s) ORDER BY timestamp DESC", - strings.Join(quoted, ","), - ), nil -} - -func writeJSON(w http.ResponseWriter, v any) { - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(v) -} diff --git a/api/internal/queryapi/search_test.go b/api/internal/queryapi/search_test.go deleted file mode 100644 index 5a76c0e..0000000 --- a/api/internal/queryapi/search_test.go +++ /dev/null @@ -1,125 +0,0 @@ -package queryapi - -import ( - "encoding/json" - "errors" - "net/http" - "net/http/httptest" - "strings" - "testing" -) - -func TestHandleSearchSuccess(t *testing.T) { - id := "5754b062-ec8b-45b1-b1b8-a50f263adcd3" - fe := &fakeExecutor{result: &QueryResult{ - Columns: []string{"message"}, - Rows: [][]any{{"hello world"}}, - }} - fs := &fakeSearchClient{recordIDs: []string{id}} - h := newTestHandlerWithSearch(fe, fs) - - body := strings.NewReader(`{"query": "hello"}`) - req := httptest.NewRequest(http.MethodPost, "/search", body) - rec := httptest.NewRecorder() - - h.Routes().ServeHTTP(rec, req) - - if rec.Code != http.StatusOK { - t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) - } - if !strings.Contains(fe.gotSQL, id) { - t.Fatalf("expected the record_id in the generated SQL, got %q", fe.gotSQL) - } - if !strings.Contains(fe.gotSQL, "WHERE record_id IN") { - t.Fatalf("expected an IN clause, got %q", fe.gotSQL) - } - - var got QueryResult - if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { - t.Fatalf("decoding response: %v", err) - } - if len(got.Rows) != 1 { - t.Fatalf("unexpected result: %+v", got) - } -} - -func TestHandleSearchRejectsEmptyQuery(t *testing.T) { - fe := &fakeExecutor{} - fs := &fakeSearchClient{} - h := newTestHandlerWithSearch(fe, fs) - - body := strings.NewReader(`{"query": " "}`) - req := httptest.NewRequest(http.MethodPost, "/search", body) - rec := httptest.NewRecorder() - - h.Routes().ServeHTTP(rec, req) - - if rec.Code != http.StatusBadRequest { - t.Fatalf("status = %d, want 400", rec.Code) - } - if fe.gotSQL != "" { - t.Fatal("executor should not have been called for an empty query") - } -} - -func TestHandleSearchNoResultsReturnsEmptyNotError(t *testing.T) { - fe := &fakeExecutor{} - fs := &fakeSearchClient{recordIDs: nil} - h := newTestHandlerWithSearch(fe, fs) - - body := strings.NewReader(`{"query": "nothing matches this"}`) - req := httptest.NewRequest(http.MethodPost, "/search", body) - rec := httptest.NewRecorder() - - h.Routes().ServeHTTP(rec, req) - - if rec.Code != http.StatusOK { - t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) - } - if fe.gotSQL != "" { - t.Fatal("executor should not have been called when search returns no IDs") - } - - var got QueryResult - if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { - t.Fatalf("decoding response: %v", err) - } - if len(got.Rows) != 0 { - t.Fatalf("expected empty rows, got %+v", got.Rows) - } -} - -func TestHandleSearchServiceErrorReturnsBadGateway(t *testing.T) { - fe := &fakeExecutor{} - fs := &fakeSearchClient{err: errors.New("search service unreachable")} - h := newTestHandlerWithSearch(fe, fs) - - body := strings.NewReader(`{"query": "hello"}`) - req := httptest.NewRequest(http.MethodPost, "/search", body) - rec := httptest.NewRecorder() - - h.Routes().ServeHTTP(rec, req) - - if rec.Code != http.StatusBadGateway { - t.Fatalf("status = %d, want 502", rec.Code) - } -} - -func TestRecordIDsQuerySkipsInvalidUUIDs(t *testing.T) { - sql, err := recordIDsQuery([]string{"not-a-uuid", "5754b062-ec8b-45b1-b1b8-a50f263adcd3"}) - if err != nil { - t.Fatalf("recordIDsQuery() error = %v", err) - } - if strings.Contains(sql, "not-a-uuid") { - t.Fatalf("expected the invalid UUID to be skipped, got %q", sql) - } - if !strings.Contains(sql, "5754b062-ec8b-45b1-b1b8-a50f263adcd3") { - t.Fatalf("expected the valid UUID to be included, got %q", sql) - } -} - -func TestRecordIDsQueryAllInvalidReturnsError(t *testing.T) { - if _, err := recordIDsQuery([]string{"not-a-uuid", "also-not-one"}); err == nil { - t.Fatal("expected an error when no IDs are valid UUIDs") - } -} diff --git a/api/internal/queryapi/validate.go b/api/internal/queryapi/validate.go deleted file mode 100644 index 4649e30..0000000 --- a/api/internal/queryapi/validate.go +++ /dev/null @@ -1,47 +0,0 @@ -package queryapi - -import ( - "errors" - "regexp" - "strings" -) - -// disallowedKeyword is defense-in-depth on top of the SELECT-only gate: it -// catches mutating/administrative statements appearing anywhere in the -// query (e.g. smuggled into a subquery), not just at the start. This is -// word-boundary matching, not a real SQL parser. -var disallowedKeyword = regexp.MustCompile(`(?i)\b(insert|update|delete|alter|drop|truncate|create|grant|revoke|attach|detach|rename|kill|optimize|system|set|exchange|watch)\b`) - -// validateSelectOnly enforces the Phase 0 query API contract: exactly one -// SELECT statement and nothing else. This is "basic injection guarding" as -// specced, not a SQL parser: it will reject some unusual-but-valid SELECTs -// (e.g. one that references a column literally named "delete") and will -// not catch every possible abuse (e.g. a syntactically pure SELECT that's -// simply expensive to run). Both are acceptable for a Phase 0 placeholder -// that's explicitly superseded by a real query layer in Phase 2 — see -// /docs/architecture.md. -func validateSelectOnly(sql string) error { - trimmed := strings.TrimSpace(sql) - if trimmed == "" { - return errors.New("query must not be empty") - } - - trimmed = strings.TrimSpace(strings.TrimSuffix(trimmed, ";")) - if trimmed == "" { - return errors.New("query must not be empty") - } - if strings.Contains(trimmed, ";") { - return errors.New("only a single statement is allowed") - } - - firstWord := strings.ToUpper(strings.Fields(trimmed)[0]) - if firstWord != "SELECT" { - return errors.New("only SELECT queries are allowed") - } - - if disallowedKeyword.MatchString(trimmed) { - return errors.New("query contains a disallowed keyword") - } - - return nil -} diff --git a/api/internal/queryapi/validate_test.go b/api/internal/queryapi/validate_test.go deleted file mode 100644 index 5999f23..0000000 --- a/api/internal/queryapi/validate_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package queryapi - -import "testing" - -func TestValidateSelectOnly(t *testing.T) { - cases := []struct { - name string - sql string - wantErr bool - }{ - {"plain select", "SELECT * FROM logs LIMIT 10", false}, - {"lowercase select", "select service, count(*) from logs group by service", false}, - {"trailing semicolon allowed", "SELECT 1;", false}, - {"trailing semicolon and whitespace allowed", "SELECT 1; ", false}, - {"empty", "", true}, - {"whitespace only", " ", true}, - {"only a semicolon", ";", true}, - {"multiple statements", "SELECT 1; SELECT 2", true}, - {"insert", "INSERT INTO logs VALUES (1)", true}, - {"delete", "DELETE FROM logs", true}, - {"drop", "DROP TABLE logs", true}, - {"select with drop keyword smuggled in", "SELECT * FROM logs WHERE message = 'DROP TABLE logs'", true}, - {"non-select start", "WITH x AS (SELECT 1) SELECT * FROM x", true}, - {"trailing garbage after semicolon", "SELECT 1; DROP TABLE logs", true}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - err := validateSelectOnly(tc.sql) - if tc.wantErr && err == nil { - t.Errorf("validateSelectOnly(%q) = nil, want error", tc.sql) - } - if !tc.wantErr && err != nil { - t.Errorf("validateSelectOnly(%q) = %v, want nil", tc.sql, err) - } - }) - } -} diff --git a/api/internal/querylang/ast/ast.go b/api/internal/querylang/ast/ast.go new file mode 100644 index 0000000..f4f3e77 --- /dev/null +++ b/api/internal/querylang/ast/ast.go @@ -0,0 +1,91 @@ +// Package ast defines the parsed pipe-syntax tree. Internal to +// querylang -- not exposed outside it. See +// /docs/query-language-design.md for the grammar this mirrors. +package ast + +// Query is `base_search ("|" pipe_stage)*`. +type Query struct { + Base BoolExpr + Pipes []PipeStage +} + +// PipeStage is one of WhereStage, StatsStage, SortStage, FieldsStage, +// HeadStage, TailStage. +type PipeStage interface{ isPipeStage() } + +type WhereStage struct{ Expr BoolExpr } +type StatsStage struct { + Aggs []AggCall + By []string +} +type SortStage struct{ Fields []SortField } +type FieldsStage struct{ Fields []string } +type HeadStage struct { + N int + HasN bool // false => default limit, decided by the planner +} +type TailStage struct { + N int + HasN bool +} + +func (WhereStage) isPipeStage() {} +func (StatsStage) isPipeStage() {} +func (SortStage) isPipeStage() {} +func (FieldsStage) isPipeStage() {} +func (HeadStage) isPipeStage() {} +func (TailStage) isPipeStage() {} + +// BoolExpr is a sequence of terms joined by "and"/"or". An empty Conjs +// entry between two terms (i.e. no explicit keyword in the source) means +// implicit AND -- SPL's convention for adjacent bare search terms, e.g. +// `error timeout` means `error AND timeout`. Conjs has len(Terms)-1 +// elements once Terms has more than one. +type BoolExpr struct { + Terms []Term + Conjs []string // "and" | "or", one per gap between consecutive Terms +} + +// Term is one of Comparison, TimeBound, FreeText. +type Term interface{ isTerm() } + +type Comparison struct { + Field string + Op string // "=", "!=", ">", ">=", "<", "<=" + Value string +} + +type TimeBound struct { + Kind string // "earliest" | "latest" + Expr TimeExpr +} + +type FreeText struct { + Query string +} + +func (Comparison) isTerm() {} +func (TimeBound) isTerm() {} +func (FreeText) isTerm() {} + +// TimeExpr is either an absolute RFC3339 timestamp or a relative offset +// like -1h/-7d, resolved to an absolute time by the planner (relative to +// compile time), not the parser -- the parser has no notion of "now". +type TimeExpr struct { + Absolute string + IsRelative bool + RelativeSign int // -1 or +1 + RelativeN int + RelativeUnit string // "s" | "m" | "h" | "d" | "w" +} + +type AggCall struct { + Func string // count, sum, avg, min, max + Field string // empty for count()/count(*) + Alias string // empty => planner assigns a default alias +} + +type SortField struct { + Field string + Desc bool +} diff --git a/api/internal/queryapi/executor.go b/api/internal/querylang/executor/chrunner.go similarity index 62% rename from api/internal/queryapi/executor.go rename to api/internal/querylang/executor/chrunner.go index 28cc6c7..83bed19 100644 --- a/api/internal/queryapi/executor.go +++ b/api/internal/querylang/executor/chrunner.go @@ -1,4 +1,4 @@ -package queryapi +package executor import ( "context" @@ -8,32 +8,30 @@ import ( "github.com/ClickHouse/clickhouse-go/v2/lib/driver" ) -type QueryResult struct { - Columns []string `json:"columns"` - Rows [][]any `json:"rows"` -} - -// Executor runs arbitrary (pre-validated) SELECT statements against +// ChRunner runs arbitrary (pre-validated) SELECT statements against // ClickHouse and shapes the result into JSON-friendly columns/rows, // discovering the result's column set at query time via reflection since -// the query itself is arbitrary. -type Executor struct { +// the query itself is arbitrary. Ported from Phase 0/1's +// api/internal/queryapi.Executor, which this replaces (see task 4) -- +// same logic, moved here since it's the query-execution layer's +// plumbing, not specific to the old placeholder /query handler. +type ChRunner struct { conn driver.Conn } -func NewExecutor(conn driver.Conn) *Executor { - return &Executor{conn: conn} +func NewChRunner(conn driver.Conn) *ChRunner { + return &ChRunner{conn: conn} } -func (e *Executor) Execute(ctx context.Context, sql string) (*QueryResult, error) { - rows, err := e.conn.Query(ctx, sql) +func (r *ChRunner) RunSQL(ctx context.Context, sql string) (*Result, error) { + rows, err := r.conn.Query(ctx, sql) if err != nil { return nil, fmt.Errorf("executing query: %w", err) } defer rows.Close() columnTypes := rows.ColumnTypes() - result := &QueryResult{ + result := &Result{ Columns: rows.Columns(), Rows: [][]any{}, } diff --git a/api/internal/querylang/executor/executor.go b/api/internal/querylang/executor/executor.go new file mode 100644 index 0000000..7f36c78 --- /dev/null +++ b/api/internal/querylang/executor/executor.go @@ -0,0 +1,72 @@ +// Package executor runs a compiled ir.Plan and returns results in a +// shape consistent regardless of which backend(s) were hit -- the point +// of compiling to one IR in the first place. See +// /docs/query-language-design.md's "Execution" section for the four +// routing cases implemented here. +package executor + +import ( + "context" + "fmt" + + "github.com/sentry/sentry/api/internal/querylang/ir" +) + +type Result struct { + Columns []string + Rows [][]any +} + +// SQLRunner executes a raw SQL statement against ClickHouse. *ChRunner +// (chrunner.go) is the production implementation; tests use a fake -- +// same narrow-interface pattern used throughout /ingest and /api. +type SQLRunner interface { + RunSQL(ctx context.Context, sql string) (*Result, error) +} + +// SearchClient resolves a Tantivy query into matching record_ids. +type SearchClient interface { + Search(ctx context.Context, query string, limit uint32) ([]string, error) +} + +// textSearchLimit caps how many record_ids a Tantivy prefilter can feed +// into a ClickHouse `IN (...)` clause. See /docs/query-language-design.md's +// "Known scaling limitation" -- this is a real, disclosed limit on result +// completeness for very broad text searches, not an oversight. +// +// 5000, not 10000: confirmed by actually running the Phase 2 benchmark +// (see /docs/phase-2-runbook.md) that 10000 quoted UUIDs (~39 bytes each +// including the comma) produces a ~390KB query string, which exceeds +// ClickHouse's default max_query_size (262144 bytes / 256KiB) and fails +// outright with a syntax error rather than degrading gracefully. 5000 +// UUIDs is ~195KB, safely under that default with headroom for the rest +// of the query. This was a real failure caught by running the benchmark, +// not a value chosen from first-principles estimation. +const textSearchLimit = 5000 + +// Execute runs plan against the given backends. The four cases (per the +// design doc): RawSQL passthrough; pure ClickHouse (no TextSearch); text +// search alone (Tantivy prefilter -> ClickHouse row fetch); text search +// plus aggregation (Tantivy prefilter -> ClickHouse aggregate). Cases 2-4 +// share the same buildSQL/buildWhereClause code (sql.go) -- the only +// difference is whether a record_id filter is threaded in. +func Execute(ctx context.Context, plan *ir.Plan, sqlRunner SQLRunner, search SearchClient) (*Result, error) { + if plan.RawSQL != "" { + return sqlRunner.RunSQL(ctx, plan.RawSQL) + } + + var recordIDFilter []string + if len(plan.TextSearch) > 0 { + ids, err := search.Search(ctx, plan.TextSearch[0].Query, textSearchLimit) + if err != nil { + return nil, fmt.Errorf("full-text search failed: %w", err) + } + if len(ids) == 0 { + return &Result{Columns: []string{}, Rows: [][]any{}}, nil + } + recordIDFilter = ids + } + + sql := buildSQL(plan, recordIDFilter) + return sqlRunner.RunSQL(ctx, sql) +} diff --git a/api/internal/querylang/executor/executor_test.go b/api/internal/querylang/executor/executor_test.go new file mode 100644 index 0000000..f054585 --- /dev/null +++ b/api/internal/querylang/executor/executor_test.go @@ -0,0 +1,309 @@ +package executor + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/sentry/sentry/api/internal/querylang/ir" +) + +func mustParseTime(t *testing.T, s string) time.Time { + t.Helper() + tm, err := time.Parse(time.RFC3339, s) + if err != nil { + t.Fatalf("parsing time %q: %v", s, err) + } + return tm +} + +type fakeSQLRunner struct { + gotSQL string + result *Result + err error + calls int +} + +func (f *fakeSQLRunner) RunSQL(_ context.Context, sql string) (*Result, error) { + f.gotSQL = sql + f.calls++ + if f.err != nil { + return nil, f.err + } + if f.result != nil { + return f.result, nil + } + return &Result{Columns: []string{}, Rows: [][]any{}}, nil +} + +type fakeSearchClient struct { + gotQuery string + gotLimit uint32 + ids []string + err error + calls int +} + +func (f *fakeSearchClient) Search(_ context.Context, query string, limit uint32) ([]string, error) { + f.gotQuery = query + f.gotLimit = limit + f.calls++ + if f.err != nil { + return nil, f.err + } + return f.ids, nil +} + +func TestExecuteRawSQLBypassesEverythingElse(t *testing.T) { + sqlRunner := &fakeSQLRunner{} + search := &fakeSearchClient{} + plan := &ir.Plan{RawSQL: "SELECT 1"} + + _, err := Execute(context.Background(), plan, sqlRunner, search) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + if sqlRunner.gotSQL != "SELECT 1" { + t.Fatalf("gotSQL = %q, want %q", sqlRunner.gotSQL, "SELECT 1") + } + if search.calls != 0 { + t.Fatalf("expected search not to be called for RawSQL, got %d calls", search.calls) + } +} + +func TestExecutePureClickHousePathSkipsSearch(t *testing.T) { + sqlRunner := &fakeSQLRunner{} + search := &fakeSearchClient{} + plan := &ir.Plan{Filters: []ir.FilterPredicate{{Field: "service", Op: "=", Value: "api"}}} + + _, err := Execute(context.Background(), plan, sqlRunner, search) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + if search.calls != 0 { + t.Fatalf("expected no search calls, got %d", search.calls) + } + if !strings.Contains(sqlRunner.gotSQL, "FROM logs") || !strings.Contains(sqlRunner.gotSQL, "`service` = 'api'") { + t.Fatalf("unexpected SQL: %s", sqlRunner.gotSQL) + } +} + +func TestExecuteTextSearchPrefiltersThenQueriesClickHouse(t *testing.T) { + sqlRunner := &fakeSQLRunner{} + search := &fakeSearchClient{ids: []string{"id-1", "id-2"}} + plan := &ir.Plan{TextSearch: []ir.TextPredicate{{Query: "connection refused"}}} + + _, err := Execute(context.Background(), plan, sqlRunner, search) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + if search.gotQuery != "connection refused" { + t.Fatalf("search query = %q", search.gotQuery) + } + if search.gotLimit != textSearchLimit { + t.Fatalf("search limit = %d, want %d", search.gotLimit, textSearchLimit) + } + if !strings.Contains(sqlRunner.gotSQL, "record_id IN ('id-1','id-2')") { + t.Fatalf("unexpected SQL: %s", sqlRunner.gotSQL) + } +} + +func TestExecuteTextSearchNoMatchesSkipsClickHouseEntirely(t *testing.T) { + sqlRunner := &fakeSQLRunner{} + search := &fakeSearchClient{ids: nil} + plan := &ir.Plan{TextSearch: []ir.TextPredicate{{Query: "nothing matches"}}} + + result, err := Execute(context.Background(), plan, sqlRunner, search) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + if sqlRunner.calls != 0 { + t.Fatalf("expected ClickHouse not to be queried when search finds nothing, got %d calls", sqlRunner.calls) + } + if len(result.Columns) != 0 || len(result.Rows) != 0 { + t.Fatalf("expected empty result, got %+v", result) + } +} + +func TestExecuteTextSearchWithAggregation(t *testing.T) { + sqlRunner := &fakeSQLRunner{} + search := &fakeSearchClient{ids: []string{"id-1"}} + plan := &ir.Plan{ + TextSearch: []ir.TextPredicate{{Query: "connection refused"}}, + Aggregation: &ir.Aggregation{ + Funcs: []ir.AggFunc{{Func: "count", Alias: "count"}}, + GroupBy: []string{"host"}, + }, + } + + _, err := Execute(context.Background(), plan, sqlRunner, search) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + if !strings.Contains(sqlRunner.gotSQL, "record_id IN ('id-1')") { + t.Fatalf("expected the text-search prefilter in the WHERE clause: %s", sqlRunner.gotSQL) + } + if !strings.Contains(sqlRunner.gotSQL, "GROUP BY `host`") { + t.Fatalf("expected GROUP BY: %s", sqlRunner.gotSQL) + } + if !strings.Contains(sqlRunner.gotSQL, "count() AS `count`") { + t.Fatalf("expected count() AS `count`: %s", sqlRunner.gotSQL) + } +} + +func TestExecuteSearchErrorPropagates(t *testing.T) { + sqlRunner := &fakeSQLRunner{} + search := &fakeSearchClient{err: errors.New("search unavailable")} + plan := &ir.Plan{TextSearch: []ir.TextPredicate{{Query: "x"}}} + + _, err := Execute(context.Background(), plan, sqlRunner, search) + if err == nil { + t.Fatal("expected the search error to propagate") + } + if sqlRunner.calls != 0 { + t.Fatalf("expected ClickHouse not to be queried after a search error, got %d calls", sqlRunner.calls) + } +} + +func TestBuildSQLNumericCastOnAttributesField(t *testing.T) { + plan := &ir.Plan{Filters: []ir.FilterPredicate{{Field: "status", Op: ">=", Value: "500"}}} + sql := buildSQL(plan, nil) + want := "toFloat64OrZero(attributes['status']) >= 500" + if !strings.Contains(sql, want) { + t.Fatalf("SQL = %q, want it to contain %q", sql, want) + } +} + +func TestBuildSQLStringComparisonOnAttributesField(t *testing.T) { + plan := &ir.Plan{Filters: []ir.FilterPredicate{{Field: "status", Op: "=", Value: "unknown"}}} + sql := buildSQL(plan, nil) + want := "attributes['status'] = 'unknown'" + if !strings.Contains(sql, want) { + t.Fatalf("SQL = %q, want it to contain %q", sql, want) + } +} + +func TestBuildSQLTopLevelFieldNeverCast(t *testing.T) { + plan := &ir.Plan{Filters: []ir.FilterPredicate{{Field: "service", Op: "=", Value: "123"}}} + sql := buildSQL(plan, nil) + if strings.Contains(sql, "toFloat64OrZero") { + t.Fatalf("top-level field should never be numeric-cast: %s", sql) + } + if !strings.Contains(sql, "`service` = '123'") { + t.Fatalf("unexpected SQL: %s", sql) + } +} + +func TestBuildSQLEscapesInjectionAttemptInValue(t *testing.T) { + plan := &ir.Plan{Filters: []ir.FilterPredicate{{Field: "service", Op: "=", Value: "x'; DROP TABLE logs; --"}}} + sql := buildSQL(plan, nil) + // The whole attacker-controlled value must land inside exactly one + // quoted literal, with its embedded quote backslash-escaped so it + // can't terminate the literal early -- checking for the escaped + // form directly, not just the absence of the raw substring (which + // is a weaker check: "\\'; DROP TABLE" still *contains* "'; DROP + // TABLE" as a substring, so that alone doesn't prove escaping + // happened). + want := `'x\'; DROP TABLE logs; --'` + if !strings.Contains(sql, want) { + t.Fatalf("expected the literal %q in SQL, got: %s", want, sql) + } +} + +func TestBuildSQLDefaultLimitAppliedWhenNoneGiven(t *testing.T) { + plan := &ir.Plan{Filters: []ir.FilterPredicate{{Field: "service", Op: "=", Value: "api"}}} + sql := buildSQL(plan, nil) + if !strings.Contains(sql, "LIMIT 100") { + t.Fatalf("expected the default row limit, got: %s", sql) + } +} + +func TestBuildSQLExplicitLimitOverridesDefault(t *testing.T) { + plan := &ir.Plan{ + Filters: []ir.FilterPredicate{{Field: "service", Op: "=", Value: "api"}}, + Limit: &ir.Limit{N: 5}, + } + sql := buildSQL(plan, nil) + if !strings.Contains(sql, "LIMIT 5") || strings.Contains(sql, "LIMIT 100") { + t.Fatalf("expected LIMIT 5, got: %s", sql) + } +} + +func TestBuildSQLTailWithoutSortOrdersAscending(t *testing.T) { + plan := &ir.Plan{Limit: &ir.Limit{N: 10, Tail: true}} + sql := buildSQL(plan, nil) + if !strings.Contains(sql, "ORDER BY `timestamp` ASC") { + t.Fatalf("expected ascending order for tail, got: %s", sql) + } +} + +func TestBuildSQLNoSortDefaultsNewestFirst(t *testing.T) { + plan := &ir.Plan{} + sql := buildSQL(plan, nil) + if !strings.Contains(sql, "ORDER BY `timestamp` DESC") { + t.Fatalf("expected newest-first default, got: %s", sql) + } +} + +func TestBuildSQLSortByAggregateAlias(t *testing.T) { + plan := &ir.Plan{ + Aggregation: &ir.Aggregation{ + Funcs: []ir.AggFunc{{Func: "count", Alias: "count"}}, + GroupBy: []string{"host"}, + }, + Sort: []ir.SortField{{Field: "count", Desc: true}}, + } + sql := buildSQL(plan, nil) + if !strings.Contains(sql, "ORDER BY `count` DESC") { + t.Fatalf("expected ORDER BY on the aggregate alias, got: %s", sql) + } +} + +func TestBuildSQLSortByGroupByField(t *testing.T) { + plan := &ir.Plan{ + Aggregation: &ir.Aggregation{ + Funcs: []ir.AggFunc{{Func: "count", Alias: "count"}}, + GroupBy: []string{"host"}, + }, + Sort: []ir.SortField{{Field: "host", Desc: false}}, + } + sql := buildSQL(plan, nil) + if !strings.Contains(sql, "ORDER BY `host` ASC") { + t.Fatalf("expected ORDER BY on the group-by column, got: %s", sql) + } +} + +func TestBuildSQLAggregationOnAttributesFieldAlwaysCasts(t *testing.T) { + plan := &ir.Plan{ + Aggregation: &ir.Aggregation{ + Funcs: []ir.AggFunc{{Func: "avg", Field: "latency_ms", Alias: "avg_latency"}}, + }, + } + sql := buildSQL(plan, nil) + if !strings.Contains(sql, "AVG(toFloat64OrZero(attributes['latency_ms'])) AS `avg_latency`") { + t.Fatalf("unexpected SQL: %s", sql) + } +} + +func TestBuildSQLProjectionFields(t *testing.T) { + plan := &ir.Plan{Fields: []string{"host", "message"}} + sql := buildSQL(plan, nil) + if !strings.Contains(sql, "SELECT `host` AS `host`, `message` AS `message` FROM logs") { + t.Fatalf("unexpected SQL: %s", sql) + } +} + +func TestBuildSQLTimeRange(t *testing.T) { + from := mustParseTime(t, "2026-08-14T00:00:00Z") + to := mustParseTime(t, "2026-08-14T01:00:00Z") + plan := &ir.Plan{TimeRange: &ir.TimeRange{From: from, To: to}} + sql := buildSQL(plan, nil) + if !strings.Contains(sql, "`timestamp` >= '2026-08-14T00:00:00Z'") { + t.Fatalf("missing From bound: %s", sql) + } + if !strings.Contains(sql, "`timestamp` <= '2026-08-14T01:00:00Z'") { + t.Fatalf("missing To bound: %s", sql) + } +} diff --git a/api/internal/querylang/executor/sql.go b/api/internal/querylang/executor/sql.go new file mode 100644 index 0000000..d2f06ae --- /dev/null +++ b/api/internal/querylang/executor/sql.go @@ -0,0 +1,228 @@ +package executor + +import ( + "fmt" + "regexp" + "strings" + "time" + + "github.com/sentry/sentry/api/internal/querylang/ir" +) + +// defaultRowLimit is the safety net when a raw-row query has neither an +// explicit head/tail nor an aggregation -- without it, a bare `service=api` +// with no other pipe stages would return every matching row unbounded. +// Independent of planner's own defaultLimit (same value, different +// concern: that one fills in `head`/`tail` with no N given; this one +// guards queries that never mention head/tail at all). +const defaultRowLimit = 100 + +// logs' real columns, per /storage. Anything else maps to +// attributes['field'] -- see /docs/query-language-design.md's "Field +// mapping" section. +var topLevelFields = map[string]bool{ + "timestamp": true, + "host": true, + "service": true, + "severity": true, + "message": true, + "record_id": true, +} + +func buildSQL(plan *ir.Plan, recordIDFilter []string) string { + var sb strings.Builder + + sb.WriteString("SELECT ") + sb.WriteString(selectClause(plan)) + sb.WriteString(" FROM logs") + + if where := buildWhereClause(plan, recordIDFilter); where != "" { + sb.WriteString(" WHERE ") + sb.WriteString(where) + } + + if plan.Aggregation != nil && len(plan.Aggregation.GroupBy) > 0 { + sb.WriteString(" GROUP BY ") + cols := make([]string, len(plan.Aggregation.GroupBy)) + for i, g := range plan.Aggregation.GroupBy { + cols[i] = columnExpr(g) + } + sb.WriteString(strings.Join(cols, ", ")) + } + + writeOrderBy(&sb, plan) + + if plan.Limit != nil { + fmt.Fprintf(&sb, " LIMIT %d", plan.Limit.N) + } else if plan.Aggregation == nil { + fmt.Fprintf(&sb, " LIMIT %d", defaultRowLimit) + } + + return sb.String() +} + +func writeOrderBy(sb *strings.Builder, plan *ir.Plan) { + switch { + case len(plan.Sort) > 0: + sb.WriteString(" ORDER BY ") + parts := make([]string, len(plan.Sort)) + for i, s := range plan.Sort { + dir := "ASC" + if s.Desc { + dir = "DESC" + } + parts[i] = sortColumnExpr(plan, s.Field) + " " + dir + } + sb.WriteString(strings.Join(parts, ", ")) + case plan.Limit != nil && plan.Limit.Tail: + // `tail N` with no explicit sort: order ascending so LIMIT N + // takes the chronologically *last* N rows. Callers wanting + // strict newest-first display order re-sort client-side -- + // documented in the query language reference. + sb.WriteString(" ORDER BY `timestamp` ASC") + case plan.Aggregation == nil: + // Raw-row queries with no explicit sort default to newest-first, + // matching the Phase 0/1 UI default. + sb.WriteString(" ORDER BY `timestamp` DESC") + } +} + +// sortColumnExpr resolves a sort field against an aggregation's own +// output columns (alias or group-by field) before falling back to the +// normal top-level/attributes mapping -- `sort -count` after `stats +// count` refers to the aggregate's alias, not a raw column. +func sortColumnExpr(plan *ir.Plan, field string) string { + if plan.Aggregation != nil { + for _, f := range plan.Aggregation.Funcs { + if f.Alias == field { + return quoteIdent(field) + } + } + for _, g := range plan.Aggregation.GroupBy { + if g == field { + return columnExpr(field) + } + } + } + return columnExpr(field) +} + +func selectClause(plan *ir.Plan) string { + if plan.Aggregation != nil { + parts := make([]string, 0, len(plan.Aggregation.GroupBy)+len(plan.Aggregation.Funcs)) + for _, g := range plan.Aggregation.GroupBy { + parts = append(parts, columnExpr(g)+" AS "+quoteIdent(g)) + } + for _, f := range plan.Aggregation.Funcs { + parts = append(parts, aggExpr(f)+" AS "+quoteIdent(f.Alias)) + } + return strings.Join(parts, ", ") + } + if len(plan.Fields) > 0 { + parts := make([]string, len(plan.Fields)) + for i, f := range plan.Fields { + parts[i] = columnExpr(f) + " AS " + quoteIdent(f) + } + return strings.Join(parts, ", ") + } + return "*" +} + +// aggExpr always numeric-casts non-top-level (attributes-map) fields for +// sum/avg/min/max, unlike comparison predicates where casting is +// conditional on whether the compared value looks numeric -- an +// aggregate function is inherently a numeric (or, for min/max, +// order-comparable) operation, so there's no "maybe string" case the way +// there is for `field=value`. Known Phase 2 limitation: min/max on a +// non-top-level field always compares numerically, not lexicographically +// -- string min/max on attributes isn't supported this phase. +func aggExpr(f ir.AggFunc) string { + if f.Func == "count" { + return "count()" + } + col := columnExpr(f.Field) + if !topLevelFields[f.Field] { + col = "toFloat64OrZero(" + col + ")" + } + return strings.ToUpper(f.Func) + "(" + col + ")" +} + +func buildWhereClause(plan *ir.Plan, recordIDFilter []string) string { + var conds []string + + if len(recordIDFilter) > 0 { + quoted := make([]string, len(recordIDFilter)) + for i, id := range recordIDFilter { + quoted[i] = quoteLiteral(id) + } + conds = append(conds, "record_id IN ("+strings.Join(quoted, ",")+")") + } + + for _, f := range plan.Filters { + conds = append(conds, buildComparisonSQL(f)) + } + + if plan.TimeRange != nil { + if !plan.TimeRange.From.IsZero() { + conds = append(conds, "`timestamp` >= "+quoteLiteral(plan.TimeRange.From.UTC().Format(time.RFC3339Nano))) + } + if !plan.TimeRange.To.IsZero() { + conds = append(conds, "`timestamp` <= "+quoteLiteral(plan.TimeRange.To.UTC().Format(time.RFC3339Nano))) + } + } + + return strings.Join(conds, " AND ") +} + +// buildComparisonSQL numeric-casts a non-top-level field only when the +// compared value itself looks numeric -- `status>=500` casts (numeric +// comparison intent), `status="unknown"` doesn't (string comparison +// intent). Top-level fields are never cast; ClickHouse compares them +// against a string literal natively (DateTime64 columns parse an +// RFC3339-shaped literal, LowCardinality(String)/String compare as-is). +func buildComparisonSQL(f ir.FilterPredicate) string { + if !topLevelFields[f.Field] && isNumericLiteral(f.Value) { + return "toFloat64OrZero(" + columnExpr(f.Field) + ") " + f.Op + " " + f.Value + } + return columnExpr(f.Field) + " " + f.Op + " " + quoteLiteral(f.Value) +} + +func columnExpr(field string) string { + if topLevelFields[field] { + return quoteIdent(field) + } + return "attributes[" + quoteLiteral(field) + "]" +} + +func quoteIdent(name string) string { + return "`" + strings.ReplaceAll(name, "`", "``") + "`" +} + +// quoteLiteral is the actual injection defense for every user-controlled +// string embedded in generated SQL (filter values, attribute keys, time +// bounds, record_ids). Field/keyword tokens from the lexer are already +// constrained to [a-zA-Z0-9_.] by construction (see lexer.isIdentPart) +// and can't carry SQL metacharacters at all, but quoted-string *values* +// can contain anything, so this can't be skipped for them. +func quoteLiteral(s string) string { + var sb strings.Builder + sb.WriteByte('\'') + for _, r := range s { + switch r { + case '\\': + sb.WriteString(`\\`) + case '\'': + sb.WriteString(`\'`) + default: + sb.WriteRune(r) + } + } + sb.WriteByte('\'') + return sb.String() +} + +var numericLiteralRe = regexp.MustCompile(`^-?\d+(\.\d+)?$`) + +func isNumericLiteral(s string) bool { + return numericLiteralRe.MatchString(s) +} diff --git a/api/internal/querylang/ir/ir.go b/api/internal/querylang/ir/ir.go new file mode 100644 index 0000000..bd03454 --- /dev/null +++ b/api/internal/querylang/ir/ir.go @@ -0,0 +1,80 @@ +// Package ir defines Plan, the intermediate representation both the +// pipe-syntax parser+planner and the raw-SQL passthrough compile down +// to. This is the boundary task 3 asked for: "pipe syntax X compiles to +// IR Y" is testable in planner without any backend; "IR Y executes +// correctly" is testable in executor against fakes, independent of the +// planner. See /docs/query-language-design.md. +package ir + +import "time" + +type Plan struct { + // RawSQL, when non-empty, means the entire plan is this opaque + // ClickHouse SQL string, executed as-is -- every other field below + // is unused. This is the SQL escape hatch's IR representation: a + // trivial identity compilation that still flows through the same + // Plan type and the same executor code path as a parsed pipe query. + RawSQL string + + // TextSearch predicates route to Tantivy as a prefilter. Empty means + // no Tantivy involvement at all -- pure ClickHouse. + TextSearch []TextPredicate + + // Filters are always evaluated in ClickHouse, either directly as + // WHERE clauses (no TextSearch present) or as an additional filter + // alongside a Tantivy-sourced record_id IN (...) clause. + Filters []FilterPredicate + + TimeRange *TimeRange + + // Aggregation is nil for a raw-rows query (no GROUP BY). + Aggregation *Aggregation + + Sort []SortField + + // Fields is the projection; empty means all columns. + Fields []string + + Limit *Limit +} + +type TextPredicate struct { + // Query is passed to Tantivy's query parser as-is -- phrase and + // wildcard syntax already supported there (see /search). + Query string +} + +type FilterPredicate struct { + Field string + Op string // "=", "!=", ">", ">=", "<", "<=" + Value string +} + +type Aggregation struct { + Funcs []AggFunc + GroupBy []string +} + +type AggFunc struct { + Func string // count, sum, avg, min, max + Field string // empty for count + Alias string // always set by the planner (defaulted if not given explicitly) +} + +type SortField struct { + Field string + Desc bool +} + +type Limit struct { + N int + Tail bool // true = last N (by time), false = first N +} + +type TimeRange struct { + // Absolute bounds -- any relative expression (-1h etc.) is resolved + // by the planner at compile time, since only it knows "now". A zero + // time.Time means that bound is unset. + From time.Time + To time.Time +} diff --git a/api/internal/querylang/lexer/lexer.go b/api/internal/querylang/lexer/lexer.go new file mode 100644 index 0000000..0e09fb4 --- /dev/null +++ b/api/internal/querylang/lexer/lexer.go @@ -0,0 +1,238 @@ +// Package lexer tokenizes the pipe-syntax query language. Deliberately +// simple: keywords (where/stats/sort/and/etc.) aren't distinct token +// kinds -- they're just Ident tokens whose value the parser checks +// against a keyword set, so the lexer stays context-free and the parser +// owns all the grammar decisions. See /docs/query-language-design.md. +package lexer + +import "fmt" + +type Kind int + +const ( + EOF Kind = iota + Illegal + Ident // bare words: field names, keywords, unquoted values/free-text terms + String // quoted string: "..." + Number // 123, 1.5 + Pipe // | + Eq // = + Neq // != + Gt // > + Gte // >= + Lt // < + Lte // <= + Colon // : + Comma // , + LParen // ( + RParen // ) + Minus // - + Plus // + + Star // * (only meaningful inside count(*), same as SQL) +) + +type Token struct { + Kind Kind + Value string + Pos int // byte offset into the original input, for error messages +} + +func (t Token) String() string { + return fmt.Sprintf("%s(%q)@%d", t.Kind, t.Value, t.Pos) +} + +func (k Kind) String() string { + switch k { + case EOF: + return "EOF" + case Illegal: + return "ILLEGAL" + case Ident: + return "IDENT" + case String: + return "STRING" + case Number: + return "NUMBER" + case Pipe: + return "PIPE" + case Eq: + return "EQ" + case Neq: + return "NEQ" + case Gt: + return "GT" + case Gte: + return "GTE" + case Lt: + return "LT" + case Lte: + return "LTE" + case Colon: + return "COLON" + case Comma: + return "COMMA" + case LParen: + return "LPAREN" + case RParen: + return "RPAREN" + case Minus: + return "MINUS" + case Plus: + return "PLUS" + case Star: + return "STAR" + default: + return "UNKNOWN" + } +} + +type Lexer struct { + input []rune + pos int +} + +func New(input string) *Lexer { + return &Lexer{input: []rune(input)} +} + +func (l *Lexer) Next() Token { + l.skipWhitespace() + if l.pos >= len(l.input) { + return Token{Kind: EOF, Pos: l.pos} + } + + start := l.pos + c := l.input[l.pos] + + switch { + case c == '|': + l.pos++ + return Token{Kind: Pipe, Value: "|", Pos: start} + case c == '=': + l.pos++ + return Token{Kind: Eq, Value: "=", Pos: start} + case c == '!' && l.peek(1) == '=': + l.pos += 2 + return Token{Kind: Neq, Value: "!=", Pos: start} + case c == '>' && l.peek(1) == '=': + l.pos += 2 + return Token{Kind: Gte, Value: ">=", Pos: start} + case c == '>': + l.pos++ + return Token{Kind: Gt, Value: ">", Pos: start} + case c == '<' && l.peek(1) == '=': + l.pos += 2 + return Token{Kind: Lte, Value: "<=", Pos: start} + case c == '<': + l.pos++ + return Token{Kind: Lt, Value: "<", Pos: start} + case c == ':': + l.pos++ + return Token{Kind: Colon, Value: ":", Pos: start} + case c == ',': + l.pos++ + return Token{Kind: Comma, Value: ",", Pos: start} + case c == '(': + l.pos++ + return Token{Kind: LParen, Value: "(", Pos: start} + case c == ')': + l.pos++ + return Token{Kind: RParen, Value: ")", Pos: start} + case c == '-': + l.pos++ + return Token{Kind: Minus, Value: "-", Pos: start} + case c == '+': + l.pos++ + return Token{Kind: Plus, Value: "+", Pos: start} + case c == '*': + l.pos++ + return Token{Kind: Star, Value: "*", Pos: start} + case c == '"': + return l.lexString() + case isDigit(c): + return l.lexNumber() + case isIdentStart(c): + return l.lexIdent() + default: + l.pos++ + return Token{Kind: Illegal, Value: string(c), Pos: start} + } +} + +func (l *Lexer) peek(offset int) rune { + p := l.pos + offset + if p >= len(l.input) { + return 0 + } + return l.input[p] +} + +func (l *Lexer) skipWhitespace() { + for l.pos < len(l.input) { + switch l.input[l.pos] { + case ' ', '\t', '\n', '\r': + l.pos++ + default: + return + } + } +} + +func (l *Lexer) lexString() Token { + start := l.pos + l.pos++ // consume opening quote + var sb []rune + for l.pos < len(l.input) { + c := l.input[l.pos] + if c == '"' { + l.pos++ + return Token{Kind: String, Value: string(sb), Pos: start} + } + if c == '\\' && l.pos+1 < len(l.input) { + l.pos++ + sb = append(sb, l.input[l.pos]) + l.pos++ + continue + } + sb = append(sb, c) + l.pos++ + } + // unterminated string -- return what we have as Illegal so the + // parser can produce a clear "unterminated string" error rather than + // the lexer silently accepting it. + return Token{Kind: Illegal, Value: string(sb), Pos: start} +} + +func (l *Lexer) lexNumber() Token { + start := l.pos + for l.pos < len(l.input) && isDigit(l.input[l.pos]) { + l.pos++ + } + if l.pos < len(l.input) && l.input[l.pos] == '.' && l.pos+1 < len(l.input) && isDigit(l.input[l.pos+1]) { + l.pos++ + for l.pos < len(l.input) && isDigit(l.input[l.pos]) { + l.pos++ + } + } + return Token{Kind: Number, Value: string(l.input[start:l.pos]), Pos: start} +} + +func (l *Lexer) lexIdent() Token { + start := l.pos + for l.pos < len(l.input) && isIdentPart(l.input[l.pos]) { + l.pos++ + } + return Token{Kind: Ident, Value: string(l.input[start:l.pos]), Pos: start} +} + +func isDigit(c rune) bool { return c >= '0' && c <= '9' } + +func isIdentStart(c rune) bool { + return c == '_' || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') +} + +// Identifiers allow dots (e.g. winevt.event_id, a real attribute key +// shape from Phase 1) and digits after the first character. +func isIdentPart(c rune) bool { + return isIdentStart(c) || isDigit(c) || c == '.' || c == '_' +} diff --git a/api/internal/querylang/lexer/lexer_test.go b/api/internal/querylang/lexer/lexer_test.go new file mode 100644 index 0000000..fde00ca --- /dev/null +++ b/api/internal/querylang/lexer/lexer_test.go @@ -0,0 +1,140 @@ +package lexer + +import "testing" + +func collectKinds(input string) []Kind { + l := New(input) + var kinds []Kind + for { + tok := l.Next() + kinds = append(kinds, tok.Kind) + if tok.Kind == EOF { + return kinds + } + } +} + +func TestLexSimpleFilter(t *testing.T) { + got := collectKinds(`service=api`) + want := []Kind{Ident, Eq, Ident, EOF} + assertKinds(t, got, want) +} + +func TestLexPipeline(t *testing.T) { + got := collectKinds(`service=api | where status>=500 | stats count(*) by host`) + want := []Kind{ + Ident, Eq, Ident, Pipe, + Ident, Ident, Gte, Number, Pipe, + Ident, Ident, LParen, Star, RParen, Ident, Ident, + EOF, + } + assertKinds(t, got, want) +} + +func TestLexOperators(t *testing.T) { + got := collectKinds(`= != > >= < <=`) + want := []Kind{Eq, Neq, Gt, Gte, Lt, Lte, EOF} + assertKinds(t, got, want) +} + +func TestLexQuotedString(t *testing.T) { + l := New(`"connection refused"`) + tok := l.Next() + if tok.Kind != String { + t.Fatalf("Kind = %v, want String", tok.Kind) + } + if tok.Value != "connection refused" { + t.Fatalf("Value = %q, want %q", tok.Value, "connection refused") + } +} + +func TestLexQuotedStringWithEscapes(t *testing.T) { + l := New(`"has \"quotes\" and \\backslash"`) + tok := l.Next() + if tok.Kind != String { + t.Fatalf("Kind = %v, want String", tok.Kind) + } + want := `has "quotes" and \backslash` + if tok.Value != want { + t.Fatalf("Value = %q, want %q", tok.Value, want) + } +} + +func TestLexUnterminatedStringIsIllegal(t *testing.T) { + l := New(`"unterminated`) + tok := l.Next() + if tok.Kind != Illegal { + t.Fatalf("Kind = %v, want Illegal", tok.Kind) + } +} + +func TestLexFieldWithDots(t *testing.T) { + l := New(`winevt.event_id=4625`) + tok := l.Next() + if tok.Kind != Ident || tok.Value != "winevt.event_id" { + t.Fatalf("got %v, want Ident(winevt.event_id)", tok) + } +} + +func TestLexNumber(t *testing.T) { + cases := []string{"123", "1.5", "0"} + for _, c := range cases { + l := New(c) + tok := l.Next() + if tok.Kind != Number || tok.Value != c { + t.Errorf("lexing %q: got %v, want Number(%s)", c, tok, c) + } + } +} + +func TestLexNegativeTimeExpr(t *testing.T) { + // "-1h" lexes as MINUS, NUMBER, IDENT -- the parser composes these, + // not the lexer (see package doc comment). + got := collectKinds(`-1h`) + want := []Kind{Minus, Number, Ident, EOF} + assertKinds(t, got, want) +} + +func TestLexWhitespaceIsSkipped(t *testing.T) { + got := collectKinds(" service = api ") + want := []Kind{Ident, Eq, Ident, EOF} + assertKinds(t, got, want) +} + +func TestLexEmptyInput(t *testing.T) { + got := collectKinds("") + want := []Kind{EOF} + assertKinds(t, got, want) +} + +func TestLexIllegalCharacter(t *testing.T) { + l := New(`$`) + tok := l.Next() + if tok.Kind != Illegal { + t.Fatalf("Kind = %v, want Illegal", tok.Kind) + } +} + +func TestTokenPositionsAreByteOffsets(t *testing.T) { + l := New(`service=api`) + first := l.Next() + second := l.Next() + if first.Pos != 0 { + t.Errorf("first.Pos = %d, want 0", first.Pos) + } + if second.Pos != 7 { + t.Errorf("second.Pos = %d, want 7", second.Pos) + } +} + +func assertKinds(t *testing.T, got, want []Kind) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("got %d tokens %v, want %d tokens %v", len(got), got, len(want), want) + } + for i := range got { + if got[i] != want[i] { + t.Fatalf("token %d: got %v, want %v (full: got=%v want=%v)", i, got[i], want[i], got, want) + } + } +} diff --git a/api/internal/querylang/parser/parser.go b/api/internal/querylang/parser/parser.go new file mode 100644 index 0000000..16d9812 --- /dev/null +++ b/api/internal/querylang/parser/parser.go @@ -0,0 +1,452 @@ +// Package parser is a hand-written recursive-descent parser for the +// pipe-syntax query language, per /docs/query-language-design.md's +// choice of parser approach (no combinator/generator library -- this +// grammar is small and stable, and hand-written gives full control over +// error messages, which matter for a user-facing query language). +package parser + +import ( + "fmt" + "strconv" + + "github.com/sentry/sentry/api/internal/querylang/ast" + "github.com/sentry/sentry/api/internal/querylang/lexer" +) + +// Parse parses a pipe-syntax query. Callers are responsible for routing +// SQL (queries starting with "SELECT") elsewhere before calling this -- +// see planner.Plan and /docs/query-language-design.md's "SQL escape +// hatch" section for why this parser never sees SQL at all. +func Parse(input string) (*ast.Query, error) { + p := newParser(input) + return p.parseQuery() +} + +type parser struct { + lex *lexer.Lexer + cur lexer.Token + next lexer.Token +} + +func newParser(input string) *parser { + p := &parser{lex: lexer.New(input)} + p.next = p.lex.Next() + p.advance() + return p +} + +func (p *parser) advance() { + p.cur = p.next + p.next = p.lex.Next() +} + +func (p *parser) parseQuery() (*ast.Query, error) { + base, err := p.parseBoolExpr() + if err != nil { + return nil, err + } + q := &ast.Query{Base: base} + for p.cur.Kind == lexer.Pipe { + p.advance() + stage, err := p.parsePipeStage() + if err != nil { + return nil, err + } + q.Pipes = append(q.Pipes, stage) + } + if p.cur.Kind != lexer.EOF { + return nil, p.errorf("unexpected %s after query", p.cur) + } + return q, nil +} + +func (p *parser) parseBoolExpr() (ast.BoolExpr, error) { + var expr ast.BoolExpr + term, err := p.parseTerm() + if err != nil { + return expr, err + } + expr.Terms = append(expr.Terms, term) + + for { + var conj string + switch { + case p.cur.Kind == lexer.Ident && (p.cur.Value == "and" || p.cur.Value == "or"): + conj = p.cur.Value + p.advance() + case p.canStartTerm(): + // Adjacent bare terms with no explicit keyword between them + // implicitly AND, matching SPL's convention (e.g. `error + // timeout` means `error AND timeout`). + conj = "and" + default: + return expr, nil + } + term, err := p.parseTerm() + if err != nil { + return expr, err + } + expr.Terms = append(expr.Terms, term) + expr.Conjs = append(expr.Conjs, conj) + } +} + +func (p *parser) canStartTerm() bool { + switch p.cur.Kind { + case lexer.Ident, lexer.String: + return true + default: + return false + } +} + +func (p *parser) parseTerm() (ast.Term, error) { + switch p.cur.Kind { + case lexer.Ident: + if p.cur.Value == "earliest" || p.cur.Value == "latest" { + return p.parseTimeBound() + } + if p.cur.Value == "message" && p.next.Kind == lexer.Colon { + return p.parseExplicitFreeText() + } + if isComparatorStart(p.next.Kind) { + return p.parseComparison() + } + // A bare word with no comparator following it is a free-text + // search term, not a malformed comparison. + val := p.cur.Value + p.advance() + return ast.FreeText{Query: val}, nil + case lexer.String: + val := p.cur.Value + p.advance() + return ast.FreeText{Query: val}, nil + default: + return nil, p.errorf("expected a filter, comparison, or search term, got %s", p.cur) + } +} + +func (p *parser) parseComparison() (ast.Term, error) { + field := p.cur.Value + p.advance() + op, err := p.parseComparator() + if err != nil { + return nil, err + } + value, err := p.parseValue() + if err != nil { + return nil, err + } + return ast.Comparison{Field: field, Op: op, Value: value}, nil +} + +func (p *parser) parseComparator() (string, error) { + if !isComparatorStart(p.cur.Kind) { + return "", p.errorf("expected a comparator (=, !=, >, >=, <, <=), got %s", p.cur) + } + op := p.cur.Value + p.advance() + return op, nil +} + +func isComparatorStart(k lexer.Kind) bool { + switch k { + case lexer.Eq, lexer.Neq, lexer.Gt, lexer.Gte, lexer.Lt, lexer.Lte: + return true + default: + return false + } +} + +func (p *parser) parseValue() (string, error) { + switch p.cur.Kind { + case lexer.Ident, lexer.String, lexer.Number: + v := p.cur.Value + p.advance() + return v, nil + default: + return "", p.errorf("expected a value, got %s", p.cur) + } +} + +func (p *parser) parseTimeBound() (ast.Term, error) { + kind := p.cur.Value + p.advance() + if err := p.expect(lexer.Eq); err != nil { + return nil, err + } + expr, err := p.parseTimeExpr() + if err != nil { + return nil, err + } + return ast.TimeBound{Kind: kind, Expr: expr}, nil +} + +func (p *parser) parseTimeExpr() (ast.TimeExpr, error) { + if p.cur.Kind == lexer.String { + v := p.cur.Value + p.advance() + return ast.TimeExpr{Absolute: v}, nil + } + + sign := 1 + switch p.cur.Kind { + case lexer.Minus: + sign = -1 + p.advance() + case lexer.Plus: + p.advance() + } + + if p.cur.Kind != lexer.Number { + return ast.TimeExpr{}, p.errorf("expected a quoted absolute timestamp or a relative offset like -1h, got %s", p.cur) + } + n, err := strconv.Atoi(p.cur.Value) + if err != nil { + return ast.TimeExpr{}, p.errorf("invalid number %q in time expression", p.cur.Value) + } + p.advance() + + if p.cur.Kind != lexer.Ident || !isValidTimeUnit(p.cur.Value) { + return ast.TimeExpr{}, p.errorf("expected a time unit (s/m/h/d/w) after %d, got %s", n, p.cur) + } + unit := p.cur.Value + p.advance() + + return ast.TimeExpr{IsRelative: true, RelativeSign: sign, RelativeN: n, RelativeUnit: unit}, nil +} + +func isValidTimeUnit(u string) bool { + switch u { + case "s", "m", "h", "d", "w": + return true + default: + return false + } +} + +func (p *parser) parseExplicitFreeText() (ast.Term, error) { + p.advance() // "message" + if err := p.expect(lexer.Colon); err != nil { + return nil, err + } + if p.cur.Kind != lexer.String { + return nil, p.errorf("expected a quoted string after message:, got %s", p.cur) + } + v := p.cur.Value + p.advance() + return ast.FreeText{Query: v}, nil +} + +func (p *parser) parsePipeStage() (ast.PipeStage, error) { + if p.cur.Kind != lexer.Ident { + return nil, p.errorf("expected a pipe stage (where/stats/sort/fields/head/tail), got %s", p.cur) + } + switch p.cur.Value { + case "where": + p.advance() + expr, err := p.parseBoolExpr() + if err != nil { + return nil, err + } + return ast.WhereStage{Expr: expr}, nil + case "stats": + return p.parseStatsStage() + case "sort": + return p.parseSortStage() + case "fields": + return p.parseFieldsStage() + case "head": + return p.parseHeadTailStage(false) + case "tail": + return p.parseHeadTailStage(true) + default: + return nil, p.errorf("unknown pipe stage %q (expected where/stats/sort/fields/head/tail)", p.cur.Value) + } +} + +func (p *parser) parseStatsStage() (ast.PipeStage, error) { + p.advance() // "stats" + var stage ast.StatsStage + + agg, err := p.parseAggCall() + if err != nil { + return nil, err + } + stage.Aggs = append(stage.Aggs, agg) + + for p.cur.Kind == lexer.Comma { + p.advance() + agg, err := p.parseAggCall() + if err != nil { + return nil, err + } + stage.Aggs = append(stage.Aggs, agg) + } + + if p.cur.Kind == lexer.Ident && p.cur.Value == "by" { + p.advance() + field, err := p.parseFieldIdent() + if err != nil { + return nil, err + } + stage.By = append(stage.By, field) + for p.cur.Kind == lexer.Comma { + p.advance() + field, err := p.parseFieldIdent() + if err != nil { + return nil, err + } + stage.By = append(stage.By, field) + } + } + + return stage, nil +} + +func (p *parser) parseAggCall() (ast.AggCall, error) { + if p.cur.Kind != lexer.Ident { + return ast.AggCall{}, p.errorf("expected an aggregation function (count/sum/avg/min/max), got %s", p.cur) + } + fn := p.cur.Value + if !isValidAggFunc(fn) { + return ast.AggCall{}, p.errorf("unknown aggregation function %q (want count/sum/avg/min/max)", fn) + } + p.advance() + + // Parens are optional when there's no field: `count`, `count()`, and + // `count(*)` are all equivalent. `sum(field)` etc. still need them, + // since that's the only way to name the field. + var field string + if p.cur.Kind == lexer.LParen { + p.advance() + switch p.cur.Kind { + case lexer.Ident: + field = p.cur.Value + p.advance() + case lexer.Star: + p.advance() // count(*) is the same as count() + } + if err := p.expect(lexer.RParen); err != nil { + return ast.AggCall{}, err + } + } + + var alias string + if p.cur.Kind == lexer.Ident && p.cur.Value == "as" { + p.advance() + if p.cur.Kind != lexer.Ident { + return ast.AggCall{}, p.errorf("expected an alias after 'as', got %s", p.cur) + } + alias = p.cur.Value + p.advance() + } + + return ast.AggCall{Func: fn, Field: field, Alias: alias}, nil +} + +func isValidAggFunc(f string) bool { + switch f { + case "count", "sum", "avg", "min", "max": + return true + default: + return false + } +} + +func (p *parser) parseSortStage() (ast.PipeStage, error) { + p.advance() // "sort" + var stage ast.SortStage + + field, err := p.parseSortField() + if err != nil { + return nil, err + } + stage.Fields = append(stage.Fields, field) + + for p.cur.Kind == lexer.Comma { + p.advance() + field, err := p.parseSortField() + if err != nil { + return nil, err + } + stage.Fields = append(stage.Fields, field) + } + return stage, nil +} + +func (p *parser) parseSortField() (ast.SortField, error) { + desc := true // no explicit sign defaults to descending, same as an explicit "-" + switch p.cur.Kind { + case lexer.Minus: + p.advance() + case lexer.Plus: + desc = false + p.advance() + } + if p.cur.Kind != lexer.Ident { + return ast.SortField{}, p.errorf("expected a field name in sort, got %s", p.cur) + } + field := p.cur.Value + p.advance() + return ast.SortField{Field: field, Desc: desc}, nil +} + +func (p *parser) parseFieldsStage() (ast.PipeStage, error) { + p.advance() // "fields" + var stage ast.FieldsStage + field, err := p.parseFieldIdent() + if err != nil { + return nil, err + } + stage.Fields = append(stage.Fields, field) + for p.cur.Kind == lexer.Comma { + p.advance() + field, err := p.parseFieldIdent() + if err != nil { + return nil, err + } + stage.Fields = append(stage.Fields, field) + } + return stage, nil +} + +func (p *parser) parseFieldIdent() (string, error) { + if p.cur.Kind != lexer.Ident { + return "", p.errorf("expected a field name, got %s", p.cur) + } + v := p.cur.Value + p.advance() + return v, nil +} + +func (p *parser) parseHeadTailStage(tail bool) (ast.PipeStage, error) { + p.advance() // "head" / "tail" + if p.cur.Kind == lexer.Number { + n, err := strconv.Atoi(p.cur.Value) + if err != nil { + return nil, p.errorf("invalid number %q", p.cur.Value) + } + p.advance() + if tail { + return ast.TailStage{N: n, HasN: true}, nil + } + return ast.HeadStage{N: n, HasN: true}, nil + } + if tail { + return ast.TailStage{}, nil + } + return ast.HeadStage{}, nil +} + +func (p *parser) expect(k lexer.Kind) error { + if p.cur.Kind != k { + return p.errorf("expected %s, got %s", k, p.cur) + } + p.advance() + return nil +} + +func (p *parser) errorf(format string, args ...any) error { + return fmt.Errorf("query syntax error at position %d: %s", p.cur.Pos, fmt.Sprintf(format, args...)) +} diff --git a/api/internal/querylang/parser/parser_test.go b/api/internal/querylang/parser/parser_test.go new file mode 100644 index 0000000..245343d --- /dev/null +++ b/api/internal/querylang/parser/parser_test.go @@ -0,0 +1,278 @@ +package parser + +import ( + "testing" + + "github.com/sentry/sentry/api/internal/querylang/ast" +) + +func TestParseSimpleFilter(t *testing.T) { + q, err := Parse(`service=api`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if len(q.Base.Terms) != 1 { + t.Fatalf("expected 1 base term, got %d", len(q.Base.Terms)) + } + cmp, ok := q.Base.Terms[0].(ast.Comparison) + if !ok { + t.Fatalf("expected Comparison, got %T", q.Base.Terms[0]) + } + if cmp.Field != "service" || cmp.Op != "=" || cmp.Value != "api" { + t.Fatalf("unexpected comparison: %+v", cmp) + } + if len(q.Pipes) != 0 { + t.Fatalf("expected no pipes, got %d", len(q.Pipes)) + } +} + +func TestParseFullPipeline(t *testing.T) { + q, err := Parse(`service=api | where status>=500 | stats count(*) as errors by host | sort -errors`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if len(q.Pipes) != 3 { + t.Fatalf("expected 3 pipe stages, got %d: %+v", len(q.Pipes), q.Pipes) + } + + where, ok := q.Pipes[0].(ast.WhereStage) + if !ok { + t.Fatalf("stage 0: expected WhereStage, got %T", q.Pipes[0]) + } + cmp := where.Expr.Terms[0].(ast.Comparison) + if cmp.Field != "status" || cmp.Op != ">=" || cmp.Value != "500" { + t.Fatalf("unexpected where comparison: %+v", cmp) + } + + stats, ok := q.Pipes[1].(ast.StatsStage) + if !ok { + t.Fatalf("stage 1: expected StatsStage, got %T", q.Pipes[1]) + } + if len(stats.Aggs) != 1 || stats.Aggs[0].Func != "count" || stats.Aggs[0].Alias != "errors" { + t.Fatalf("unexpected stats aggs: %+v", stats.Aggs) + } + if len(stats.By) != 1 || stats.By[0] != "host" { + t.Fatalf("unexpected stats by: %+v", stats.By) + } + + sort, ok := q.Pipes[2].(ast.SortStage) + if !ok { + t.Fatalf("stage 2: expected SortStage, got %T", q.Pipes[2]) + } + if len(sort.Fields) != 1 || sort.Fields[0].Field != "errors" || !sort.Fields[0].Desc { + t.Fatalf("unexpected sort fields: %+v", sort.Fields) + } +} + +func TestParseExplicitFreeTextWithAggregation(t *testing.T) { + q, err := Parse(`message:"connection refused" | stats count by host`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + ft, ok := q.Base.Terms[0].(ast.FreeText) + if !ok { + t.Fatalf("expected FreeText, got %T", q.Base.Terms[0]) + } + if ft.Query != "connection refused" { + t.Fatalf("Query = %q, want %q", ft.Query, "connection refused") + } + stats := q.Pipes[0].(ast.StatsStage) + if stats.Aggs[0].Func != "count" || stats.Aggs[0].Field != "" { + t.Fatalf("unexpected agg: %+v", stats.Aggs[0]) + } +} + +func TestParseBareWordIsFreeText(t *testing.T) { + q, err := Parse(`timeout`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + ft, ok := q.Base.Terms[0].(ast.FreeText) + if !ok || ft.Query != "timeout" { + t.Fatalf("expected FreeText(timeout), got %+v", q.Base.Terms[0]) + } +} + +func TestParseImplicitAndBetweenBareTerms(t *testing.T) { + q, err := Parse(`error timeout`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if len(q.Base.Terms) != 2 { + t.Fatalf("expected 2 terms, got %d", len(q.Base.Terms)) + } + if len(q.Base.Conjs) != 1 || q.Base.Conjs[0] != "and" { + t.Fatalf("expected implicit 'and', got %+v", q.Base.Conjs) + } +} + +func TestParseExplicitAndOr(t *testing.T) { + q, err := Parse(`service=api and status=500`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if len(q.Base.Conjs) != 1 || q.Base.Conjs[0] != "and" { + t.Fatalf("expected explicit 'and', got %+v", q.Base.Conjs) + } + + q2, err := Parse(`service=api or service=web`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if len(q2.Base.Conjs) != 1 || q2.Base.Conjs[0] != "or" { + t.Fatalf("expected 'or', got %+v", q2.Base.Conjs) + } +} + +func TestParseTimeBoundsRelativeAndAbsolute(t *testing.T) { + q, err := Parse(`earliest=-1h latest="2026-08-14T00:00:00Z"`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if len(q.Base.Terms) != 2 { + t.Fatalf("expected 2 terms, got %d", len(q.Base.Terms)) + } + earliest := q.Base.Terms[0].(ast.TimeBound) + if earliest.Kind != "earliest" || !earliest.Expr.IsRelative || earliest.Expr.RelativeSign != -1 || + earliest.Expr.RelativeN != 1 || earliest.Expr.RelativeUnit != "h" { + t.Fatalf("unexpected earliest: %+v", earliest) + } + latest := q.Base.Terms[1].(ast.TimeBound) + if latest.Kind != "latest" || latest.Expr.Absolute != "2026-08-14T00:00:00Z" { + t.Fatalf("unexpected latest: %+v", latest) + } +} + +func TestParseFieldsStage(t *testing.T) { + q, err := Parse(`service=api | fields host, message, severity`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + fields := q.Pipes[0].(ast.FieldsStage) + want := []string{"host", "message", "severity"} + if len(fields.Fields) != len(want) { + t.Fatalf("Fields = %v, want %v", fields.Fields, want) + } + for i := range want { + if fields.Fields[i] != want[i] { + t.Fatalf("Fields = %v, want %v", fields.Fields, want) + } + } +} + +func TestParseHeadTailWithAndWithoutN(t *testing.T) { + q, err := Parse(`service=api | head 10`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + head := q.Pipes[0].(ast.HeadStage) + if !head.HasN || head.N != 10 { + t.Fatalf("unexpected head: %+v", head) + } + + q2, err := Parse(`service=api | tail`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + tail := q2.Pipes[0].(ast.TailStage) + if tail.HasN { + t.Fatalf("expected no N, got %+v", tail) + } +} + +func TestParseDottedFieldName(t *testing.T) { + q, err := Parse(`winevt.event_id=4625`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + cmp := q.Base.Terms[0].(ast.Comparison) + if cmp.Field != "winevt.event_id" || cmp.Value != "4625" { + t.Fatalf("unexpected comparison: %+v", cmp) + } +} + +func TestParseSortAscendingWithPlus(t *testing.T) { + q, err := Parse(`service=api | sort +host`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + sort := q.Pipes[0].(ast.SortStage) + if sort.Fields[0].Desc { + t.Fatalf("expected ascending sort, got %+v", sort.Fields[0]) + } +} + +func TestParseMultipleSortFields(t *testing.T) { + q, err := Parse(`service=api | sort -severity, +host`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + sort := q.Pipes[0].(ast.SortStage) + if len(sort.Fields) != 2 { + t.Fatalf("expected 2 sort fields, got %d", len(sort.Fields)) + } + if sort.Fields[0].Field != "severity" || !sort.Fields[0].Desc { + t.Fatalf("unexpected first sort field: %+v", sort.Fields[0]) + } + if sort.Fields[1].Field != "host" || sort.Fields[1].Desc { + t.Fatalf("unexpected second sort field: %+v", sort.Fields[1]) + } +} + +func TestParseMultipleAggregations(t *testing.T) { + q, err := Parse(`service=api | stats count() as n, avg(latency_ms) as avg_latency by host`) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + stats := q.Pipes[0].(ast.StatsStage) + if len(stats.Aggs) != 2 { + t.Fatalf("expected 2 aggs, got %d: %+v", len(stats.Aggs), stats.Aggs) + } + if stats.Aggs[1].Func != "avg" || stats.Aggs[1].Field != "latency_ms" || stats.Aggs[1].Alias != "avg_latency" { + t.Fatalf("unexpected second agg: %+v", stats.Aggs[1]) + } +} + +// --- error cases --- + +func TestParseErrorEmptyQuery(t *testing.T) { + if _, err := Parse(``); err == nil { + t.Fatal("expected an error for an empty query") + } +} + +func TestParseErrorUnknownPipeStage(t *testing.T) { + if _, err := Parse(`service=api | bogus`); err == nil { + t.Fatal("expected an error for an unknown pipe stage") + } +} + +func TestParseErrorMissingComparatorValue(t *testing.T) { + if _, err := Parse(`service=`); err == nil { + t.Fatal("expected an error for a missing comparison value") + } +} + +func TestParseErrorUnknownAggFunc(t *testing.T) { + if _, err := Parse(`service=api | stats median(latency)`); err == nil { + t.Fatal("expected an error for an unknown aggregation function") + } +} + +func TestParseErrorInvalidTimeUnit(t *testing.T) { + if _, err := Parse(`earliest=-1x`); err == nil { + t.Fatal("expected an error for an invalid time unit") + } +} + +func TestParseErrorTrailingGarbage(t *testing.T) { + if _, err := Parse(`service=api extra ) tokens`); err == nil { + t.Fatal("expected an error for trailing unparseable tokens") + } +} + +func TestParseErrorUnterminatedStringInFreeText(t *testing.T) { + if _, err := Parse(`message:"unterminated`); err == nil { + t.Fatal("expected an error for an unterminated quoted string") + } +} diff --git a/api/internal/querylang/planner/planner.go b/api/internal/querylang/planner/planner.go new file mode 100644 index 0000000..1888c4c --- /dev/null +++ b/api/internal/querylang/planner/planner.go @@ -0,0 +1,286 @@ +// Package planner compiles a query string (either syntax) into ir.Plan. +// This is the single entry point querylang exposes to callers (the /query +// HTTP handler) -- see Compile. +package planner + +import ( + "fmt" + "regexp" + "strings" + "time" + + "github.com/sentry/sentry/api/internal/querylang/ast" + "github.com/sentry/sentry/api/internal/querylang/ir" + "github.com/sentry/sentry/api/internal/querylang/parser" +) + +// Language selects which syntax a query is written in. +type Language string + +const ( + Auto Language = "" // detect from the query text (default) + SQL Language = "sql" + SPL Language = "spl" // the pipe syntax; named to match the query-language-reference doc +) + +const defaultLimit = 100 + +// Compile turns a query string into a Plan. language overrides +// auto-detection; pass Auto to use the SELECT-prefix heuristic (see +// /docs/query-language-design.md's "Detection" section) -- this exists +// for the rare case a pipe query legitimately starts with the literal +// word "select" as a bare search term. +func Compile(query string, language Language, now time.Time) (*ir.Plan, error) { + isSQL := language == SQL + if language == Auto { + isSQL = looksLikeSQL(query) + } + + if isSQL { + if err := validateSelectOnly(query); err != nil { + return nil, err + } + return &ir.Plan{RawSQL: strings.TrimSpace(strings.TrimSuffix(strings.TrimSpace(query), ";"))}, nil + } + + q, err := parser.Parse(query) + if err != nil { + return nil, err + } + return compileQuery(q, now) +} + +func looksLikeSQL(query string) bool { + trimmed := strings.TrimSpace(query) + if trimmed == "" { + return false + } + fields := strings.Fields(trimmed) + return len(fields) > 0 && strings.EqualFold(fields[0], "SELECT") +} + +func compileQuery(q *ast.Query, now time.Time) (*ir.Plan, error) { + plan := &ir.Plan{} + + textParts, err := compileBoolExpr(q.Base, plan, now) + if err != nil { + return nil, err + } + + for _, stage := range q.Pipes { + switch s := stage.(type) { + case ast.WhereStage: + parts, err := compileBoolExpr(s.Expr, plan, now) + if err != nil { + return nil, err + } + textParts = append(textParts, parts...) + case ast.StatsStage: + agg, err := compileStats(s) + if err != nil { + return nil, err + } + plan.Aggregation = agg + case ast.SortStage: + for _, f := range s.Fields { + plan.Sort = append(plan.Sort, ir.SortField{Field: f.Field, Desc: f.Desc}) + } + case ast.FieldsStage: + plan.Fields = append(plan.Fields, s.Fields...) + case ast.HeadStage: + n := defaultLimit + if s.HasN { + n = s.N + } + plan.Limit = &ir.Limit{N: n, Tail: false} + case ast.TailStage: + n := defaultLimit + if s.HasN { + n = s.N + } + plan.Limit = &ir.Limit{N: n, Tail: true} + default: + return nil, fmt.Errorf("internal error: unhandled pipe stage %T", stage) + } + } + + if len(textParts) > 0 { + plan.TextSearch = []ir.TextPredicate{{Query: strings.Join(textParts, " ")}} + } + + return plan, nil +} + +// textPart is one piece of a combined Tantivy query string, tagged with +// the conjunction that precedes it (empty for the first piece). +type textPart struct { + conj string // "", "and", "or" + query string +} + +// compileBoolExpr walks one bool_expr (the base search, or a `where` +// stage's expression), populating plan.Filters and plan.TimeRange +// directly, and returning free-text pieces for the caller to fold into +// the combined Tantivy query string. +// +// Scope decision: "or" is only supported between free-text terms, which +// Tantivy's own query parser handles natively once composed into one +// string. "or" between structured comparisons/time-bounds is rejected +// with a clear error rather than silently compiled as "and" -- see +// /docs/query-language-reference.md's limitations section. This keeps +// the executor's generated SQL a flat AND-only WHERE clause, which is +// most of what real queries need; full boolean-tree support for +// structured filters is future work if usage shows it's needed. +func compileBoolExpr(expr ast.BoolExpr, plan *ir.Plan, now time.Time) ([]string, error) { + var textParts []string + + for i, term := range expr.Terms { + conj := "" + if i > 0 { + conj = expr.Conjs[i-1] + } + + switch t := term.(type) { + case ast.Comparison: + if conj == "or" { + return nil, fmt.Errorf("query error: \"or\" is not supported between structured filters (%q) in Phase 2 -- only between free-text search terms", t.Field) + } + plan.Filters = append(plan.Filters, ir.FilterPredicate{Field: t.Field, Op: t.Op, Value: t.Value}) + case ast.TimeBound: + if conj == "or" { + return nil, fmt.Errorf("query error: \"or\" is not supported on time bounds (%s) in Phase 2", t.Kind) + } + if err := applyTimeBound(plan, t, now); err != nil { + return nil, err + } + case ast.FreeText: + q := t.Query + if strings.ContainsAny(q, " \t") { + q = `"` + strings.ReplaceAll(q, `"`, `\"`) + `"` + } + if conj == "or" { + textParts = append(textParts, "OR", q) + } else if len(textParts) > 0 { + textParts = append(textParts, "AND", q) + } else { + textParts = append(textParts, q) + } + default: + return nil, fmt.Errorf("internal error: unhandled term %T", term) + } + } + + return textParts, nil +} + +func applyTimeBound(plan *ir.Plan, t ast.TimeBound, now time.Time) error { + when, err := resolveTimeExpr(t.Expr, now) + if err != nil { + return err + } + if plan.TimeRange == nil { + plan.TimeRange = &ir.TimeRange{} + } + switch t.Kind { + case "earliest": + plan.TimeRange.From = when + case "latest": + plan.TimeRange.To = when + } + return nil +} + +func resolveTimeExpr(e ast.TimeExpr, now time.Time) (time.Time, error) { + if !e.IsRelative { + t, err := time.Parse(time.RFC3339, e.Absolute) + if err != nil { + return time.Time{}, fmt.Errorf("query error: invalid absolute timestamp %q, want RFC3339 (e.g. 2026-08-14T00:00:00Z): %w", e.Absolute, err) + } + return t, nil + } + + var d time.Duration + switch e.RelativeUnit { + case "s": + d = time.Duration(e.RelativeN) * time.Second + case "m": + d = time.Duration(e.RelativeN) * time.Minute + case "h": + d = time.Duration(e.RelativeN) * time.Hour + case "d": + d = time.Duration(e.RelativeN) * 24 * time.Hour + case "w": + d = time.Duration(e.RelativeN) * 7 * 24 * time.Hour + default: + return time.Time{}, fmt.Errorf("internal error: unknown time unit %q", e.RelativeUnit) + } + if e.RelativeSign < 0 { + d = -d + } + return now.Add(d), nil +} + +func compileStats(s ast.StatsStage) (*ir.Aggregation, error) { + agg := &ir.Aggregation{GroupBy: s.By} + seen := map[string]bool{} + for _, a := range s.Aggs { + if a.Func != "count" && a.Field == "" { + return nil, fmt.Errorf("query error: %s() requires a field, e.g. %s(latency_ms)", a.Func, a.Func) + } + alias := a.Alias + if alias == "" { + alias = defaultAggAlias(a) + } + if seen[alias] && a.Alias == "" { + // Two unnamed aggs of the same shape would otherwise collide + // (e.g. `stats sum(a), sum(b)` both defaulting to "sum") -- + // disambiguate by field name. + alias = alias + "_" + a.Field + } + seen[alias] = true + agg.Funcs = append(agg.Funcs, ir.AggFunc{Func: a.Func, Field: a.Field, Alias: alias}) + } + return agg, nil +} + +func defaultAggAlias(a ast.AggCall) string { + if a.Func == "count" { + return "count" + } + return a.Func +} + +// --- SQL escape hatch validation, ported from the Phase 0/1 +// api/internal/queryapi/validate.go guard it replaces (see task 4) --- + +// disallowedKeyword is defense-in-depth on top of the SELECT-only gate: +// it catches mutating/administrative statements appearing anywhere in +// the query, not just at the start. Word-boundary matching, not a real +// SQL parser -- same tradeoffs as the Phase 0/1 version this replaces. +var disallowedKeyword = regexp.MustCompile(`(?i)\b(insert|update|delete|alter|drop|truncate|create|grant|revoke|attach|detach|rename|kill|optimize|system|set|exchange|watch)\b`) + +func validateSelectOnly(sql string) error { + trimmed := strings.TrimSpace(sql) + if trimmed == "" { + return fmt.Errorf("query must not be empty") + } + + trimmed = strings.TrimSpace(strings.TrimSuffix(trimmed, ";")) + if trimmed == "" { + return fmt.Errorf("query must not be empty") + } + if strings.Contains(trimmed, ";") { + return fmt.Errorf("only a single statement is allowed") + } + + firstWord := strings.ToUpper(strings.Fields(trimmed)[0]) + if firstWord != "SELECT" { + return fmt.Errorf("only SELECT queries are allowed") + } + + if disallowedKeyword.MatchString(trimmed) { + return fmt.Errorf("query contains a disallowed keyword") + } + + return nil +} diff --git a/api/internal/querylang/planner/planner_test.go b/api/internal/querylang/planner/planner_test.go new file mode 100644 index 0000000..3dbcf2d --- /dev/null +++ b/api/internal/querylang/planner/planner_test.go @@ -0,0 +1,221 @@ +package planner + +import ( + "strings" + "testing" + "time" + + "github.com/sentry/sentry/api/internal/querylang/ir" +) + +var fixedNow = time.Date(2026, 8, 14, 12, 0, 0, 0, time.UTC) + +func TestCompileDetectsSQL(t *testing.T) { + plan, err := Compile(`SELECT * FROM logs LIMIT 10`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if plan.RawSQL == "" { + t.Fatalf("expected RawSQL to be set, got plan: %+v", plan) + } + if plan.RawSQL != "SELECT * FROM logs LIMIT 10" { + t.Fatalf("RawSQL = %q", plan.RawSQL) + } +} + +func TestCompileDetectsSQLCaseInsensitive(t *testing.T) { + plan, err := Compile(`select 1`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if plan.RawSQL != "select 1" { + t.Fatalf("RawSQL = %q", plan.RawSQL) + } +} + +func TestCompileRejectsNonSelectSQLKeyword(t *testing.T) { + _, err := Compile(`DELETE FROM logs`, SQL, fixedNow) + if err == nil { + t.Fatal("expected an error for a non-SELECT statement forced to SQL language") + } +} + +func TestCompileExplicitLanguageOverridesAutoDetect(t *testing.T) { + // "select" as a bare free-text search term -- would be misdetected + // as SQL by the heuristic alone, hence the override. + plan, err := Compile(`select`, SPL, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if plan.RawSQL != "" { + t.Fatalf("expected pipe-syntax compilation, got RawSQL = %q", plan.RawSQL) + } + if len(plan.TextSearch) != 1 || plan.TextSearch[0].Query != "select" { + t.Fatalf("expected a free-text search for 'select', got %+v", plan.TextSearch) + } +} + +func TestCompileSimpleFilter(t *testing.T) { + plan, err := Compile(`service=api`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + want := ir.FilterPredicate{Field: "service", Op: "=", Value: "api"} + if len(plan.Filters) != 1 || plan.Filters[0] != want { + t.Fatalf("unexpected filters: %+v, want [%+v]", plan.Filters, want) + } +} + +func TestCompileFullPipeline(t *testing.T) { + plan, err := Compile(`service=api | where status>=500 | stats count by host | sort -count`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if len(plan.Filters) != 2 { + t.Fatalf("expected 2 filters (service=api, status>=500), got %+v", plan.Filters) + } + if plan.Aggregation == nil || len(plan.Aggregation.Funcs) != 1 || plan.Aggregation.Funcs[0].Alias != "count" { + t.Fatalf("unexpected aggregation: %+v", plan.Aggregation) + } + if len(plan.Aggregation.GroupBy) != 1 || plan.Aggregation.GroupBy[0] != "host" { + t.Fatalf("unexpected group by: %+v", plan.Aggregation.GroupBy) + } + if len(plan.Sort) != 1 || plan.Sort[0].Field != "count" || !plan.Sort[0].Desc { + t.Fatalf("unexpected sort: %+v", plan.Sort) + } +} + +func TestCompileTextSearchWithAggregation(t *testing.T) { + plan, err := Compile(`message:"connection refused" | stats count by host`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if len(plan.TextSearch) != 1 || plan.TextSearch[0].Query != `"connection refused"` { + t.Fatalf("unexpected text search: %+v", plan.TextSearch) + } + if plan.Aggregation == nil { + t.Fatal("expected an aggregation") + } +} + +func TestCompileImplicitAndBetweenFreeTextTerms(t *testing.T) { + plan, err := Compile(`error timeout`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if len(plan.TextSearch) != 1 { + t.Fatalf("expected 1 combined text predicate, got %+v", plan.TextSearch) + } + if plan.TextSearch[0].Query != "error AND timeout" { + t.Fatalf("Query = %q, want %q", plan.TextSearch[0].Query, "error AND timeout") + } +} + +func TestCompileOrBetweenFreeTextTerms(t *testing.T) { + plan, err := Compile(`error or timeout`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if plan.TextSearch[0].Query != "error OR timeout" { + t.Fatalf("Query = %q, want %q", plan.TextSearch[0].Query, "error OR timeout") + } +} + +func TestCompileOrBetweenStructuredFiltersErrors(t *testing.T) { + _, err := Compile(`service=api or service=web`, Auto, fixedNow) + if err == nil { + t.Fatal("expected an error: OR between structured filters isn't supported in Phase 2") + } + if !strings.Contains(err.Error(), "or") { + t.Fatalf("error should mention 'or', got: %v", err) + } +} + +func TestCompileRelativeTimeBound(t *testing.T) { + plan, err := Compile(`earliest=-1h`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if plan.TimeRange == nil { + t.Fatal("expected a TimeRange") + } + want := fixedNow.Add(-1 * time.Hour) + if !plan.TimeRange.From.Equal(want) { + t.Fatalf("From = %v, want %v", plan.TimeRange.From, want) + } +} + +func TestCompileAbsoluteTimeBound(t *testing.T) { + plan, err := Compile(`latest="2026-08-14T00:00:00Z"`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + want := time.Date(2026, 8, 14, 0, 0, 0, 0, time.UTC) + if !plan.TimeRange.To.Equal(want) { + t.Fatalf("To = %v, want %v", plan.TimeRange.To, want) + } +} + +func TestCompileInvalidAbsoluteTimestampErrors(t *testing.T) { + _, err := Compile(`latest="not-a-timestamp"`, Auto, fixedNow) + if err == nil { + t.Fatal("expected an error for an invalid absolute timestamp") + } +} + +func TestCompileSumWithoutFieldErrors(t *testing.T) { + _, err := Compile(`service=api | stats sum`, Auto, fixedNow) + if err == nil { + t.Fatal("expected an error: sum() requires a field") + } +} + +func TestCompileAggAliasDefaultsAndCollisionIsDisambiguated(t *testing.T) { + plan, err := Compile(`service=api | stats sum(a), sum(b)`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if len(plan.Aggregation.Funcs) != 2 { + t.Fatalf("expected 2 agg funcs, got %+v", plan.Aggregation.Funcs) + } + if plan.Aggregation.Funcs[0].Alias == plan.Aggregation.Funcs[1].Alias { + t.Fatalf("expected distinct aliases, got both %q", plan.Aggregation.Funcs[0].Alias) + } +} + +func TestCompileHeadDefaultsLimit(t *testing.T) { + plan, err := Compile(`service=api | head`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if plan.Limit == nil || plan.Limit.N != defaultLimit || plan.Limit.Tail { + t.Fatalf("unexpected limit: %+v", plan.Limit) + } +} + +func TestCompileTailSetsTailFlag(t *testing.T) { + plan, err := Compile(`service=api | tail 5`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if plan.Limit == nil || plan.Limit.N != 5 || !plan.Limit.Tail { + t.Fatalf("unexpected limit: %+v", plan.Limit) + } +} + +func TestCompileFieldsProjection(t *testing.T) { + plan, err := Compile(`service=api | fields host, message`, Auto, fixedNow) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + if len(plan.Fields) != 2 || plan.Fields[0] != "host" || plan.Fields[1] != "message" { + t.Fatalf("unexpected fields: %+v", plan.Fields) + } +} + +func TestCompileParseErrorPropagates(t *testing.T) { + _, err := Compile(`service=api | bogus`, Auto, fixedNow) + if err == nil { + t.Fatal("expected a parse error to propagate") + } +} diff --git a/api/internal/searchclient/client.go b/api/internal/searchclient/client.go index 73c4a81..0b1d6ad 100644 --- a/api/internal/searchclient/client.go +++ b/api/internal/searchclient/client.go @@ -1,6 +1,8 @@ // Package searchclient adapts the generated gRPC SearchServiceClient to -// the narrow queryapi.searchClient interface, so queryapi doesn't need to -// know anything about gRPC/protobuf directly. +// the narrow querylang/executor.SearchClient interface (Search(ctx, +// query string, limit uint32) ([]string, error), satisfied structurally, +// no adapter type needed), so the query executor doesn't need to know +// anything about gRPC/protobuf directly. package searchclient import ( diff --git a/cli/README.md b/cli/README.md index 415e27b..4aed991 100644 --- a/cli/README.md +++ b/cli/README.md @@ -1,6 +1,6 @@ # sentryctl -Sentry's control CLI. Phase 0: a single command. +Sentry's control CLI. ```sh sentryctl ping # checks http://localhost:8080/healthz @@ -11,9 +11,24 @@ SENTRYCTL_API_URL=http://api.internal:8080 sentryctl ping Exits 0 and prints `ok` if `/api`'s `/healthz` responds 200; exits 1 with an error on `stderr` otherwise. -No CLI framework (cobra/urfave-cli/etc.) — a single command doesn't need -one, and stdlib `os.Args` handling is boring enough not to need a -dependency. Revisit once there's a real command tree to justify one. +```sh +sentryctl query 'service=api | where status>=500 | stats count by host' +sentryctl query 'SELECT * FROM logs LIMIT 10' --language sql +sentryctl query 'message:"connection refused"' --json +``` + +Quote the query in your shell — pipe syntax uses `|`, which your shell +interprets as an actual pipe if you don't. Hits the exact same `POST +/query` endpoint the web UI does (`internal/querylang` in `/api` does the +compiling; there's no separate query logic here to drift out of sync — +see `/docs/query-language-reference.md`). `--language` overrides +auto-detection, same optional override the HTTP API itself exposes. +Prints a table by default (stdlib `text/tabwriter`, no new dependency); +`--json` prints the raw `{columns, rows}` response instead. + +No CLI framework (cobra/urfave-cli/etc.) — two commands don't need one, +and stdlib `os.Args` handling is boring enough not to need a dependency. +Revisit once there's a real command tree to justify one. ## Building & testing diff --git a/cli/cmd/sentryctl/main.go b/cli/cmd/sentryctl/main.go index c6c3615..6b1ad1b 100644 --- a/cli/cmd/sentryctl/main.go +++ b/cli/cmd/sentryctl/main.go @@ -1,13 +1,18 @@ -// Command sentryctl is Sentry's control CLI. Phase 0: a single "ping" -// command that checks the api service is reachable. More commands land as -// the control plane grows real operations to expose. +// Command sentryctl is Sentry's control CLI: "ping" (Phase 0) and +// "query" (Phase 2), which accepts either query syntax and hits the same +// POST /query endpoint the web UI does -- no separate query logic here, +// per the Phase 2 task list's explicit instruction. package main import ( + "bytes" + "encoding/json" "fmt" "io" "net/http" "os" + "strings" + "text/tabwriter" "time" ) @@ -26,6 +31,8 @@ func run(args []string, stdout, stderr io.Writer) int { switch args[0] { case "ping": return cmdPing(args[1:], stdout, stderr) + case "query": + return cmdQuery(args[1:], stdout, stderr) case "-h", "--help", "help": usage(stdout) return 0 @@ -37,15 +44,28 @@ func run(args []string, stdout, stderr io.Writer) int { } func usage(w io.Writer) { - fmt.Fprintln(w, `sentryctl: Sentry control CLI (Phase 0: ping only) + fmt.Fprintln(w, `sentryctl: Sentry control CLI Usage: sentryctl ping [--api ] + sentryctl query "" [--api ] [--language sql|spl] [--json] Commands: - ping Checks that the api service is reachable via GET /healthz. + ping Checks that the api service is reachable via GET /healthz. + query Runs a query (pipe syntax or SQL) against POST /query and + prints the result as a table, or as JSON with --json. Quote + the query in your shell -- pipe syntax uses "|", which your + shell will otherwise interpret itself. ---api defaults to $SENTRYCTL_API_URL, or `+defaultAPIURL+` if unset.`) +--api defaults to $SENTRYCTL_API_URL, or `+defaultAPIURL+` if unset. +--language overrides auto-detection; omit it for the common case.`) +} + +func resolveAPIURL(env func(string) string) string { + if v := env("SENTRYCTL_API_URL"); v != "" { + return v + } + return defaultAPIURL } // parsePingArgs resolves the api base URL for ping: --api flag wins, then @@ -53,10 +73,7 @@ Commands: // as a function) and separate from the HTTP call so it's unit-testable // without a real environment or server. func parsePingArgs(args []string, env func(string) string) string { - apiURL := env("SENTRYCTL_API_URL") - if apiURL == "" { - apiURL = defaultAPIURL - } + apiURL := resolveAPIURL(env) for i := 0; i < len(args); i++ { if args[i] == "--api" && i+1 < len(args) { apiURL = args[i+1] @@ -85,3 +102,136 @@ func cmdPing(args []string, stdout, stderr io.Writer) int { fmt.Fprintln(stdout, "ok") return 0 } + +type queryArgs struct { + apiURL string + jsonOut bool + language string + query string +} + +// parseQueryArgs is pure (env passed in, no I/O), same testability +// reasoning as parsePingArgs. Non-flag arguments are joined with spaces +// to form the query, so `sentryctl query service=api status=500` (no +// quotes, no shell-special characters) works without requiring users to +// quote every query -- though anything using "|" still needs shell +// quoting regardless, since that's a real shell pipe character otherwise. +func parseQueryArgs(args []string, env func(string) string) queryArgs { + qa := queryArgs{apiURL: resolveAPIURL(env)} + var rest []string + for i := 0; i < len(args); i++ { + switch args[i] { + case "--api": + if i+1 < len(args) { + qa.apiURL = args[i+1] + i++ + } + case "--json": + qa.jsonOut = true + case "--language": + if i+1 < len(args) { + qa.language = args[i+1] + i++ + } + default: + rest = append(rest, args[i]) + } + } + qa.query = strings.Join(rest, " ") + return qa +} + +type queryRequestBody struct { + Query string `json:"query"` + Language string `json:"language"` +} + +type queryResponseBody struct { + Columns []string `json:"columns"` + Rows [][]any `json:"rows"` +} + +type errorResponseBody struct { + Error string `json:"error"` +} + +func cmdQuery(args []string, stdout, stderr io.Writer) int { + qa := parseQueryArgs(args, os.Getenv) + if strings.TrimSpace(qa.query) == "" { + fmt.Fprintln(stderr, "sentryctl query: missing query string") + return 1 + } + + reqBody, err := json.Marshal(queryRequestBody{Query: qa.query, Language: qa.language}) + if err != nil { + fmt.Fprintf(stderr, "encoding request: %v\n", err) + return 1 + } + + client := &http.Client{Timeout: 30 * time.Second} + resp, err := client.Post(qa.apiURL+"/query", "application/json", bytes.NewReader(reqBody)) + if err != nil { + fmt.Fprintf(stderr, "query failed: %v\n", err) + return 1 + } + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + fmt.Fprintf(stderr, "reading response: %v\n", err) + return 1 + } + + if resp.StatusCode != http.StatusOK { + var errResp errorResponseBody + if json.Unmarshal(respBody, &errResp) == nil && errResp.Error != "" { + fmt.Fprintf(stderr, "query failed: %s\n", errResp.Error) + } else { + fmt.Fprintf(stderr, "query failed: api returned status %d\n", resp.StatusCode) + } + return 1 + } + + if qa.jsonOut { + _, _ = stdout.Write(respBody) + fmt.Fprintln(stdout) + return 0 + } + + var result queryResponseBody + if err := json.Unmarshal(respBody, &result); err != nil { + fmt.Fprintf(stderr, "decoding response: %v\n", err) + return 1 + } + printTable(stdout, result.Columns, result.Rows) + return 0 +} + +func printTable(w io.Writer, columns []string, rows [][]any) { + tw := tabwriter.NewWriter(w, 0, 4, 2, ' ', 0) + fmt.Fprintln(tw, strings.Join(columns, "\t")) + for _, row := range rows { + cells := make([]string, len(row)) + for i, v := range row { + cells[i] = formatCell(v) + } + fmt.Fprintln(tw, strings.Join(cells, "\t")) + } + _ = tw.Flush() + fmt.Fprintf(w, "(%d row(s))\n", len(rows)) +} + +func formatCell(v any) string { + switch t := v.(type) { + case nil: + return "" + case map[string]any, []any: + b, err := json.Marshal(t) + if err != nil { + return fmt.Sprintf("%v", t) + } + return string(b) + default: + return fmt.Sprintf("%v", t) + } +} diff --git a/cli/cmd/sentryctl/main_test.go b/cli/cmd/sentryctl/main_test.go index bf7d1ee..d4cbe58 100644 --- a/cli/cmd/sentryctl/main_test.go +++ b/cli/cmd/sentryctl/main_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "encoding/json" "net/http" "net/http/httptest" "strings" @@ -121,3 +122,129 @@ func TestRunHelp(t *testing.T) { t.Fatalf("stdout should contain usage text, got %q", stdout.String()) } } + +func TestParseQueryArgsJoinsNonFlagArgsAsQuery(t *testing.T) { + env := func(string) string { return "" } + qa := parseQueryArgs([]string{"service=api", "status=500"}, env) + if qa.query != "service=api status=500" { + t.Errorf("query = %q", qa.query) + } + if qa.apiURL != defaultAPIURL { + t.Errorf("apiURL = %q, want default", qa.apiURL) + } + if qa.jsonOut { + t.Error("jsonOut should default to false") + } +} + +func TestParseQueryArgsFlags(t *testing.T) { + env := func(string) string { return "" } + qa := parseQueryArgs([]string{"--api", "http://h:1", "--language", "sql", "--json", "SELECT", "1"}, env) + if qa.apiURL != "http://h:1" { + t.Errorf("apiURL = %q", qa.apiURL) + } + if qa.language != "sql" { + t.Errorf("language = %q", qa.language) + } + if !qa.jsonOut { + t.Error("expected jsonOut = true") + } + if qa.query != "SELECT 1" { + t.Errorf("query = %q", qa.query) + } +} + +func TestCmdQueryMissingQueryErrors(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdQuery(nil, &stdout, &stderr) + if code != 1 { + t.Fatalf("exit code = %d, want 1", code) + } + if !strings.Contains(stderr.String(), "missing query") { + t.Fatalf("stderr = %q", stderr.String()) + } +} + +func TestCmdQueryTableOutput(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/query" { + t.Errorf("unexpected path %q", r.URL.Path) + } + var body queryRequestBody + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatalf("decoding request: %v", err) + } + if body.Query != "service=api" { + t.Errorf("query = %q", body.Query) + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(queryResponseBody{ + Columns: []string{"host", "count"}, + Rows: [][]any{{"h1", float64(3)}}, + }) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdQuery([]string{"--api", srv.URL, "service=api"}, &stdout, &stderr) + + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%s", code, stderr.String()) + } + out := stdout.String() + if !strings.Contains(out, "host") || !strings.Contains(out, "h1") || !strings.Contains(out, "(1 row(s))") { + t.Fatalf("unexpected table output: %q", out) + } +} + +func TestCmdQueryJSONOutput(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(queryResponseBody{Columns: []string{"c"}, Rows: [][]any{{"v"}}}) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdQuery([]string{"--api", srv.URL, "--json", "service=api"}, &stdout, &stderr) + + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%s", code, stderr.String()) + } + var got queryResponseBody + if err := json.Unmarshal(stdout.Bytes(), &got); err != nil { + t.Fatalf("stdout is not valid JSON: %v; got %q", err, stdout.String()) + } + if len(got.Columns) != 1 || got.Columns[0] != "c" { + t.Fatalf("unexpected JSON output: %+v", got) + } +} + +func TestCmdQueryServerErrorPrintsMessage(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(errorResponseBody{Error: "only SELECT queries are allowed"}) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdQuery([]string{"--api", srv.URL, "DELETE FROM logs", "--language", "sql"}, &stdout, &stderr) + + if code != 1 { + t.Fatalf("exit code = %d, want 1", code) + } + if !strings.Contains(stderr.String(), "only SELECT queries are allowed") { + t.Fatalf("stderr = %q, want it to include the server's error message", stderr.String()) + } +} + +func TestFormatCellHandlesNilMapAndSlice(t *testing.T) { + if got := formatCell(nil); got != "" { + t.Errorf("formatCell(nil) = %q, want empty", got) + } + if got := formatCell(map[string]any{"a": "b"}); got != `{"a":"b"}` { + t.Errorf("formatCell(map) = %q", got) + } + if got := formatCell(42.0); got != "42" { + t.Errorf("formatCell(42.0) = %q", got) + } +} diff --git a/docs/phase-2-runbook.md b/docs/phase-2-runbook.md new file mode 100644 index 0000000..fa5b6ec --- /dev/null +++ b/docs/phase-2-runbook.md @@ -0,0 +1,178 @@ +# Phase 2 runbook + +Extends `/docs/phase-0-runbook.md` and `/docs/phase-1-runbook.md` with +the unified query language and the "modest dataset" query-latency +benchmark. Read those first — this assumes the stack already works; +Phase 2 replaces the two placeholder query endpoints/pages with one, and +adds a way to actually measure query performance at volume. + +## What's actually been verified + +Unlike the phrasing "define 'modest', put a rough benchmark in the +runbook" might suggest, this isn't an estimate — the numbers below are +from actually generating 1,022,000 rows in a live ClickHouse (via +`/hack/benchmark-fixture`, pushed through the real agent-facing gRPC +endpoint, so both ClickHouse and Tantivy have real, matching data) and +timing real queries against it. One real bug turned up doing this (see +"What the benchmark caught" below) that wouldn't have been found any +other way. + +## 1. Bring up the stack + +```sh +docker compose up -d --build +``` + +Same as Phase 0/1. Confirm the unified endpoint works for both syntaxes: + +```sh +curl -X POST http://localhost:8080/query -H 'Content-Type: application/json' \ + -d '{"query": "SELECT 1"}' +# -> {"columns":["1"],"rows":[[1]]} + +curl -X POST http://localhost:8080/query -H 'Content-Type: application/json' \ + -d '{"query": "service=api"}' +# -> {"columns":[...],"rows":[]} (empty is fine -- no data ingested yet) +``` + +## 2. Generate the "modest" benchmark dataset + +"Modest" is defined here as **1,000,000 rows** — enough to be a real +volume test, small enough to generate and query interactively rather +than needing a dedicated load-testing pass. + +```sh +cd hack/benchmark-fixture +go run . --count 1000000 --batch-size 1000 --concurrency 16 +``` + +Sends batched `PushBatch` calls directly to `ingest`'s gRPC endpoint — +the same path the real agent uses, just generating synthetic data +instead of reading journald, so it exercises the *real* ingest → +Redpanda → ClickHouse-writer-consumer and → search-indexer-consumer +paths, not a shortcut that bypasses them. Concurrency matters: +sequential single-batch calls topped out around 500 records/sec (would +take ~33 minutes for 1M rows); 16 concurrent `PushBatch` calls over one +shared gRPC connection (HTTP/2 multiplexes concurrent RPCs over it +natively) got over 400,000 records/sec, so the actual measured run took +about 2 seconds for the ingest call itself. + +Wait for both consumers to fully drain before benchmarking — ingestion +finishing doesn't mean ClickHouse/Tantivy are caught up yet: + +```sh +# poll until this stops climbing +curl -s -X POST http://localhost:8080/query -H 'Content-Type: application/json' \ + -d '{"query": "SELECT count() FROM logs", "language": "sql"}' +``` + +In the run this runbook was written from, ClickHouse settled at +1,022,000 rows a few seconds after ingestion finished (the extra 22,000 +were earlier smaller test runs from developing the benchmark tool itself +— harmless, still real rows). + +## 3. Run the benchmark queries + +```sh +# structured filter + aggregation -- pure ClickHouse path +curl -s -o /dev/null -w "%{time_total}s\n" -X POST http://localhost:8080/query \ + -H 'Content-Type: application/json' \ + -d '{"query": "service=api | where status>=500 | stats count by host | sort -count"}' + +# free-text + aggregation -- Tantivy prefilter feeding a ClickHouse GROUP BY, +# the case /docs/query-language-design.md called "the hardest part" +curl -s -o /dev/null -w "%{time_total}s\n" -X POST http://localhost:8080/query \ + -H 'Content-Type: application/json' \ + -d '{"query": "message:\"connection refused\" | stats count by host | sort -count"}' +``` + +**Measured results against the 1,022,000-row dataset** (this exact run, +not a projection): + +| Query | Path | Wall time | +|---|---|---| +| `service=api \| where status>=500 \| stats count by host \| sort -count` | Pure ClickHouse | 19.7ms | +| `message:"connection refused" \| stats count by host \| sort -count` | Tantivy prefilter + ClickHouse aggregate | 46.4ms | +| `message:"connection refused" \| head 50` | Tantivy prefilter, no aggregation | 49.2ms | +| `SELECT count() FROM logs WHERE service='web'` (raw SQL) | Pure ClickHouse | 17.5ms | + +All four "well under a second" — the Phase 2 exit criteria in +`/CLAUDE.md`. The combined text+aggregation case (the one everyone should +be nervous about, since it's a two-backend query) came in at 46ms, not +meaningfully slower than the pure-ClickHouse case — the Tantivy prefilter +step is fast, and 5,000 UUIDs (see below) is a small `IN` clause by +ClickHouse's standards. + +## 4. What the benchmark caught + +The original design capped the Tantivy prefilter at **10,000** matching +`record_id`s before feeding them into ClickHouse's `WHERE record_id IN +(...)`. Running the actual combined-query benchmark against real data +(where "connection refused" matched a large fraction of the 1M rows, +by design — the fixture generator seeds that phrase deliberately) hit +this immediately: + +``` +query failed: executing query: code: 62, message: Syntax error: +failed at position 262116 [...] Max query size exceeded +``` + +10,000 quoted UUIDs (~39 bytes each including the separating comma) +produces a ~390KB query string, which exceeds ClickHouse's *default* +`max_query_size` (262144 bytes / 256KiB) — a much lower ceiling than +"multi-million-entry" suggested before anyone had actually tried it. The +cap is now **5,000** (~195KB, safely under the default with headroom) — +see `api/internal/querylang/executor/executor.go`'s `textSearchLimit` +and `/docs/query-language-design.md`'s "Known scaling limitation" +section, both updated with this exact finding rather than a +first-principles guess. + +## 5. Confirm the web UI and CLI both work end to end + +Open `http://localhost:3000` — one page now (Phase 0/1's two pages are +gone), run `service=api | where status>=500 | stats count by host | sort +-count` in the query bar, confirm results render and the query appears +in the session-local history panel. + +```sh +cd cli +go run ./cmd/sentryctl query 'service=api | where status>=500 | stats count by host | sort -count' +``` + +Both hit the exact same `POST /query` endpoint — there's no separate +query logic to drift out of sync between the CLI, the web UI, and +whatever else calls this API later. + +## Tearing down + +```sh +docker compose down -v # also wipes the 1M-row benchmark dataset +``` + +## Troubleshooting + +**A text-search query with aggregation fails with a ClickHouse syntax +error / "Max query size exceeded."** +If you've raised `textSearchLimit` above 5,000, you've likely +reintroduced the exact failure this runbook's benchmark caught (see +"What the benchmark caught" above) — lower it back down, or raise +ClickHouse's `max_query_size` server-side with matching memory sizing if +you genuinely need a larger prefilter. + +**Benchmark ingestion is much slower than ~400K records/sec.** +Check `--concurrency` wasn't left at a low value, and that `ingest` +isn't CPU-starved (`docker stats`) — the numbers above are from a single +machine running the whole stack including Redpanda/ClickHouse/search +concurrently, not a dedicated load-test environment, so your exact +throughput will vary with hardware. What matters is "well under a +second" for the actual query latency, not the ingestion throughput +number itself, which is just how the test data gets there. + +**`stats sum(field)`/`avg(field)` etc. on an attributes-map field +returns 0 for everything.** +Check the field's values are actually numeric strings — non-numeric +attribute values silently cast to 0 via `toFloat64OrZero` (see +`/docs/query-language-reference.md`'s "Field mapping" section). This is +documented behavior, not a bug, but it's easy to trip over with a typo'd +field name (which also "succeeds" with all zeros, since a missing map +key reads as an empty string, which also casts to 0). diff --git a/docs/query-language-design.md b/docs/query-language-design.md new file mode 100644 index 0000000..91b38d0 --- /dev/null +++ b/docs/query-language-design.md @@ -0,0 +1,250 @@ +# Query language design + +> **Status:** Design, approved 2026-08-14, not yet implemented (that's +> Task 3). This is the reference Task 3's implementation is built against +> — if implementation reveals this design is wrong somewhere, fix this +> doc in the same change, don't let them drift apart. + +## Why this design, in one paragraph + +Phases 0–1 shipped two disconnected, placeholder query paths: raw SQL +against ClickHouse, and free-text against Tantivy. Phase 2 needs one +query language that can express both filter/aggregation and free-text +search in a single query, without picking a winner between "give up +structured querying" and "give up full-text search." The approach below +does that by keeping parsing and execution strictly separate (a small +pipe-syntax grammar and an "opaque SQL" passthrough both compile to the +same IR) and by generalizing a mechanism Phase 1 already built and proved +works (Tantivy-prefilter → ClickHouse `IN (...)`) rather than inventing a +new cross-backend join strategy from scratch. + +## Grammar + +Pipe syntax, SPL-inspired, EBNF-ish: + +``` +query := base_search ("|" pipe_stage)* +base_search := bool_expr // implicit filter/search, SPL convention +pipe_stage := "where" bool_expr + | "stats" agg_call ("," agg_call)* ["by" field ("," field)*] + | "sort" sort_field ("," sort_field)* + | "fields" field ("," field)* + | "head" [INT] + | "tail" [INT] + +bool_expr := term (("and" | "or") term)* +term := field comparator value // structured filter -> ClickHouse + | "earliest" "=" time_expr // time range lower bound + | "latest" "=" time_expr // time range upper bound + | STRING | QUOTED_STRING // bare term -> free-text (Tantivy) on `message` + | "message" ":" QUOTED_STRING // explicit free-text (Tantivy phrase/wildcard syntax passed through) + +comparator := "=" | "!=" | ">" | ">=" | "<" | "<=" +agg_call := IDENT "(" [field] ")" ["as" IDENT] // count(), sum(field), avg(field), min(field), max(field) +sort_field := ["-" | "+"] field // "-" = desc (default), "+" = asc +time_expr := QUOTED_STRING // absolute RFC3339 + | "-" INT ("s"|"m"|"h"|"d"|"w") // relative to query time, e.g. -1h, -7d +field := IDENT +``` + +### Worked examples + +- `service=api | where status>=500 | stats count by host | sort -count` + — `service=api` is the base filter (structured, top-level column); + `where status>=500` filters on `status`, which isn't a top-level + column (see field mapping below); `stats count by host` aggregates; + `sort -count` orders descending by the aggregate's implicit `count` + alias. +- `message:"connection refused" | stats count by host` — free-text + predicate feeding a ClickHouse aggregation. This is the case task 2 + called "the hardest part" — see Execution below. +- `SELECT host, count(*) FROM logs GROUP BY host` — detected as SQL (see + Detection below), executed directly against ClickHouse. + +## Parser: hand-written recursive descent, no new dependency + +This grammar is small and stable — seven pipe-stage kinds, one +expression grammar for filters. A hand-written lexer + recursive-descent +parser beats a combinator library (e.g. `participle`) or a generator +(`goyacc`) here: + +- **No new dependency.** Consistent with "ask before adding a new + external dependency, there's no case for one at this grammar size. +- **Error messages matter** for a user-facing query language in a way + they don't for most internal parsing — "expected `by` after `stats + count`, got `sort`" is easy to produce by hand, harder to get right + through a combinator or generated parser. +- Generator tooling (`goyacc`) adds a codegen build step disproportionate + to a grammar this size. +- This is the standard approach for small, real query DSLs at this + scope — not a novel choice. + +## The SQL escape hatch: not parsed, wrapped as opaque IR + +"Both syntaxes compile to the same IR" does not mean writing a SQL +parser — reimplementing ClickHouse's SQL dialect would be a large, +pointless undertaking when ClickHouse already parses its own SQL. A query +that starts with `SELECT` (case-insensitive — see Detection) skips the +pipe-syntax parser entirely and produces an IR value that wraps the raw +SQL string as an opaque passthrough node. Both syntaxes still flow +through the same `Plan` type and the same executor code path — that's +what "same IR" actually buys (one execution and testing surface), not a +shared abstract syntax tree. The existing SELECT-only / single-statement +/ keyword-blocklist validation (`api/internal/queryapi/validate.go`) is +reused unchanged as the guard before wrapping. + +## IR (`Plan`) + +```go +type Plan struct { + RawSQL string // set => everything else is ignored; opaque ClickHouse passthrough + TextSearch []TextPredicate // bare terms / message: clauses -> routed to Tantivy + Filters []FilterPredicate // structured comparisons -> ClickHouse WHERE + TimeRange *TimeRange + Aggregation *Aggregation // nil => raw rows, no GROUP BY + Sort []SortField + Fields []string // projection; empty => all columns + Limit *Limit // head/tail +} + +type TextPredicate struct { + Query string // passed to Tantivy's query parser as-is +} + +type FilterPredicate struct { + Field string + Op string // "=", "!=", ">", ">=", "<", "<=" + Value string +} + +type Aggregation struct { + Funcs []AggFunc // count/sum/avg/min/max, each with an optional field + alias + GroupBy []string +} + +type AggFunc struct { + Func string + Field string // empty for count() + Alias string +} + +type SortField struct { + Field string + Desc bool +} + +type Limit struct { + N int + Tail bool // true = last N (by time), false = first N +} + +type TimeRange struct { + From, To time.Time // relative expressions (-1h etc.) resolved to absolute at compile time +} +``` + +## Field mapping: top-level columns vs. `attributes` + +`logs`' real columns (per `/storage`) are `timestamp, host, service, +severity, message, attributes, record_id`. Any field name in a query +that isn't one of those maps to `attributes['']` — e.g. +`status>=500` compiles to a comparison against `attributes['status']`, +not a top-level column, since `status` isn't promoted (Phase 1's decision +not to promote anything without real usage data still holds). Because +`attributes` is `Map(String,String)`, every stored value is a string; +numeric comparators against a non-top-level field cast via +`toFloat64OrZero(attributes['field'])` when the compared value looks +numeric, otherwise compare as string. This is what makes `where +status>=500` work against the existing schema with no migration — +querying an unpromoted field is always slightly more expensive than a +top-level column, which is worth knowing, not hiding. + +## Execution: routing between ClickHouse and Tantivy + +The core mechanism already exists and is proven: Phase 1's `/search` +endpoint (`api/internal/queryapi/search.go`, `recordIDsQuery`) already +does exactly steps 1–2 below for text-only queries. Phase 2 generalizes +it into four cases: + +1. **No `TextSearch` predicates** → pure ClickHouse path. Build one SQL + statement directly from `Filters`/`TimeRange`/`Aggregation`/`Sort`/ + `Fields`/`Limit`. The common case, and the fast path. +2. **`TextSearch` predicates, no `Aggregation`** → Phase 1's `/search` + behavior, generalized: Tantivy resolves matching `record_id`s, then + `SELECT ... WHERE record_id IN (...)` for the rows, with `Filters`/ + `TimeRange`/`Sort`/`Fields`/`Limit` folded into that same statement. +3. **`TextSearch` predicates *and* `Aggregation`** — the genuinely new + case (`message:"connection refused" | stats count by host`): Tantivy + resolves matching `record_id`s as a *prefilter*, not a join, then one + ClickHouse statement does `WHERE record_id IN (...) AND + GROUP BY <...>`. Aggregation always happens in ClickHouse; Tantivy + only ever narrows which rows are eligible before that. +4. **`RawSQL` set** → executed as-is against ClickHouse, no Tantivy + involvement regardless of what the SQL contains. The escape hatch is + opaque by design — no attempt to detect free-text intent inside raw + SQL. + +### Known scaling limitation + +Steps 2/3's `record_id IN (...)` approach breaks down if a text search +matches a large number of rows — the `IN` clause is a literal, quoted +UUID list embedded in the query string. Phase 2's mitigation: cap the +Tantivy prefilter at **5,000** results. Tantivy's `TopDocs` already +returns most-relevant-first, so the cap keeps the *best* matches rather +than an arbitrary truncation, but it's a real limitation on result +completeness for very broad text searches combined with aggregation. +Documented in `/docs/query-language-reference.md`, not silently +swallowed. + +This number isn't a first-principles estimate — running the Phase 2 +benchmark against a real 1M-row dataset (`/docs/phase-2-runbook.md`) +caught the original 10,000 cap failing outright: 10,000 quoted UUIDs +(~39 bytes each) produces a ~390KB query string, which exceeds +ClickHouse's *default* `max_query_size` (262144 bytes / 256KiB) and +fails with a syntax error rather than degrading gracefully — a much +lower ceiling than "multi-million-entry" suggested before anyone had +actually tried it. 5,000 UUIDs (~195KB) stays safely under that default +with headroom. The real long-term fix (streaming `record_id` batches, +ClickHouse-side text indexing, a different join strategy, or simply +raising `max_query_size` server-side with matching memory sizing) is +explicitly future work, out of scope for Phase 2. + +## Where this lives: `api/internal/querylang/` + +Not a new top-level component. This subsystem always executes in-process +within `/api` — it doesn't run standalone, doesn't get its own Docker +image, and needs both connections `/api` already holds (the ClickHouse +driver, the search gRPC client). A new top-level directory would imply a +new deployable service, which this isn't. + +``` +api/internal/querylang/ + lexer/ tokenizer + ast/ parsed pipe-syntax tree + parser/ tokens -> ast (recursive descent) + ir/ Plan and supporting types + planner/ ast -> Plan (field-mapping rule, SQL-passthrough detection) + executor/ Plan -> results (the four-case routing above) +``` + +Mirrors the existing `internal/queryapi`, `internal/searchclient` +convention already in `/api`. Each layer is independently testable per +task 3's requirement: "pipe syntax X compiles to IR Y" tests live in +`parser`/`planner` against fixture ASTs/Plans, no backend needed; "IR Y +executes correctly" tests live in `executor` against fakes for both the +ClickHouse and search-client interfaces, same pattern already used +throughout `/ingest` and `/api`. + +## `/query` endpoint: auto-detect, with an explicit override + +Detection: a request body's query starting with `SELECT` +(case-insensitive, same rule `validateSelectOnly` already applies) is +SQL; otherwise pipe syntax. Covers the overwhelming common case with no +extra field required. An optional `"language": "sql" | "spl"` field in +the request body overrides detection, for the rare case a pipe query +legitimately starts with the literal word "select" as a bare search +term. Auto-detect-with-override matches the shape of other +inference-with-explicit-override choices already made in this stack +(e.g. severity hints winning over parsed values when present) — good +default ergonomics, no silent ambiguity once a caller cares enough to be +explicit. diff --git a/docs/query-language-reference.md b/docs/query-language-reference.md new file mode 100644 index 0000000..7634772 --- /dev/null +++ b/docs/query-language-reference.md @@ -0,0 +1,322 @@ +# Query language reference + +Sentry has one query language for everything: filtering, free-text +search, and aggregation, in a single query, against a single endpoint +(`POST /query`), from a single query bar in the web UI or `sentryctl +query` on the command line. You don't pick a "search mode" or a +"reporting mode" first — you write one query, and Sentry figures out +which parts need ClickHouse, which parts need the full-text index, and +combines them. + +If you already know Splunk's SPL, most of this will feel immediately +familiar: a base search, piped through a sequence of processing stages. +Sentry's language is a deliberately smaller subset — the operators +people actually use day to day, not SPL's full surface area — plus raw +SQL as an escape hatch for anything the pipe syntax doesn't (yet) cover. + +## The shape of a query + +``` + | | | ... +``` + +Everything before the first `|` is the base search — a filter and/or a +free-text search. Everything after each `|` is a processing stage that +narrows, reshapes, or summarizes what came before it. + +``` +service=api | where status>=500 | stats count by host | sort -count +``` + +Read left to right: start with everything logged by the `api` service, +keep only the entries with `status >= 500`, count how many there are per +`host`, and show the busiest hosts first. + +## Filtering + +``` +field=value +field!=value +field>value +field>=value +field=500 +host!=host-03 +``` + +Multiple filters combine with `and` (the default when you don't write a +conjunction at all — see "Combining terms" below): + +``` +service=api status>=500 +service=api and status>=500 (equivalent) +``` + +## Free-text search + +Three ways to search the `message` field's text: + +``` +timeout a single bare word +"connection refused" a quoted phrase +message:"connection refused" the same thing, explicit +``` + +Free-text search is powered by Sentry's full-text index (Tantivy), which +supports phrase matching and wildcards: + +``` +message:"exact phrase" +message:"time*" +``` + +Bare words and quoted phrases can be mixed freely with structured filters +in the same query — that's the whole point of having one language: + +``` +service=api "connection refused" +message:"connection refused" | stats count by host +``` + +## Combining terms: `and` / `or` + +Adjacent terms with nothing between them are implicitly `and`ed, matching +what most people expect from a search bar: + +``` +error timeout same as: error and timeout +``` + +`or` works between free-text terms, and Sentry's full-text index handles +it natively: + +``` +error or timeout +``` + +**Current limitation:** `or` is not supported between structured filters +(`service=api or service=web` returns a clear error rather than silently +being treated as `and`). If you need this, use two separate queries for +now, or the raw SQL escape hatch. This is a known gap, not an oversight — +full boolean-tree support for structured filters is on the list for a +future release once there's real usage data on how much it's needed. + +## Time ranges + +``` +earliest=-1h relative: last hour +earliest=-15m latest=-5m relative window +earliest="2026-08-14T00:00:00Z" absolute (RFC 3339) +``` + +Relative offsets: a number followed by `s` (seconds), `m` (minutes), `h` +(hours), `d` (days), or `w` (weeks), always relative to when the query +runs. + +## Pipe stages + +### `where` — additional filtering after the base search + +Same syntax as the base search's filter terms: + +``` +service=api | where status>=500 +``` + +### `stats` — aggregation + +``` +stats count by host +stats count(), avg(latency_ms) as avg_latency by host, service +``` + +Supported functions: `count`, `sum`, `avg`, `min`, `max`. `count` doesn't +need a field (`count`, `count()`, and `count(*)` are all equivalent); +every other function requires one (`sum(latency_ms)`). Give a result an +explicit name with `as`, or accept the default (the function name, or +`count` for a bare count). + +``` +stats sum(bytes_sent) as total_bytes by host +``` + +### `sort` — ordering + +``` +sort -count descending by count (the default direction) +sort +host ascending by host +sort -severity, +host descending by severity, then ascending by host +``` + +`-` and `+` mean the same thing they do in most search tools: `-` for +descending, `+` for ascending. No sign at all also means descending. +You can sort by any field from the base data, or by a `stats` result's +column name/alias. + +### `fields` — choosing which columns come back + +``` +fields host, message, severity +``` + +Without `fields`, you get every column. + +### `head` / `tail` — limiting results + +``` +head first 100 (the default) results +head 20 first 20 +tail 50 last 50, chronologically +``` + +## Field mapping: what's a "real" column vs. an attribute + +Sentry's structured columns are `timestamp`, `host`, `service`, +`severity`, `message`, and `record_id`. Anything else you reference by +name — `status`, `latency_ms`, `winevt.event_id`, whatever your logs +happen to carry — is looked up in the per-record attributes, which are +always stored as text. + +This matters for comparisons: `status>=500` only makes sense as a number, +so Sentry casts the attribute's text value to a number for you +automatically when the value you're comparing against looks numeric. +`status="unknown"` compares as text instead, since `"unknown"` isn't a +number. You don't need to do anything differently — this happens based +on what you write on the right-hand side of the comparison — but it's +worth knowing that: + +- A field that's missing entirely, or whose value isn't actually numeric, + reads as `0` in a numeric comparison or aggregation (`toFloat64OrZero` + semantics) rather than erroring. A typo'd field name will "succeed" + with everything showing as `0` — if a `stats sum(...)` looks + suspiciously empty, double-check the field name. +- `stats min()`/`max()` on an attribute field always compares + numerically, not alphabetically, in this release. +- Querying an attribute is always a little more work for ClickHouse than + querying a real column — if a field turns out to be central to how you + query your logs, that's a signal it might be worth promoting to a real + column in a future schema change (not something you can do yourself + today). + +## Raw SQL + +Anything starting with `SELECT` is treated as raw ClickHouse SQL and run +directly, no pipe-syntax parsing involved: + +``` +SELECT host, count(*) FROM logs WHERE service = 'api' GROUP BY host +``` + +SELECT-only, single statement — Sentry allowlists this at the API level. +Use this for anything the pipe syntax doesn't cover yet: window +functions, `WITH` clauses, ClickHouse-specific functions, joins across +other tables you've added, and so on. There's no performance penalty for +using SQL over the pipe syntax or vice versa — both compile to the same +execution plan internally. + +## Which syntax am I using? + +Sentry detects automatically: a query starting with `SELECT` runs as +SQL, anything else runs as the pipe syntax. This covers the overwhelming +majority of real queries with no extra step. If you're writing a pipe +query that happens to start with the literal word "select" as a search +term, set the language explicitly instead of relying on detection: + +```json +{"query": "select", "language": "spl"} +``` + +`language` accepts `"sql"`, `"spl"`, or can be omitted entirely (the +default, auto-detect). The web UI's query bar shows which one it +detected next to the query box, with a dropdown to override it. + +## Combining free-text search with aggregation + +This is the case that makes Sentry's query language more than "SQL with +extra steps" — free text and aggregation, together, in one query: + +``` +message:"connection refused" | stats count by host +``` + +Under the hood: the full-text index resolves which records match the +text search first, then ClickHouse does the counting and grouping over +just those records. You don't need to know this to use it — it's +mentioned here because of the one limitation it implies: + +**A single free-text search is capped at 5,000 matching records** when +it's combined with a `stats`/filter stage that needs to know exactly +which records matched (the most-relevant 5,000, not an arbitrary +truncation). A text search alone, with no aggregation, isn't affected by +this cap. If your combined query's text search is broad enough to match +more than 5,000 records, narrow it — a more specific phrase, an added +`where` filter, or a tighter time range — the same way you'd narrow an +overly broad search in any tool. + +## Response shape + +Every query, regardless of syntax or which backend(s) it touched, +returns the same shape: + +```json +{"columns": ["host", "count"], "rows": [["api-01", 42], ["api-02", 17]]} +``` + +or, on error: + +```json +{"error": "a description of what went wrong"} +``` + +## Quick reference + +| Syntax | Meaning | +|---|---| +| `field=value` | equals | +| `field!=value` | not equals | +| `field>value` / `>=` / `<` / `<=` | comparison | +| `"phrase"` / bare word | free-text search on `message` | +| `message:"phrase"` | explicit free-text search | +| `earliest=-1h` / `latest=...` | time range | +| `\| where ...` | additional filter | +| `\| stats count by field` | aggregate | +| `\| sort -field` / `+field` | sort desc / asc | +| `\| fields a, b` | choose columns | +| `\| head N` / `\| tail N` | limit results | +| `SELECT ...` | raw SQL | + +## Examples + +``` +service=api | where status>=500 | stats count by host | sort -count +``` +Which hosts are producing the most 5xx errors from the `api` service? + +``` +message:"connection refused" | stats count by host +``` +Where are connection-refused errors coming from? + +``` +earliest=-24h severity=ERROR | stats count by service | sort -count +``` +Error volume by service over the last day. + +``` +winevt.event_id=4625 | fields host, message | head 20 +``` +Recent failed Windows logon attempts (a `winevt.*` attribute from the +Windows Event Log source — see `/docs/phase-1-runbook.md`). + +``` +SELECT host, avg(toFloat64OrZero(attributes['latency_ms'])) AS avg_latency +FROM logs WHERE service = 'api' GROUP BY host ORDER BY avg_latency DESC +``` +The same kind of query the pipe syntax's `stats avg(latency_ms) by host` +would produce, written by hand — useful as a starting point if you need +something the pipe syntax doesn't support yet. diff --git a/hack/benchmark-fixture/go.mod b/hack/benchmark-fixture/go.mod new file mode 100644 index 0000000..8c05d9d --- /dev/null +++ b/hack/benchmark-fixture/go.mod @@ -0,0 +1,18 @@ +module github.com/sentry/sentry/hack/benchmark-fixture + +go 1.25.0 + +replace github.com/sentry/sentry/proto => ../../proto + +require ( + github.com/sentry/sentry/proto v0.0.0-00010101000000-000000000000 + google.golang.org/grpc v1.83.0 +) + +require ( + golang.org/x/net v0.55.0 // indirect + golang.org/x/sys v0.45.0 // indirect + golang.org/x/text v0.37.0 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa // indirect + google.golang.org/protobuf v1.36.12 // indirect +) diff --git a/hack/benchmark-fixture/go.sum b/hack/benchmark-fixture/go.sum new file mode 100644 index 0000000..fcc6745 --- /dev/null +++ b/hack/benchmark-fixture/go.sum @@ -0,0 +1,38 @@ +github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= +github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= +github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= +github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= +github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= +github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= +github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= +go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= +go.opentelemetry.io/otel v1.44.0 h1:JjwHmHpA4iZ3wBxluu2fbbE7j4kqlE8jXyAyPXH7HqU= +go.opentelemetry.io/otel v1.44.0/go.mod h1:BMgjTHL9WPRlRjL2oZCBTL4whCGtXch2H4BhOPIAyYc= +go.opentelemetry.io/otel/metric v1.44.0 h1:1w0gILTcHdr3YI+ixLyjemwrVnsMURbTZFrSYCdDdmc= +go.opentelemetry.io/otel/metric v1.44.0/go.mod h1:8O7hanEPBNgEMmybD3s2VBKcgWOCsA6tzHBPODAiquo= +go.opentelemetry.io/otel/sdk v1.44.0 h1:nHYwb9lK+fJPU/dnT6s7W7Z8itMWyqrnVfbheVYrZ58= +go.opentelemetry.io/otel/sdk v1.44.0/go.mod h1:Osuydd3Se74nqjAKxid74N5eC+jfEqfTegHRnq58oK0= +go.opentelemetry.io/otel/sdk/metric v1.44.0 h1:3LlKgI+VjbVsjNRFZJZAJ30WjXC5VkNRks6si09iEfI= +go.opentelemetry.io/otel/sdk/metric v1.44.0/go.mod h1:5B5pMARnXxKhltooO4xUuCBorl65a4EpnTalObqOigA= +go.opentelemetry.io/otel/trace v1.44.0 h1:jxF5CsGYCe74MCRx2X4g7WsY/VBKRqqpNvXlX/6gtIk= +go.opentelemetry.io/otel/trace v1.44.0/go.mod h1:oLl1jrMQAVo6v3GAggN+1VH9VIz9iUSvW53sW1Q8PIE= +golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= +golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= +golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= +gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= +gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa h1:mZHHdPZl0dbGHCflZgAq/Q468DWVFcU2whhB2KAo8fk= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= +google.golang.org/grpc v1.83.0 h1:JeNZEKJFbQxArAMl+hiytHauacDNqJUllNfmIMmpqnQ= +google.golang.org/grpc v1.83.0/go.mod h1:kDyl6SKsiHKt0uylY5gtn5cEjkrIOhQOGDgIc4JGwzQ= +google.golang.org/protobuf v1.36.12 h1:pJOKDDOyeXErUroCihFAd5LQuwXBSpVnKGrj5o/fwxc= +google.golang.org/protobuf v1.36.12/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= diff --git a/hack/benchmark-fixture/main.go b/hack/benchmark-fixture/main.go new file mode 100644 index 0000000..0f63877 --- /dev/null +++ b/hack/benchmark-fixture/main.go @@ -0,0 +1,172 @@ +// Command benchmark-fixture pushes a large, realistically varied +// synthetic dataset directly to ingest's gRPC endpoint, batched, so +// Phase 2's "modest dataset" query-latency benchmark +// (/docs/phase-2-runbook.md) has real data to measure against instead of +// an asserted number. Distinct from /hack/windows-fixture: that one +// sends a handful of realistic Windows events to test pipeline +// *correctness*; this one sends a lot of Linux-shaped events to test +// query *performance* at volume. +package main + +import ( + "context" + "crypto/tls" + "crypto/x509" + "flag" + "fmt" + "math/rand" + "os" + "sync" + "sync/atomic" + "time" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials" + + logsv1 "github.com/sentry/sentry/proto/sentry/logs/v1" +) + +var ( + services = []string{"api", "web", "worker", "db", "auth"} + hosts = []string{"host-01", "host-02", "host-03", "host-04", "host-05", "host-06", "host-07", "host-08"} + severites = []logsv1.Severity{ + logsv1.Severity_SEVERITY_DEBUG, + logsv1.Severity_SEVERITY_INFO, + logsv1.Severity_SEVERITY_INFO, + logsv1.Severity_SEVERITY_INFO, + logsv1.Severity_SEVERITY_WARN, + logsv1.Severity_SEVERITY_ERROR, + } + // A mix of messages, some containing terms worth full-text + // searching for (connection refused, timeout) so the benchmark's + // text-search-plus-aggregation case has real matches to find, not + // just structured rows. + messages = []string{ + "request completed successfully", + "connection refused by upstream", + "request timeout after 30s", + "cache miss, falling back to database", + "connection refused: too many open connections", + "user authentication succeeded", + "slow query detected: timeout approaching", + "health check passed", + "retrying after connection refused error", + "scheduled job completed", + } +) + +func main() { + addr := flag.String("addr", "localhost:4317", "ingest gRPC address") + caFile := flag.String("ca", "../dev-certs/out/ca.pem", "CA cert path") + certFile := flag.String("cert", "../dev-certs/out/client.pem", "client cert path") + keyFile := flag.String("key", "../dev-certs/out/client-key.pem", "client key path") + count := flag.Int("count", 1_000_000, "total number of records to generate") + batchSize := flag.Int("batch-size", 1000, "records per PushBatch call") + concurrency := flag.Int("concurrency", 16, "concurrent PushBatch calls in flight") + flag.Parse() + + tlsConf, err := loadTLSConfig(*caFile, *certFile, *keyFile) + if err != nil { + fmt.Fprintln(os.Stderr, "loading TLS config:", err) + os.Exit(1) + } + + // One shared connection: gRPC multiplexes concurrent RPCs over HTTP/2 + // streams on a single connection, so concurrency here comes from + // concurrent PushBatch calls, not from opening more connections. + conn, err := grpc.NewClient(*addr, grpc.WithTransportCredentials(credentials.NewTLS(tlsConf))) + if err != nil { + fmt.Fprintln(os.Stderr, "dialing ingest:", err) + os.Exit(1) + } + defer conn.Close() + + client := logsv1.NewLogIngestClient(conn) + + numBatches := (*count + *batchSize - 1) / *batchSize + batchIndexes := make(chan int, numBatches) + for i := 0; i < numBatches; i++ { + batchIndexes <- i + } + close(batchIndexes) + + var sent atomic.Int64 + var wg sync.WaitGroup + start := time.Now() + + for w := 0; w < *concurrency; w++ { + wg.Add(1) + go func() { + defer wg.Done() + for batchIdx := range batchIndexes { + offset := batchIdx * *batchSize + n := *batchSize + if remaining := *count - offset; remaining < n { + n = remaining + } + records := make([]*logsv1.LogRecord, n) + for i := range records { + records[i] = randomRecord() + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + resp, err := client.PushBatch(ctx, &logsv1.PushBatchRequest{ + BatchId: fmt.Sprintf("benchmark-%d", batchIdx), + Records: records, + }) + cancel() + if err != nil { + fmt.Fprintf(os.Stderr, "PushBatch %d failed: %v\n", batchIdx, err) + os.Exit(1) + } + + total := sent.Add(int64(resp.GetAccepted())) + if total%int64(*batchSize*50) < int64(*batchSize) { + elapsed := time.Since(start) + rate := float64(total) / elapsed.Seconds() + fmt.Printf("sent %d/%d (%.0f records/sec)\n", total, *count, rate) + } + } + }() + } + wg.Wait() + + elapsed := time.Since(start) + total := sent.Load() + fmt.Printf("done: %d records in %s (%.0f records/sec)\n", total, elapsed, float64(total)/elapsed.Seconds()) +} + +func randomRecord() *logsv1.LogRecord { + return &logsv1.LogRecord{ + TimestampUnixNano: time.Now().UnixNano(), + Host: hosts[rand.Intn(len(hosts))], + Service: services[rand.Intn(len(services))], + Severity: severites[rand.Intn(len(severites))], + Message: messages[rand.Intn(len(messages))], + Attributes: map[string]string{ + "status": fmt.Sprintf("%d", []int{200, 200, 200, 301, 404, 500, 503}[rand.Intn(7)]), + "latency_ms": fmt.Sprintf("%d", rand.Intn(2000)), + }, + } +} + +func loadTLSConfig(caFile, certFile, keyFile string) (*tls.Config, error) { + caPEM, err := os.ReadFile(caFile) + if err != nil { + return nil, fmt.Errorf("reading CA cert %s: %w", caFile, err) + } + caPool := x509.NewCertPool() + if !caPool.AppendCertsFromPEM(caPEM) { + return nil, fmt.Errorf("no valid certificates found in %s", caFile) + } + + cert, err := tls.LoadX509KeyPair(certFile, keyFile) + if err != nil { + return nil, fmt.Errorf("loading client cert/key: %w", err) + } + + return &tls.Config{ + RootCAs: caPool, + Certificates: []tls.Certificate{cert}, + }, nil +} diff --git a/web/src/routes/+layout.svelte b/web/src/routes/+layout.svelte index 8161d42..8098bbb 100644 --- a/web/src/routes/+layout.svelte +++ b/web/src/routes/+layout.svelte @@ -1,6 +1,5 @@ @@ -10,32 +9,4 @@ - - {@render children()} - - diff --git a/web/src/routes/+page.svelte b/web/src/routes/+page.svelte index 6302290..aa1488b 100644 --- a/web/src/routes/+page.svelte +++ b/web/src/routes/+page.svelte @@ -1,19 +1,61 @@
-

Sentry — Log Query

+

Sentry — Query

- Raw SQL only, SELECT statements against the logs table. No auth, no query - builder yet — see /api for what's actually allowed. Looking for free-text - search instead? See the Full-Text Search page. + One query bar for both filter/stats queries and free-text search — see + /docs/query-language-reference.md in the repo for the full syntax, or the cheat + sheet below.

- -
- + ⌘/Ctrl+Enter to run
{#if error} @@ -64,6 +137,39 @@ {/if} + + {#if history.length > 0} +
+ Query history ({history.length}) +
    + {#each history as entry (entry.at)} +
  • + +
    • + {/each} +
+
+ {/if} + +
+ Pipe syntax cheat sheet + + + + + + + + + + + + +
field=valuefilter on a structured field
"free text" / bare wordfull-text search on message
message:"exact phrase"explicit full-text search
| where field>valueadditional structured filter
| stats count by fieldaggregate (count/sum/avg/min/max)
| sort -fieldsort descending (+field for ascending)
| fields a, bproject specific columns
| head 50 / | tail 50limit results
earliest=-1h / latest=...time range (relative or RFC3339)
+

Full reference: /docs/query-language-reference.md in the repo.

+
diff --git a/web/src/routes/search/+page.svelte b/web/src/routes/search/+page.svelte deleted file mode 100644 index 03187b9..0000000 --- a/web/src/routes/search/+page.svelte +++ /dev/null @@ -1,96 +0,0 @@ - - -
-

Sentry — Full-Text Search

-

- Free-text search over the message field, via Tantivy. Supports plain terms, - "exact phrases", and wildcard* — see - /search for the full query syntax. Looking for structured/aggregation queries - instead? See the SQL Query page. -

- - e.key === 'Enter' && runSearch()} - /> -
- -
- - {#if error} -

Error: {error}

- {/if} - - -
- - diff --git a/web/src/routes/search/+page.ts b/web/src/routes/search/+page.ts deleted file mode 100644 index 8a3e476..0000000 --- a/web/src/routes/search/+page.ts +++ /dev/null @@ -1,4 +0,0 @@ -// Same reasoning as the root page's +page.ts: no load function (all data -// comes from a client-side fetch on submit), so a plain prerender is -// enough for the static adapter. -export const prerender = true;