[OPIK-7249] [BE][FE] fix: cheap existence probe for Logs empty state#7378
Conversation
⏱️ pre-commit per-hook timing
⏭️ 38 skipped (no matching files changed)
|
| @GET | ||
| @Path("/exists") | ||
| @Operation(operationId = "spansExist", summary = "Check whether a project has spans", description = "Returns whether the project has at least one span matching the given scope. Cheap existence probe (LIMIT 1) used to drive empty-state decisions without scanning or aggregating the whole project.", responses = { | ||
| @ApiResponse(responseCode = "200", description = "Span existence", content = @Content(schema = @Schema(implementation = ExistenceResponse.class))) | ||
| }) |
There was a problem hiding this comment.
Missing API docs for endpoint
According to the PR description, the new GET /v1/private/spans/exists and GET /v1/private/traces/exists routes weren't regenerated into the OpenAPI/Fern outputs, so the shared API contract and published SDKs/docs don't expose them to clients — should we rerun scripts/generate_openapi.sh and commit the generated files?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/SpansResource.java
around lines 405-409 (the new `@GET @Path("/exists")` operation), note that this
endpoint is implemented but not included in the published OpenAPI/SDK contract because
`scripts/generate_openapi.sh` was not run. Regenerate the OpenAPI spec and Fern/SDK
artifacts using the repo’s script(s), then commit the updated generated outputs (the
files under the OpenAPI/SDK output directories) so the new `/v1/private/spans/exists`
operation appears in the shared API contract. Also verify the corresponding `GET
/v1/private/traces/exists` endpoint is present in the generated contract and adjust
generation config if needed.
ccfb663 to
8a99471
Compare
Backend Tests - Integration Group 11310 tests +29 310 ✅ +29 10m 49s ⏱️ +58s Results for commit 5b71cdd. ± Comparison against base commit 8428b81. This pull request removes 16 and adds 45 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
thiagohora
left a comment
There was a problem hiding this comment.
Could /traces/exists and /spans/exists ship as a minimal probe — project_id/project_name + thread_only only — instead of mirroring the full find contract (filters, search, from_time/to_time)?
The two current consumers (ThreadsTab, TracesSpansTab empty-state) only ever pass project scope + thread_only + source/visibility; there's no backend-internal or SDK caller. So the filter/search/time-range params have zero callers today, which means:
- Correctness: they only exist to feed the
canUseCheapExistsgate + COUNT fallback — the source of the two issues here (experiment_filterssilently dropped on the cheap path; the fallback COUNT has nothread_onlyclause, sothread_only=true+ an aggregate filter counts across all threads). Both are unreachable in prod today but latent for any future/direct caller. - Performance: the cheap win is entirely from
LIMIT 1on project scope. As soon as a filter trips the fallback you hit a full O(project) dedup COUNT — ~15s / ~2.5 GiB on a ~19M-trace project vs ~10ms for the probe.
Trimming to the minimal probe can't break any existing consumer and removes both the risk and the dead surface. Add filters back only when a concrete caller needs them?
AI-WATERMARK: yes
5b71cdd to
3e95fc0
Compare
The project Logs page decided whether to show the "log your first trace" onboarding by firing an auxiliary size=1 list probe (useTracesOrSpansList / useThreadList). Even for a single row the backend builds the full trace/span/cost/feedback aggregation across the entire project before LIMIT 1 -- a multi-GiB, multi-second ClickHouse scan on large projects, fired on every Logs page load (Traces, Spans and Threads tabs) and ignoring the selected time range. Same class as #6148 / #6949. Add a minimal existence endpoint per entity and use it for the probe: - BE: GET /v1/private/traces/exists (+thread_only) and /v1/private/spans/exists, each a project-scoped SELECT 1 ... LIMIT 1. Deliberately minimal -- no filters/search/time-range surface -- so the query is always a primary-key-prunable LIMIT 1 with no full-project COUNT fallback. - FE: useTracesExist / useSpansExist / useTracesOrSpansExist hooks; the Traces, Spans and Threads tabs derive hasProjectData from the existence endpoint (threads via thread_only) instead of a size=1 list query. Turns a whole-project scan into a single-row LIMIT 1 lookup. Not done by forwarding from_time/to_time into the probe: that breaks the empty-state (data older than the window would show onboarding) and revives the id<->event-time mismatch that got #6949 reverted in #7007. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3e95fc0 to
3a95a38
Compare
thiagohora
left a comment
There was a problem hiding this comment.
Approving — the design feedback was fully addressed: /traces/exists and /spans/exists are now minimal probes (project_id/project_name + thread_only), canUseCheapExists and the COUNT fallback are gone, and both EXISTS templates are stripped to project scope. That structurally removes the two correctness bugs from the previous round.
Leaving a few non-blocking follow-ups inline — the rate-limit gap (both probes are currently unthrottled) and the dropped source=sdk scope are the two worth a look before merge.
AI-WATERMARK: yes
- back the thread probe with trace_threads (PK-prunable) instead of traces WHERE thread_id != '', which the sort key + bloom index can't prune - define tracesExist/spansExist rate-limit buckets in config.yml so the @ratelimited probes are actually throttled - restore the source=sdk scope on the probes (with the ENUM_LEGACY unknown fallback) so the Logs empty state matches the sdk-scoped list again - add project_id-form and source-scoping tests for traces and spans Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The round-2 source=sdk restoration left the round-1 'source-agnostic' comment in TracesSpansTab, contradicting the code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Generic regression gating the Logs empty-vs-populated decision the existence probe (#7378) governs: empty project shows the onboarding empty-state, a project with one seeded SDK trace shows the traces table. No-behavior-delta perf fix, so the gate is deliberately generic, not repro-specific. Adds emptyState accessors + waitForEmptyOrTable to the Logs POM. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Details
Fixes the full-project ClickHouse scan reported in #7298 (same class as #6148 / #6949). The project Logs page decides the empty state ("show Log your first trace onboarding, or the table?") with an auxiliary
size=1list probe (useTracesOrSpansList/useThreadList) that has no time bounds — and even for one row the backend builds the full trace/span/cost/feedback aggregation over the entire project beforeLIMIT 1. It runs on every Logs page load (Traces, Spans and Threads tabs), ignores the selected time range, and on large projects is a multi-hundred-MiB to multi-GiB, multi-second scan.Fix — add a minimal existence endpoint per entity and use it for the probe:
GET /v1/private/traces/exists(+ athread_onlyflag →AND thread_id != ''for the Threads empty state) andGET /v1/private/spans/exists, each a project-scopedSELECT 1 … LIMIT 1. Deliberately minimal — nofilters/search/from_time/to_timesurface — so the query is always a primary-key-prunableLIMIT 1with no full-projectCOUNTfallback path (per review: those params have no caller today and would only add a latent full-O(project)fallback). SharedExistenceResponseDTO.useTracesExist/useSpansExist/useTracesOrSpansExisthooks; the Traces, Spans and Threads tabs derivehasProjectDatafrom the existence endpoint (threads viathread_only) instead of asize=1list query.LIMIT 1lookup.from_time/to_timeinto the probe: that breaks the empty state (data older than the window would show onboarding) and revives theid↔event-time mismatch that got [OPIK-6385] [FE] Pass thread time range to ThreadDetailsPanel traces query #6949 reverted in [OPIK-6385] [FE] revert: thread time range in ThreadDetailsPanel traces query #7007.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
mvn compile— clean.mvn test -Dtest='TracesResourceTest$TracesExistence'— existence true, empty-project false, andthread_onlydistinguishing thread projects from trace-only projects.mvn test -Dtest='SpansResourceTest$SpansExistence'— span existence true, and false for a project that only has traces (span existence ≠ trace existence).eslint(changed FE files,--max-warnings=0) andtsc --noEmit— clean, no new type errors.LIMIT 1read (~24–42K rows / ~8 ms) vs. the previous whole-project scan (millions of rows / hundreds of ms – multiple seconds).Documentation
No user-facing docs needed (internal empty-state/perf fix).
Follow-up: regenerate the OpenAPI spec + Fern SDKs for the two new operations (
tracesExist,spansExist) viascripts/generate_openapi.sh(needs thefernCLI) — not committed here to avoid a large partial/desynced generated diff.