feat: logs search v2 - #4615
Conversation
Project closed source windows with durable watermarks and leases. Keep v2 reads and backfill disabled by default until sufficient history exists.
Fetch bounded extra rows and remove duplicate projection identities in the application. Keep exact keyset pagination while background merges collapse physical copies.
Create the scheduled-projector schema directly in migration 038 and remove the intermediate migration.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (23)
🧰 Additional context used📓 Path-based instructions (1)internal-packages/clickhouse/schema/[0-9][0-9][0-9]_*.sql📄 CodeRabbit inference engine (internal-packages/clickhouse/CLAUDE.md)
Files:
🔇 Additional comments (1)
WalkthroughAdded a ClickHouse V2 search table and projection pipeline with persistent state, leases, backfills, telemetry, scheduled processing, and admin controls. Updated log search normalization, minimum-length validation, bounded pagination, cursor handling, and period expansion. Centralized logs access checks across navigation and routes. Added ClickHouse command support, runtime configuration, database state storage, and integration tests. Merge Risk: 🟠 High · up to This PR adds asynchronous log indexing and a new search path, but unresolved configuration and data-correctness issues could prevent the webapp from starting in existing deployments or create incorrect projected timestamps and search behavior. These issues should be fixed before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Observability mapAs of 20/100 over 426 measured of 442 entry points (base 20, no change) What this PR changed
FIX FIRST
AUDIT 3 of 50 sensitive mutations record an actor. 47 without one. What the score is made ofThe score and findings here are report-only and never gate the merge. Separately, a required test suite keeps this tool's symbol and route lists in sync with the code they name, and can fail a pull request that renames or removes a symbol they reference, or that adds the first route with a segment they anticipate. Each failure names the list to edit. The rules and their reasons: internal-packages/observability-map/README.md. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| projection_fingerprint UInt128 DEFAULT reinterpretAsUInt128( | ||
| sipHash128(trace_id, span_id, run_id, start_time) | ||
| ), | ||
|
|
||
| INDEX idx_run_id run_id TYPE bloom_filter(0.001) GRANULARITY 1, | ||
| INDEX idx_search_text search_text | ||
| TYPE text(tokenizer = 'ngrams', preprocessor = lowerUTF8(search_text)) | ||
| ) | ||
| ENGINE = ReplacingMergeTree | ||
| PARTITION BY toDate(triggered_timestamp) | ||
| ORDER BY ( | ||
| organization_id, | ||
| environment_id, | ||
| triggered_timestamp, | ||
| trace_id, | ||
| span_id, | ||
| projection_fingerprint | ||
| ) |
There was a problem hiding this comment.
🔍 ReplacingMergeTree dedup key can collapse two genuinely distinct events
The v2 sorting/dedup key is (organization_id, environment_id, triggered_timestamp, trace_id, span_id, projection_fingerprint) where projection_fingerprint = sipHash128(trace_id, span_id, run_id, start_time). Span events reuse their parent span's span_id (apps/webapp/app/v3/eventRepository/clickhouseEventRepository.server.ts:590, :626), so two records on the same span that share an identical start_time (e.g. two span events recorded at the same nanosecond, or a zero-duration span event coinciding exactly with a zero-duration span) hash identically and would be silently merged into one row by ReplacingMergeTree — permanent loss of a log line, not just a retry-copy collapse. v1's plain MergeTree kept both. Log records are safe because otlpTransform.server.ts:124 mints a fresh span_id per log. If exact-timestamp collisions are considered possible, adding kind/message (or an insert sequence) to the fingerprint would make the identity total.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Renumber search_v2 table migration to 039 and drop the inserted_at index DDL, which now ships as standalone migration 038.
1 participant
Summary
Adds a new global logs search path with a smaller search representation, asynchronous indexing, and bounded controls for building history. Existing reads remain on v1 by default.
task_events_search_v2attributes_textand twongrambf_v1indexes with one capped, normalizedsearch_textcolumn and one ClickHousetextindex using thengramstokenizer.error_messageand only the fields needed by the logs list.ReplacingMergeTreeremoves retry copies during merges.minmax(inserted_at)source index to prune projector windows in newly written source parts.Sync strategy
inserted_atwindows behind a configurable safety delay, using ClickHouse time for the cutoff.Environment variables and enabling v2
LOGS_SEARCH_TABLE_VERSIONselects v1 or v2 reads and defaults tov1.LOGS_SEARCH_PROJECTOR_ENABLEDdefaults to disabled.LOGS_SEARCH_PROJECTOR_CLICKHOUSE_URLcan route projector writes separately, while additional settings bound work, time, rows, memory, and threads.Webapp changes
Supporting changes
command()support, per-client request timeouts, and command summaries.Prereqs