ingest events; instrument quickwit ingest and search functions

[dinmukhamedm]

Note

Add Quickwit event indexing alongside spans, make ingest generic/instrumented, and update index configs.

• Quickwit indexing pipeline:
  • Add QuickwitIndexedEvent, IndexerQueueMessage, and IndexerQueuePayload in app-server/src/quickwit/mod.rs.
  • Extract/flatten attributes for spans/events in app-server/src/quickwit/consumer.rs; ingest to spans and new events indexes.
  • Add Event::is_exception() in app-server/src/db/events.rs.
• Client:
  • Make ingest and build_doc_batch generic over serde::Serialize and add #[instrument] to ingest, search_spans, build_doc_batch in app-server/src/quickwit/client.rs.
• Producers/Consumers:
  • Publish combined spans+events payload in app-server/src/quickwit/producer.rs.
  • Handle backward-compatible queue payloads and conditional acks/retries in app-server/src/quickwit/consumer.rs.
• Configs:
  • Add frontend/lib/quickwit/indexes/events.yaml and bump frontend/lib/quickwit/indexes/spans.yaml to version "0.8".
  • Clarify immutability when indexes already exist in frontend/lib/quickwit/migrations.ts.

^{Written by Cursor Bugbot for commit cdca2f5. This will update automatically on new commits. Configure here.}

---

Important

Add tracing to Quickwit client, generalize ingest, and introduce event indexing with new models, queue payload, consumer/producer logic, and index configs.

• Backend:
  • Quickwit client: Add #[instrument] to ingest, search_spans, and build_doc_batch; make ingest<T: Serialize> generic.
  • Indexing models: Add QuickwitIndexedEvent, IndexerQueueMessage, and IndexerQueuePayload (spans+events or spans-only); extend QuickwitIndexedSpan unchanged.
  • Events: Add Event::is_exception() for flagging exceptions.
  • Producer/Consumer:
    • Producer publish_spans_for_indexing now publishes both spans and events in a single payload with size checks.
    • Consumer reads unified payload, extracts searchable text from attributes for spans and events, and ingests to spans and events indexes with per-ingest error handling and reconnect.
• Frontend (Quickwit configs):
  • Add indexes/events.yaml (v0.8) defining event fields and search settings.
  • Bump indexes/spans.yaml to v0.8.
  • Update migrations.ts to initialize indexes (skip if existing, note immutability of most config).

^{This description was created by for cdca2f5. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 830167e in 42 seconds. Click for details.

• Reviewed 36 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. app-server/src/quickwit/client.rs:6

• Draft comment:
  New tracing import: Ensure it’s ordered/grouped consistently with other imports.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. app-server/src/quickwit/client.rs:69

• Draft comment:
  Instrumentation added on 'ingest' function. Skipping 'self' and 'spans' is appropriate to avoid logging large data, but consider if 'index_id' should also be explicitly mentioned.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

3. app-server/src/quickwit/client.rs:89

• Draft comment:
  Instrumentation on 'search_spans' logs the 'query_body' parameter. Verify that it doesn’t expose sensitive or overly large data.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

4. app-server/src/quickwit/client.rs:121

• Draft comment:
  Instrumentation on 'build_doc_batch' skips 'spans'. Ensure this is intentional to prevent logging voluminous data.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_sOHOuf0xuqfRc7V5

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed a56e9fd in 2 minutes and 31 seconds. Click for details.

• Reviewed 399 lines of code in 9 files
• Skipped 0 files when reviewing.
• Skipped posting 8 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. app-server/src/quickwit/client.rs:68

• Draft comment:
  Good instrumentation update. The generic parameter T: serde::Serialize adds flexibility. Ensure that any type passed truly serializes to the expected document format for Quickwit ingestion.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50% None

2. app-server/src/quickwit/consumer.rs:176

• Draft comment:
  The changes correctly support both spans and events. As a potential enhancement, consider ingesting spans and events concurrently (using futures::join! or try_join!) to reduce overall latency if performance becomes a concern.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

3. app-server/src/quickwit/mod.rs:45

• Draft comment:
  The introduction of QuickwitIndexedEvent and the untagged enum IndexerQueuePayload cleanly supports both spans and events. Ensure that the untagged enum remains unambiguous during serialization/deserialization as the data structure evolves.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the author to ensure that a specific behavior remains consistent as the data structure evolves. It is not making a specific suggestion or pointing out a specific issue with the current code. It is more of a general cautionary note, which violates the rule against asking the author to ensure behavior is intended or tested.

4. app-server/src/quickwit/producer.rs:19

• Draft comment:
  Potential Issue: The function returns early if spans is empty, even if events are not. Consider adjusting the condition to publish the payload when either spans or events are present.
• Reason this comment was not posted:
  Comment was on unchanged code.

5. app-server/src/traces/consumer.rs:382

• Draft comment:
  When publishing to the Quickwit indexing queue, both spans and events are converted and passed to the producer. Verify that downstream consumers correctly handle cases where one of the arrays might be empty, especially in light of the producer’s early return condition.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

6. frontend/lib/quickwit/indexes/events.yaml:1

• Draft comment:
  The events index configuration looks well defined with version bumped to 0.8. Verify that field mappings (especially for booleans and text) match expected query patterns in production.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50% None

7. frontend/lib/quickwit/indexes/spans.yaml:1

• Draft comment:
  The spans index configuration is updated to version 0.8. Ensure that the field mappings, tokenizers, and retention policy remain consistent with production requirements.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50% None

8. frontend/lib/quickwit/migrations.ts:121

• Draft comment:
  The migration script correctly handles index creation. Note that once an index is created, most configuration changes are ignored, so ensure initial settings are final.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50% None

Workflow ID: wflow_iuL5DPGJEIaBMNO2

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Consider using eq_ignore_ascii_case (e.g. self.name.eq_ignore_ascii_case("exception")) instead of converting to lowercase and trimming. This avoids extra allocation.

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed f085ae0 in 34 seconds. Click for details.

• Reviewed 26 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. app-server/src/db/events.rs:36

• Draft comment:
  Good improvement: using eq_ignore_ascii_case avoids allocating a new string. Ensure that event names will always be ASCII since this method only handles ASCII case comparisons.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. app-server/src/quickwit/producer.rs:19

• Draft comment:
  The condition now checks that both spans and events are empty before returning early, which aligns with the combined payload approach. Looks good.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_Xol6Tknn6uoXLTnZ

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed cdca2f5 in 1 minute and 49 seconds. Click for details.

• Reviewed 30 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/lib/quickwit/indexes/events.yaml:7

• Draft comment:
  Removed 'stored: false' from 'is_exception'. Confirm this omission is intentional for search results.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. frontend/lib/quickwit/indexes/events.yaml:11

• Draft comment:
  Removed 'stored: false' from 'project_id'. Ensure this aligns with desired retrieval behavior.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 40% <= threshold 50% The comment is asking the author to ensure that the removal of stored: false aligns with the desired behavior. This is asking for confirmation of intention, which is against the rules. However, it does point out a specific change that might affect behavior, which could be useful. I need to evaluate if this is more of a suggestion or a request for confirmation.

3. frontend/lib/quickwit/indexes/events.yaml:19

• Draft comment:
  Changed 'span_id' fast flag from false to true. Verify this change meets indexing performance and search expectations.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify the change meets certain expectations, which violates the rule against asking the author to ensure behavior is intended or tested. It doesn't provide a specific suggestion or point out a specific issue.

4. frontend/lib/quickwit/indexes/events.yaml:20

• Draft comment:
  New 'id' field added. Consider if an explicit 'stored' setting is needed for consistent retrieval.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment is asking the author to "consider" whether a setting is needed, which is speculative and not actionable. It doesn't say there's definitely a problem - it's asking the author to think about it. The comment doesn't provide evidence that the current configuration is wrong. Looking at the existing fields in the file, many don't have explicit stored settings, suggesting there's a default behavior. The comment is essentially asking "did you think about this?" rather than pointing out a concrete issue. This violates the rule about not asking the author to confirm their intention or double-check things. Maybe there's a specific reason why the id field would need special handling compared to other fields? Perhaps in this particular system, IDs should always be stored or never stored, and the comment is trying to prevent a common mistake? Even if there were a best practice about ID fields, the comment doesn't state what it is or why it matters. It just asks the author to "consider" it, which is not actionable. If there was a specific requirement, the comment should state it definitively rather than asking the author to think about it. This comment should be deleted. It's asking the author to "consider" something without providing concrete guidance or identifying a definite issue. It's speculative and asks the author to double-check their work, which violates the rules.

Workflow ID: wflow_iXPc7iiMy9rXiEZe

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[Rainhunter13]

stored false or need it when querying?