Skip to content

fix(query-engine): charts span the service pages' longest presets #2325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Makisuo merged 1 commit into from
Nov 7, 2026
+91 −9
Merged

fix(query-engine): charts span the service pages' longest presets #2325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions packages/backend/src/services/warehouse/QueryEngineService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,71 @@ describe("makeQueryEngineExecute", () => {
}),
)

it.effect("serves a timeseries over the service pages' longest presets", () =>
Effect.gen(function* () {
let queried = false
const execute = makeQueryEngineExecute(
makeTinybirdStub({
sqlQuery: () =>
Effect.sync(() => {
queried = true
return []
}),
}),
)
const exit = yield* Effect.exit(
execute(
tenant,
new QueryEngineExecuteRequest({
startTime: "2025-11-06 00:00:00",
endTime: "2026-11-07 01:00:00", // "12mo" plus the DST hour
query: {
kind: "timeseries",
source: "traces",
metric: "count",
allMetrics: true,
groupBy: ["service"],
filters: { rootSpansOnly: true },
bucketSeconds: 86400,
},
}),
),
)

assert.isTrue(Exit.isSuccess(exit))
assert.isTrue(queried)
}),
)

it.effect("still refuses a breakdown wider than its own ceiling", () =>
Effect.gen(function* () {
const execute = makeQueryEngineExecute(
makeTinybirdStub({ sqlQuery: () => Effect.die(new Error("should not be called")) }),
)
const exit = yield* Effect.exit(
execute(
tenant,
new QueryEngineExecuteRequest({
startTime: "2026-08-01 00:00:00",
endTime: "2026-11-01 00:00:00",
query: {
kind: "breakdown",
source: "traces",
metric: "count",
groupBy: "service",
filters: { serviceName: asServiceName("checkout") },
},
}),
),
)

assert.include(
(getFailure(exit) as { message?: string } | undefined)?.message ?? "",
"Time range too large",
)
}),
)

it.effect("rejects timeseries requests that exceed the point budget", () =>
Effect.gen(function* () {
const execute = makeQueryEngineExecute(makeTinybirdStub())
Expand Down
10 changes: 8 additions & 2 deletions packages/query-engine/src/limits.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,15 @@

import { relativeRangeSeconds } from "./datetime"

/** Widest window any query kind may span. */
/** Widest window any query kind may span, timeseries aside. */
export const MAX_QUERY_RANGE_SECONDS = 60 * 60 * 24 * 31

/**
* Timeseries read the 365-day hourly rollups, and the service pages offer presets up to 12 months
* (a calendar year, a leap day and a DST hour included). Raw tables keep 30 days, which bounds the rest.
*/
export const MAX_TIMESERIES_RANGE_SECONDS = 60 * 60 * 24 * 367

/**
* List queries (recent traces/logs tiles) scan raw rows rather than a rollup,
* so they carry a much tighter ceiling than charts.
Expand Down Expand Up @@ -78,7 +84,7 @@ export function maxRangeSecondsForKind(kind: QueryRangeKind): number {
case "breakdown":
return MAX_BREAKDOWN_RANGE_SECONDS
case "timeseries":
return MAX_QUERY_RANGE_SECONDS
return MAX_TIMESERIES_RANGE_SECONDS
}
}

Expand Down
25 changes: 18 additions & 7 deletions packages/query-engine/src/runtime/query-engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
MAX_LIST_RANGE_SECONDS,
MAX_QUERY_RANGE_SECONDS,
MAX_TIMESERIES_POINTS,
MAX_TIMESERIES_RANGE_SECONDS,
MAX_UNFILTERED_BREAKDOWN_RANGE_SECONDS,
formatRangeSeconds,
} from "../limits"
Expand Down Expand Up @@ -108,6 +109,7 @@
MAX_LIST_RANGE_SECONDS,
MAX_QUERY_RANGE_SECONDS,
MAX_TIMESERIES_POINTS,
MAX_TIMESERIES_RANGE_SECONDS,
MAX_UNFILTERED_BREAKDOWN_RANGE_SECONDS,
formatRangeSeconds,
maxRangeSecondsForKind,
Expand Down Expand Up @@ -489,10 +491,13 @@
return raw
}

const validateTimeRange = Effect.fn("QueryEngineService.validateTimeRange")(function* (request: {
readonly startTime: string
readonly endTime: string
}): Effect.fn.Return<TimeRangeBounds, QueryEngineValidationError> {
const validateTimeRange = Effect.fn("QueryEngineService.validateTimeRange")(function* (
request: {
readonly startTime: string
readonly endTime: string
},
maxRangeSeconds: number = MAX_QUERY_RANGE_SECONDS,
): Effect.fn.Return<TimeRangeBounds, QueryEngineValidationError> {
const startMs = toEpochMs(request.startTime)
const endMs = toEpochMs(request.endTime)

Expand All @@ -511,10 +516,10 @@
}

const rangeSeconds = (endMs - startMs) / 1000
if (rangeSeconds > MAX_QUERY_RANGE_SECONDS) {
if (rangeSeconds > maxRangeSeconds) {
return yield* new QueryEngineValidationError({
message: "Time range too large",
details: [`Maximum supported range is ${formatRangeSeconds(MAX_QUERY_RANGE_SECONDS)}`],
details: [`Maximum supported range is ${formatRangeSeconds(maxRangeSeconds)}`],
})
}

Expand Down Expand Up @@ -840,10 +845,16 @@
}))
}

/** Timeseries and the services facets read the 365-day rollups; everything else gets the general ceiling. */
const executeRangeCeiling = (query: QuerySpec): number =>
query.kind === "timeseries" || (query.kind === "facets" && query.source === "services")

Check notice on line 850 in packages/query-engine/src/runtime/query-engine.ts

View check run for this annotation

Maple Review Bot / Maple / review

tests: `facets`/`services` half of `executeRangeCeiling` has no test 

The added tests cover a 12-month timeseries and a 3-month breakdown. Nothing pins the second arm of this condition: that the services facet rail is allowed a year while `facets` on traces, logs and errors stays at 31 days, which the services list page now depends on.

@maple-review-bot maple-review-bot Bot Nov 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

executeRangeCeiling widens timeseries over raw metrics and product_events

F1 · Warning · performance

Every timeseries now passes the 367-day ceiling, but only traces reads a 365-day rollup. A 12-month window over metrics_* (90-day retention) or product_events (365-day fact table, queried raw) used to fail fast and now scans up to a year of rows in one query. The limits comment claiming rollup-less sources keep 30 days does not hold for either.

Gate the wider ceiling on the timeseries sources that read a 365-day rollup (e.g. `query.source === "traces"` plus the services facets) and leave `metrics`, `logs` and `product_events` on `MAX_QUERY_RANGE_SECONDS`.
🤖 Prompt to fix with an AI agent 
In `packages/query-engine/src/runtime/query-engine.ts:850-852`: `executeRangeCeiling` widens timeseries over raw `metrics` and `product_events`.

Every timeseries now passes the 367-day ceiling, but only `traces` reads a 365-day rollup. A 12-month window over `metrics_*` (90-day retention) or `product_events` (365-day fact table, queried raw) used to fail fast and now scans up to a year of rows in one query. The limits comment claiming rollup-less sources keep 30 days does not hold for either.

Suggested fix: Gate the wider ceiling on the timeseries sources that read a 365-day rollup (e.g. `query.source === "traces"` plus the services facets) and leave `metrics`, `logs` and `product_events` on `MAX_QUERY_RANGE_SECONDS`.

Verify the problem exists at that location before changing it, and keep the fix to those lines.

? MAX_TIMESERIES_RANGE_SECONDS
: MAX_QUERY_RANGE_SECONDS

Check warning on line 852 in packages/query-engine/src/runtime/query-engine.ts

View check run for this annotation

Maple Review Bot / Maple / review

performance: `executeRangeCeiling` widens timeseries over raw `metrics` and `product_events` 

Every timeseries now passes the 367-day ceiling, but only `traces` reads a 365-day rollup. A 12-month window over `metrics_*` (90-day retention) or `product_events` (365-day fact table, queried raw) used to fail fast and now scans up to a year of rows in one query. The limits comment claiming rollup-less sources keep 30 days does not hold for either.

const validateExecute = Effect.fn("QueryEngineService.validateExecute")(function* (
request: QueryEngineExecuteRequest,
): Effect.fn.Return<TimeRangeBounds, QueryEngineValidationError> {
const range = yield* validateTimeRange(request)
const range = yield* validateTimeRange(request, executeRangeCeiling(request.query))
yield* validateTraceAttributeFilters(request.query)
yield* validateMetricsAttributeFilters(request.query)
yield* validateProductEventsAttributeGrouping(request.query)
Expand Down
Loading