-
Notifications
You must be signed in to change notification settings - Fork 153
unification of events + clusters #1080
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.
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
Conversation
|
Generated with ❤️ by ellipsis.dev |
| const GetLastEventSchema = z.object({ | ||
| projectId: z.string(), | ||
| name: z.string(), | ||
| eventSource: z.enum(["CODE", "SEMANTIC"]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Case mismatch in eventSource query parameter
The GetLastEventSchema defines eventSource as z.enum(["CODE", "SEMANTIC"]) (uppercase), but the backend Rust code stores the source field as lowercase "code" or "semantic" in ClickHouse (via EventSource::to_string()). The frontend pages pass 'CODE' and 'SEMANTIC' to getLastEvent, which queries ClickHouse with these uppercase values. Since ClickHouse string comparisons are case-sensitive, the query source = {source: String} will never match any records, causing getLastEvent to always return no results.
Additional Locations (2)
* feat: add template prefill * feat: update type, always pass prompt, structured output
frontend/app/api/projects/[projectId]/events/[name]/cluster-config/route.ts
Outdated
Show resolved
Hide resolved
| .map_err(|e| { | ||
| log::warn!("Failed to call service ({}): {:?}", service_url, e); | ||
| backoff::Error::transient(anyhow::Error::from(e)) | ||
| })?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing HTTP request timeout in service utility function
The new call_service_with_retry function does not set a timeout on individual HTTP requests. The old clustering code explicitly included .timeout(Duration::from_secs(60)) to prevent requests from hanging indefinitely. The backoff's max_elapsed_time only controls retry timing, not individual request duration. If an external service becomes unresponsive, worker threads calling this function (clustering and semantic events handlers) will block forever, causing the worker pool to gradually degrade.
Additional Locations (1)
| // 8 bytes for timestamp, | ||
| return 16 + 16 + 16 + 8 + 8 + self.name.len() + estimate_json_size(&self.attributes); | ||
| return 16 + 16 + 16 + 8 + self.name.len() + estimate_json_size(&self.attributes); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Event size estimate missing new source field
The estimate_size_bytes function calculates event size for usage tracking but doesn't include the newly added source field. The old created_at field (8 bytes) was removed from the calculation, but the new source field (which stores "CODE" or "SEMANTIC") wasn't added. This causes size estimates to be slightly smaller than actual, affecting workspace usage limit calculations.
| "project_id": message.project_id.to_string(), | ||
| "trace_id": message.trace_id.to_string(), | ||
| "event_definition": serde_json::to_value(event_definition).unwrap(), | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing trigger_span_id in semantic event service request
The request body sent to the semantic event service is missing trigger_span_id. The SemanticEventMessage struct contains this field and it's passed through the queue correctly, but when constructing the HTTP request body, only project_id, trace_id, and event_definition are included. The old code in summary.rs included "trigger_span_id": message.trigger_span_id.to_string() in the request. The external service likely needs this field to identify which span triggered the semantic event analysis.
Note
Introduce semantic events pipeline with clustering and source-aware events, unify workers, and add frontend for managing code/semantic events and clusters.
SemanticEventHandler, queues (semantic_event_*), trigger spans cache (semantic_event_trigger_spans), and DB modules for semantic event definitions/triggers; write events to ClickHouse withsource.event_cluster_configs,event_clustering_*), mustache templating, locking, and service call; new CH viewevents_v0with clusters.workerframework and migrate spans, browser events, evaluators, payloads, notifications, Quickwit indexer, semantic events, and clustering to it.record_span_events; addEventSourceand propagate to CH/queries; remove old trace summary flow.SemanticEventsand updateClusteringflags.SemanticandCodetabs/pages; new CRUD/API for semantic event definitions; manage trigger spans and prompts; display events with source filter and attributes first.event_cluster_configs,event_clusters,semantic_event_definitions,semantic_event_trigger_spans; enablevectorextension.events.sourceandevents_v0view; update queries to filter bysource.Written by Cursor Bugbot for commit 712f06e. This will update automatically on new commits. Configure here.