Optimize the in-progress traces query and update related indexes by nforro · Pull Request #713 · packit/ai-workflows

nforro · 2026-07-24T12:56:38Z

Also introduce a startup probe so that the pod isn't killed before init_db completes.

qodo-for-packit · 2026-07-24T12:57:26Z

PR Summary by Qodo

Optimize in-progress trace lookup and add Kubernetes startup probe

✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Split the in-progress traces query into candidate selection plus root-span filtering for
 efficiency.
• Add a startup probe so pods aren’t restarted while init_db is still running.
• Simplify liveness/readiness probing by removing explicit initial delays.

Diagram

graph TD
  A["OpenShift Deployment YAML"] --> B["Collector Pod/Container"] --> C["HTTP /health endpoint"] --> D["trace_server (server.py)"] --> E[("SQLite DB: spans + span_issues")]
  B --> F["startupProbe gates startup"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Single SQL with NOT EXISTS / LEFT JOIN anti-join

➕ Keeps logic in one query, potentially fewer round trips
➕ Often optimizes better than NOT IN on many databases
➖ Still risks scanning large portions of spans without selective predicates/indexes
➖ Harder to reason about performance across SQLite query planner variations

2. Add targeted indexes to support the original anti-filter

➕ May make either approach fast (e.g., on (trace_id), (parent_span_id,name), or composite)
➕ Keeps application logic simpler
➖ Schema migration/operational overhead
➖ Index choice depends on real query patterns and dataset sizes

3. Materialize candidate trace_ids via temporary table/CTE and join

➕ Makes the two-phase intent explicit while remaining SQL-driven
➕ Can improve planner behavior and avoid large IN lists
➖ More complex SQL; may be less portable/maintainable
➖ Still requires validating SQLite execution plan and performance

Recommendation: The PR’s two-phase approach (select candidate trace_ids first, then check for root workflow spans only within that reduced set) is a pragmatic win for SQLite and likely reduces worst-case scans compared to the prior NOT IN subquery. If performance is still a concern at scale, consider adding an index for root-span detection (trace_id + parent_span_id + name) or moving the second step to a temp table/CTE to avoid large IN lists.

Files changed (2) +25 / -11

Enhancement (1) +19 / -9

server.pySplit in-progress traces query into candidate+filter phases +19/-9

Split in-progress traces query into candidate+filter phases

• Refactors in-progress trace detection to first fetch candidate trace_ids based on issue-linked spans in the time window, then filters out traces that already have a root Workflow span. Uses existing root trace ids plus a targeted follow-up query limited to the candidate set, and applies the effective limit after filtering.

trace_server/server.py

Other (1) +6 / -2

deployment-otel-collector.ymlAdd startupProbe and adjust health probe timing +6/-2

Add startupProbe and adjust health probe timing

• Introduces a startupProbe against /health to prevent the pod from being killed during initialization (e.g., while init_db runs). Removes explicit initial delays from liveness/readiness probes, relying on the startup probe to gate early failures.

openshift/deployment-otel-collector.yml

qodo-for-packit · 2026-07-24T13:02:10Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 7 rules

1. ~~IN clause exceeds SQLite limit~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

query_recent_traces builds a single trace_id IN (?,...,?) query over all candidate_ids without
chunking, so large candidate sets can trigger SQLite “too many SQL variables” and make
/traces/recent return HTTP 500. This violates the repo’s existing _SQL_VAR_LIMIT pattern used
elsewhere to avoid this exact failure mode.

Code

trace_server/server.py[R625-633]

+    if candidate_ids:
+        ph = ",".join("?" * len(candidate_ids))
+        completed = {
+            r[0]
+            for r in db.execute(
+                f"SELECT DISTINCT trace_id FROM spans "  # noqa: S608
+                f"WHERE trace_id IN ({ph}) AND parent_span_id = '' AND name LIKE '%Workflow'",
+                candidate_ids,
+            ).fetchall()

Relevance

⭐⭐⭐ High
Clear reliability bug: unbounded IN placeholders can hit SQLite variable limit; repo already
documents _SQL_VAR_LIMIT pattern.
PR-#665
PR-#699

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

candidate_ids is unbounded (no LIMIT on the candidate query), then used to build a placeholder
string sized to len(candidate_ids) for an IN clause. The repo already defines _SQL_VAR_LIMIT
and chunks IN queries elsewhere, indicating this code path is expected to respect SQLite variable
limits; failures are surfaced to clients as HTTP 500 on /traces/recent.

trace_server/server.py[78-92]
trace_server/server.py[378-394]
trace_server/server.py[614-635]
trace_server/server.py[784-800]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`query_recent_traces()` constructs `ph = ",".join("?" * len(candidate_ids))` and executes a single `WHERE trace_id IN ({ph}) ...` query. Because `candidate_ids` is unbounded, this can exceed SQLite’s bound-variable limit and fail the request.

## Issue Context
This codebase already defines `_SQL_VAR_LIMIT` and chunks `IN (...)` queries in other hot paths (e.g., `ingest_spans`) to avoid hitting SQLite variable limits.

## Fix Focus Areas
- trace_server/server.py[91-91]
- trace_server/server.py[614-635]
- trace_server/server.py[378-394]

## Suggested fix
- Chunk `candidate_ids` using `_SQL_VAR_LIMIT` when querying for completed traces:
 - Build placeholders per chunk.
 - Accumulate `completed` across chunks.
- Alternatively, remove the need for the large `IN` list by rewriting the filter as `NOT EXISTS` / `LEFT JOIN` against root workflow spans, keeping the filtering in SQL.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Startup probe budget too small~~ ✗ Dismissed 🐞 Bug ☼ Reliability ⭐ New

Description

The startupProbe allows ~300s for /health to become reachable, but the server only starts serving
/health after init_db() completes, and init_db() can do potentially large backfill/retention work.
On large databases this can still cause restart loops where the pod never reaches /health before the
startupProbe budget is exhausted.

Code

openshift/deployment-otel-collector.yml[R71-76]

+        startupProbe:
+          httpGet:
+            path: /health
+            port: 8080
+          failureThreshold: 30
+          periodSeconds: 10

Relevance

⭐⭐⭐ High
Low-risk probe tuning aligns with PR intent to prevent init_db restart loops; likely to increase
startupProbe budget.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The deployment sets a 300s startup budget, but the trace server doesn't serve /health until after
init_db() finishes, and init_db() includes potentially expensive table backfill and retention
cleanup work over existing data.
openshift/deployment-otel-collector.yml[71-76]
trace_server/server.py[113-172]
trace_server/server.py[844-849]
trace_server/server.py[772-774]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The new startupProbe gives the container ~300 seconds to begin serving `/health`. However, `trace_server` runs `init_db()` before binding the HTTP server, so `/health` cannot succeed until all DB initialization work completes. If initialization exceeds ~5 minutes (e.g., large DB, slow storage), Kubernetes will restart the container before it ever becomes healthy.

### Issue Context
`/health` returns 200 only once the HTTP server is running; the server is started after `init_db()`.

### Fix Focus Areas
- openshift/deployment-otel-collector.yml[71-76]

Suggested changes:
- Increase `failureThreshold` (or reduce `periodSeconds`) to cover worst-case init time (e.g., 15–30 minutes if large DBs are expected).
- Consider setting `timeoutSeconds` > 1 to avoid false negatives under load/slow startup.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~NOT EXISTS index mismatch~~ ✓ Resolved 🐞 Bug ➹ Performance ⭐ New

Description

The new correlated NOT EXISTS predicate filters by r.trace_id as well as r.parent_span_id and
r.name, but idx_root_spans is defined only on (parent_span_id, name) and does not support efficient
lookups by trace_id. This can cause the NOT EXISTS check to do more work than expected and may not
deliver the intended performance improvement implied by the comment.

Code

trace_server/server.py[R619-622]

+              AND NOT EXISTS (
+                SELECT 1 FROM spans r
+                WHERE r.trace_id = s.trace_id AND r.parent_span_id = '' AND r.name LIKE '%Workflow'
+              )

Relevance

⭐⭐ Medium
Index tuning is plausible but team has rejected related idx_root_spans redesign suggestions; outcome
uncertain.
PR-#712

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The NOT EXISTS subquery includes a trace_id equality predicate, but the only explicitly-created
"root" index is on (parent_span_id, name), so it cannot directly support a lookup keyed by trace_id
for that subquery.

trace_server/server.py[613-626]
trace_server/server.py[132-136]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The new query uses a correlated `NOT EXISTS` subquery with `r.trace_id = s.trace_id`, but the referenced index `idx_root_spans` is `(parent_span_id, name)` (no `trace_id`). This index cannot efficiently seek by `trace_id`, so the correlated check may still scan many candidate rows and not achieve the intended optimization.

### Issue Context
`idx_root_spans` is created during DB init and is presumably meant to accelerate identifying root workflow spans.

### Fix Focus Areas
- trace_server/server.py[619-622]
- trace_server/server.py[132-136]

Suggested changes:
- Consider replacing/augmenting the index with one that matches the predicate shape, e.g.:
 - `CREATE INDEX ... ON spans(trace_id, parent_span_id, name)` (or at least `(trace_id, parent_span_id)`)
- Re-check `EXPLAIN QUERY PLAN` after the index change to confirm the intended access path.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. ~~Unbounded candidate trace scan~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

query_recent_traces now fetches all grouped candidate traces in the time window with .fetchall()
and builds a map for them, which can significantly increase DB work and Python memory usage before
results are trimmed to limit. This can slow down /traces/recent (or increase memory pressure)
under higher trace volumes.

Code

trace_server/server.py[R614-637]

+    candidate_rows = db.execute(
        f"""SELECT s.trace_id, MIN(s.start_time) as first_start,
                  MAX(json_extract(s.attributes, '$."workflow.name".stringValue')) as workflow_name
            FROM spans s
            JOIN span_issues si ON s.trace_id = si.trace_id AND s.span_id = si.span_id
            WHERE s.start_time >= ?{inprog_filter}
            GROUP BY s.trace_id
-            HAVING s.trace_id NOT IN (
-                SELECT trace_id FROM spans
-                WHERE parent_span_id = '' AND name LIKE '%Workflow'
-            )
-            ORDER BY first_start DESC
-            LIMIT ?""",  # noqa: S608
-        [*inprog_bindings, effective_limit],
+            ORDER BY first_start DESC""",  # noqa: S608
+        inprog_bindings,
    ).fetchall()
+    candidate_ids = [r["trace_id"] for r in candidate_rows if r["trace_id"] not in root_trace_ids]
+    if candidate_ids:
+        ph = ",".join("?" * len(candidate_ids))
+        completed = {
+            r[0]
+            for r in db.execute(
+                f"SELECT DISTINCT trace_id FROM spans "  # noqa: S608
+                f"WHERE trace_id IN ({ph}) AND parent_span_id = '' AND name LIKE '%Workflow'",
+                candidate_ids,
+            ).fetchall()
+        }
+        candidate_ids = [tid for tid in candidate_ids if tid not in completed]
+    candidate_map = {r["trace_id"]: r for r in candidate_rows}
+    inprog_rows = [candidate_map[tid] for tid in candidate_ids[:effective_limit]]

Relevance

⭐⭐ Medium
Performance risk plausible; but no close acceptance/rejection precedent for unbounded fetchall/limit
mismatch.
PR-#665
PR-#699

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The candidate query no longer has a LIMIT and immediately calls .fetchall(), then constructs
candidate_map from the entire result set, meaning work and memory scale with total candidates in
the window rather than the requested limit.

trace_server/server.py[614-637]
trace_server/server.py[784-803]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `candidate_rows = ... GROUP BY ... ORDER BY ... .fetchall()` has no `LIMIT`, so the endpoint materializes all matching candidate traces in memory and then slices to `effective_limit` later.

## Issue Context
`/traces/recent` is a user-facing endpoint and already caps the final result set via `effective_limit`, but the current implementation does most of its work before that cap is applied.

## Fix Focus Areas
- trace_server/server.py[614-637]

## Suggested fix
- Add a DB-side bound to the candidate query (e.g., `LIMIT ?`) and pass a capped overfetch value (commonly `effective_limit * N`, with an upper cap).
- If you must post-filter (exclude completed traces), implement incremental fetching (pagination / overfetch loop) until you have `effective_limit` in-progress rows, instead of fetching all candidates.
- As an alternative, fold the “has root workflow span” exclusion into the candidate SQL via `NOT EXISTS` / `LEFT JOIN` so the database can apply `LIMIT` to the final filtered set.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Previous review results

Review updated until commit f7d5630

Results up to commit 23483ba ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~IN clause exceeds SQLite limit~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

query_recent_traces builds a single trace_id IN (?,...,?) query over all candidate_ids without
chunking, so large candidate sets can trigger SQLite “too many SQL variables” and make
/traces/recent return HTTP 500. This violates the repo’s existing _SQL_VAR_LIMIT pattern used
elsewhere to avoid this exact failure mode.

Code

trace_server/server.py[R625-633]

+    if candidate_ids:
+        ph = ",".join("?" * len(candidate_ids))
+        completed = {
+            r[0]
+            for r in db.execute(
+                f"SELECT DISTINCT trace_id FROM spans "  # noqa: S608
+                f"WHERE trace_id IN ({ph}) AND parent_span_id = '' AND name LIKE '%Workflow'",
+                candidate_ids,
+            ).fetchall()

Relevance

⭐⭐⭐ High
Clear reliability bug: unbounded IN placeholders can hit SQLite variable limit; repo already
documents _SQL_VAR_LIMIT pattern.
PR-#665
PR-#699

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

candidate_ids is unbounded (no LIMIT on the candidate query), then used to build a placeholder
string sized to len(candidate_ids) for an IN clause. The repo already defines _SQL_VAR_LIMIT
and chunks IN queries elsewhere, indicating this code path is expected to respect SQLite variable
limits; failures are surfaced to clients as HTTP 500 on /traces/recent.

trace_server/server.py[78-92]
trace_server/server.py[378-394]
trace_server/server.py[614-635]
trace_server/server.py[784-800]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`query_recent_traces()` constructs `ph = ",".join("?" * len(candidate_ids))` and executes a single `WHERE trace_id IN ({ph}) ...` query. Because `candidate_ids` is unbounded, this can exceed SQLite’s bound-variable limit and fail the request.

## Issue Context
This codebase already defines `_SQL_VAR_LIMIT` and chunks `IN (...)` queries in other hot paths (e.g., `ingest_spans`) to avoid hitting SQLite variable limits.

## Fix Focus Areas
- trace_server/server.py[91-91]
- trace_server/server.py[614-635]
- trace_server/server.py[378-394]

## Suggested fix
- Chunk `candidate_ids` using `_SQL_VAR_LIMIT` when querying for completed traces:
 - Build placeholders per chunk.
 - Accumulate `completed` across chunks.
- Alternatively, remove the need for the large `IN` list by rewriting the filter as `NOT EXISTS` / `LEFT JOIN` against root workflow spans, keeping the filtering in SQL.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Unbounded candidate trace scan~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

query_recent_traces now fetches all grouped candidate traces in the time window with .fetchall()
and builds a map for them, which can significantly increase DB work and Python memory usage before
results are trimmed to limit. This can slow down /traces/recent (or increase memory pressure)
under higher trace volumes.

Code

trace_server/server.py[R614-637]

+    candidate_rows = db.execute(
        f"""SELECT s.trace_id, MIN(s.start_time) as first_start,
                  MAX(json_extract(s.attributes, '$."workflow.name".stringValue')) as workflow_name
            FROM spans s
            JOIN span_issues si ON s.trace_id = si.trace_id AND s.span_id = si.span_id
            WHERE s.start_time >= ?{inprog_filter}
            GROUP BY s.trace_id
-            HAVING s.trace_id NOT IN (
-                SELECT trace_id FROM spans
-                WHERE parent_span_id = '' AND name LIKE '%Workflow'
-            )
-            ORDER BY first_start DESC
-            LIMIT ?""",  # noqa: S608
-        [*inprog_bindings, effective_limit],
+            ORDER BY first_start DESC""",  # noqa: S608
+        inprog_bindings,
    ).fetchall()
+    candidate_ids = [r["trace_id"] for r in candidate_rows if r["trace_id"] not in root_trace_ids]
+    if candidate_ids:
+        ph = ",".join("?" * len(candidate_ids))
+        completed = {
+            r[0]
+            for r in db.execute(
+                f"SELECT DISTINCT trace_id FROM spans "  # noqa: S608
+                f"WHERE trace_id IN ({ph}) AND parent_span_id = '' AND name LIKE '%Workflow'",
+                candidate_ids,
+            ).fetchall()
+        }
+        candidate_ids = [tid for tid in candidate_ids if tid not in completed]
+    candidate_map = {r["trace_id"]: r for r in candidate_rows}
+    inprog_rows = [candidate_map[tid] for tid in candidate_ids[:effective_limit]]

Relevance

⭐⭐ Medium
Performance risk plausible; but no close acceptance/rejection precedent for unbounded fetchall/limit
mismatch.
PR-#665
PR-#699

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The candidate query no longer has a LIMIT and immediately calls .fetchall(), then constructs
candidate_map from the entire result set, meaning work and memory scale with total candidates in
the window rather than the requested limit.

trace_server/server.py[614-637]
trace_server/server.py[784-803]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `candidate_rows = ... GROUP BY ... ORDER BY ... .fetchall()` has no `LIMIT`, so the endpoint materializes all matching candidate traces in memory and then slices to `effective_limit` later.

## Issue Context
`/traces/recent` is a user-facing endpoint and already caps the final result set via `effective_limit`, but the current implementation does most of its work before that cap is applied.

## Fix Focus Areas
- trace_server/server.py[614-637]

## Suggested fix
- Add a DB-side bound to the candidate query (e.g., `LIMIT ?`) and pass a capped overfetch value (commonly `effective_limit * N`, with an upper cap).
- If you must post-filter (exclude completed traces), implement incremental fetching (pagination / overfetch loop) until you have `effective_limit` in-progress rows, instead of fetching all candidates.
- As an alternative, fold the “has root workflow span” exclusion into the candidate SQL via `NOT EXISTS` / `LEFT JOIN` so the database can apply `LIMIT` to the final filtered set.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

nforro · 2026-07-24T13:17:32Z

/agentic_review

qodo-for-packit · 2026-07-24T13:20:53Z

Code review by qodo was updated up to the latest commit c022551

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

nforro · 2026-07-24T13:48:02Z

/agentic_review

qodo-for-packit · 2026-07-24T13:51:21Z

Code review by qodo was updated up to the latest commit f7d5630

majamassarini previously approved these changes Jul 24, 2026

View reviewed changes

qodo-for-packit Bot reviewed Jul 24, 2026

View reviewed changes

Comment thread trace_server/server.py Outdated

Comment thread trace_server/server.py Outdated

nforro dismissed majamassarini’s stale review via c022551 July 24, 2026 13:16

nforro force-pushed the trace-server branch from 23483ba to c022551 Compare July 24, 2026 13:16

nforro changed the title ~~Split the in-progress traces query for better efficiency~~ Use NOT EXISTS for in-progress traces query to leverage idx_root_spans Jul 24, 2026

qodo-for-packit Bot reviewed Jul 24, 2026

View reviewed changes

Comment thread openshift/deployment-otel-collector.yml

Comment thread trace_server/server.py

Optimize the in-progress traces query and update related indexes

f7d5630

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

nforro force-pushed the trace-server branch from c022551 to f7d5630 Compare July 24, 2026 13:47

nforro changed the title ~~Use NOT EXISTS for in-progress traces query to leverage idx_root_spans~~ Optimize the in-progress traces query and update related indexes Jul 24, 2026

majamassarini approved these changes Jul 24, 2026

View reviewed changes

nforro merged commit d4aa19d into packit:main Jul 24, 2026
19 of 20 checks passed

nforro deleted the trace-server branch July 24, 2026 14:02

Navigation Menu

Uh oh!

Optimize the in-progress traces query and update related indexes - #713

Optimize the in-progress traces query and update related indexes#713
nforro merged 1 commit into
packit:mainfrom
nforro:trace-server

nforro commented Jul 24, 2026

Uh oh!

qodo-for-packit Bot commented Jul 24, 2026

Uh oh!

qodo-for-packit Bot commented Jul 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

nforro commented Jul 24, 2026

Uh oh!

Uh oh!

Uh oh!

qodo-for-packit Bot commented Jul 24, 2026

Uh oh!

nforro commented Jul 24, 2026

Uh oh!

qodo-for-packit Bot commented Jul 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

nforro commented Jul 24, 2026

Uh oh!

qodo-for-packit Bot commented Jul 24, 2026

PR Summary by Qodo

Uh oh!

qodo-for-packit Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

Uh oh!

Uh oh!

nforro commented Jul 24, 2026

Uh oh!

Uh oh!

Uh oh!

qodo-for-packit Bot commented Jul 24, 2026

Uh oh!

nforro commented Jul 24, 2026

Uh oh!

qodo-for-packit Bot commented Jul 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qodo-for-packit Bot commented Jul 24, 2026 •

edited

Loading