Skip to content

A couple of deployment changes and trace server improvements - #665

Merged
nforro merged 3 commits into
packit:mainfrom
nforro:deployment
Jul 9, 2026
Merged

A couple of deployment changes and trace server improvements#665
nforro merged 3 commits into
packit:mainfrom
nforro:deployment

Conversation

@nforro

@nforro nforro commented Jul 8, 2026

Copy link
Copy Markdown
Member
  • Allow DuckDuckGoSearchTool on the firewall
  • Increase resources for trace server
  • Add multi-issue observability for MR consolidation agent

nforro added 2 commits July 8, 2026 16:55
Signed-off-by: Nikola Forró <nforro@redhat.com>
Assisted-by: Claude Opus 4.6 via Claude Code
Signed-off-by: Nikola Forró <nforro@redhat.com>
Assisted-by: Claude Opus 4.6 via Claude Code

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates resource limits for the OpenTelemetry collector, allows egress traffic to Brave Search, and refactors the trace server to support multiple Jira issues per span/trace using a new span_issues database table. It also introduces a new /traces/recent endpoint to query recent root workflow spans. The review feedback highlights several critical areas for improvement: resolving an N+1 query bottleneck in the new /traces/recent endpoint, ensuring all spans in a trace receive the full merged set of Jira issues during propagation, preventing a potential AttributeError when parsing arrayValue in OTLP payloads, and restoring a fallback transaction name in the consolidation agent when no source issues are present.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread trace_server/server.py Outdated
Comment thread trace_server/server.py
Comment thread trace_server/server.py Outdated
Comment thread ymir/agents/mr_consolidation_agent.py
@qodo-for-packit

Copy link
Copy Markdown

PR Summary by Qodo

OpenShift egress/resource tweaks + trace server multi-issue observability

✨ Enhancement ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Allow egress to search.brave.com for DuckDuckGoSearchTool usage.
• Increase OTel Collector CPU/memory requests and limits for higher trace throughput.
• Enhance trace server to support multi-issue traces and add /traces/recent querying.
Diagram

graph TD
  Deploy[/"deployment-otel-collector.yml"/] -->|"CPU/mem"| Collector["OTel Collector"] -->|"OTLP HTTP"| Trace["Trace Server"] -->|"store/query"| DB[("SQLite DB")]
  User["Ops / UI"] -->|"GET /traces*"| Trace
  Agent["MR Consolidation Agent"] -->|"OTLP spans"| Collector
  Agent -->|"HTTPS"| Egress[/"tenant-egress.yml"/] -->|"Allow"| Brave{{"search.brave.com"}}
  subgraph Legend
    direction LR
    _svc["Service"] ~~~ _cfg[/"Config"/] ~~~ _db[("Database")] ~~~ _ext{{"External"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use a trace-level issue table (trace_issues) instead of span_issues
  • ➕ Much smaller data footprint (deduped per trace instead of per span).
  • ➕ Simpler lookups for “issue -> traces” queries.
  • ➖ Harder to support span-level joins if future queries need (trace_id, span_id) precision.
  • ➖ Would require a different migration path and query rewrites.
2. Store jira.issue as JSON/array in spans and index with SQLite JSON1
  • ➕ Avoids an additional table and joins.
  • ➕ Keeps all span metadata in one place.
  • ➖ JSON indexing/querying in SQLite is less portable and can be slower/fragile.
  • ➖ Harder to enforce uniqueness and avoid duplicates consistently.

Recommendation: The PR’s normalized approach (span_issues + propagation) is a solid, low-friction way to make full traces discoverable from any associated issue and to support filtered queries. If DB growth becomes a concern (many spans × many issues), consider evolving to a deduped trace_issues table, but the current design is a reasonable starting point.

Files changed (4) +179 / -30

Enhancement (2) +171 / -26
server.pyAdd multi-issue indexing and a /traces/recent endpoint +167/-24

Add multi-issue indexing and a /traces/recent endpoint

• Extends span ingestion to support jira.issue as comma-delimited strings or arrays, propagates issues across all spans in a trace, and stores issue mappings in a new span_issues table. Updates issue and span querying to use the junction table and adds GET /traces/recent for efficient retrieval of recent root workflow traces with basic stats.

trace_server/server.py

mr_consolidation_agent.pyPropagate multi-issue Jira context into tracing for MR consolidation +4/-2

Propagate multi-issue Jira context into tracing for MR consolidation

• Sets the current Jira context to a comma-separated list of all collected source issues, and uses that multi-issue key when starting consolidation transactions to improve trace discoverability.

ymir/agents/mr_consolidation_agent.py

Other (2) +8 / -4
deployment-otel-collector.ymlIncrease OTel Collector CPU/memory requests and limits +4/-4

Increase OTel Collector CPU/memory requests and limits

• Raises CPU and memory limits/requests for the OTel Collector container to handle higher telemetry throughput and reduce resource pressure.

openshift/deployment-otel-collector.yml

tenant-egress.ymlAllow Brave Search egress for DuckDuckGoSearchTool +4/-0

Allow Brave Search egress for DuckDuckGoSearchTool

• Adds an EgressFirewall allow rule for search.brave.com, enabling the DuckDuckGoSearchTool’s Brave-backed search to work in-cluster.

openshift/tenant-egress.yml

Comment thread trace_server/server.py
@qodo-for-packit

qodo-for-packit Bot commented Jul 8, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 7 rules

Grey Divider


Action required

1. Stale issue mappings persist ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New
Description
ingest_spans() updates spans with INSERT OR REPLACE but only INSERT OR IGNOREs into
span_issues, so if a span is re-ingested with a corrected/reduced Jira issue set, old
span_issues rows are never removed. Because query_issues()/query_spans() now rely on
span_issues, this can make traces discoverable under issues that are no longer actually
associated.
Code

trace_server/server.py[R332-337]

+    issue_rows = [(s.trace_id, s.span_id, issue) for s in spans for issue in s.jira_issues]
+    if issue_rows:
+        db.executemany(
+            "INSERT OR IGNORE INTO span_issues (trace_id, span_id, jira_issue) VALUES (?, ?, ?)",
+            issue_rows,
+        )
Relevance

⭐⭐ Medium

No historical evidence on deleting stale junction rows on re-ingest; no span_issues lifecycle
guidance found.

PR-#516
PR-#533

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new code writes span_issues incrementally (OR IGNORE) while the canonical spans row can be
replaced, and the read paths now depend on span_issues, so stale rows affect user-visible
discovery.

trace_server/server.py[319-339]
trace_server/server.py[348-416]

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

## Issue description
`span_issues` is append-only: on re-ingest, new issue rows are inserted but removed/incorrect issues are never deleted. This makes `/traces` and `/traces/<issue>` return results for stale issue associations.

## Issue Context
- `spans` rows are updated via `INSERT OR REPLACE`.
- `span_issues` rows are updated via `INSERT OR IGNORE`, with no delete step.

## Fix Focus Areas
- trace_server/server.py[319-339]
- trace_server/server.py[348-416]

## Implementation guidance
- Before inserting new rows for a batch of spans, delete existing mappings for those exact `(trace_id, span_id)` pairs, then insert the freshly computed mappings.
 - Example approach:
   - Build `span_keys = {(s.trace_id, s.span_id) for s in spans}`
   - `executemany("DELETE FROM span_issues WHERE trace_id=? AND span_id=?", list(span_keys))`
   - Then `executemany(INSERT ...)` for the new `(trace_id, span_id, jira_issue)` tuples.
- Keep the current PK `(trace_id, span_id, jira_issue)`; deletion ensures the table matches the latest ingested state.

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



Remediation recommended

2. Invalid issue keys indexed ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New
Description
_parse_jira_issues() stringifies every element when jira.issue is array-valued, so malformed
values (e.g., None or dict-like elements) can become unintended issue keys and be inserted into
span_issues. This can pollute /traces and make traces discoverable under non-issue strings.
Code

trace_server/server.py[R244-253]

+def _parse_jira_issues(raw) -> list[str]:
+    """Parse jira.issue attribute into a list of issue keys.
+
+    Handles string (possibly comma-delimited) and array values.
+    """
+    if raw is None:
+        return []
+    if isinstance(raw, list):
+        return [s for v in raw if (s := str(v).strip())]
+    return [s for part in str(raw).split(",") if (s := part.strip())]
Relevance

⭐⭐⭐ High

Team accepted hardening issue fields against dict-shaped/unexpected values (PR #584); matches issue
normalization.

PR-#584

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
_get_val() can return lists for arrayValue, and _parse_jira_issues() currently stringifies
list elements; those strings are then inserted into span_issues without validation.

trace_server/server.py[144-152]
trace_server/server.py[244-253]
trace_server/server.py[319-337]
PR-#584

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

## Issue description
For array-valued `jira.issue`, `_parse_jira_issues()` currently accepts and stringifies any element, which can generate bogus issue keys (e.g. `"None"`, `"{'key': ...}"`) and store them in `span_issues`.

## Issue Context
- `_get_val()` now supports OTLP `arrayValue` and returns a Python list.
- `_parse_jira_issues()` converts list entries via `str(v).strip()` without validation.

## Fix Focus Areas
- trace_server/server.py[144-152]
- trace_server/server.py[244-253]
- trace_server/server.py[319-337]

## Implementation guidance
- In `_parse_jira_issues(raw)`:
 - For list input, ignore `None` values.
 - If an element is a dict and contains a Jira key-like field (e.g. `{"key": "RHEL-123"}`), extract it; otherwise drop it.
 - Only accept strings that match your Jira key pattern (e.g. `^[A-Z][A-Z0-9]+-\d+$`), and drop anything else.
- Ensure the returned list contains only valid issue-key strings before inserting into `span_issues`.

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


3. Unstable issue string ✓ Resolved 🐞 Bug ◔ Observability
Description
state.jira_issues_collected = list(set(all_jira)) produces nondeterministic ordering, and the
comma-joined value is used as current_jira_issue. This can fragment log buffering and transaction
naming/correlation across runs for the same set of issues.
Code

ymir/agents/mr_consolidation_agent.py[R468-472]

                all_jira.extend(_extract_jira_issues_from_description(mr.get("description", "")))
            state.jira_issues_collected = list(set(all_jira))
            state.jira_issue = state.jira_issues_collected[0] if state.jira_issues_collected else package
+            if state.jira_issues_collected:
+                current_jira_issue.set(",".join(state.jira_issues_collected))
Relevance

⭐⭐⭐ High

Team previously accepted order-preserving dedup of Jira issue lists (determinism concerns).

PR-#611

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The MR consolidation code builds the multi-issue string from a set, and the logging handler uses
the issue string as a dictionary key for buffering, so nondeterministic ordering causes multiple
distinct buffers/keys for the same issue set.

ymir/agents/mr_consolidation_agent.py[466-472]
ymir/common/logging_setup.py[37-67]

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

### Issue description
`list(set(all_jira))` yields an arbitrary order, so the resulting `current_jira_issue` string can vary run-to-run for identical inputs.

### Issue Context
`BufferedTaskHandler` buffers logs keyed by the exact `current_jira_issue` string, so different orderings create separate buffers for the same logical multi-issue set.

### Fix Focus Areas
- ymir/agents/mr_consolidation_agent.py[466-472]
- ymir/common/logging_setup.py[37-67]

### Suggested fix
- Replace `list(set(all_jira))` with `sorted(set(all_jira))` (or other stable ordering).
- Use the sorted list consistently for:
 - `state.jira_issues_collected`
 - `state.jira_issue = ...[0] ...`
 - `current_jira_issue.set(",".join(...))`

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


4. Recent traces N+1 queries ✓ Resolved 🐞 Bug ➹ Performance
Description
query_recent_traces() performs two additional SQL queries per returned trace (issues + counts),
which can become costly at higher limits and request rates. This increases latency and DB load for
/traces/recent.
Code

trace_server/server.py[R460-471]

+    results = []
+    for r in rows:
+        trace_id = r["trace_id"]
+        issues = db.execute(
+            "SELECT DISTINCT jira_issue FROM span_issues WHERE trace_id = ?",
+            (trace_id,),
+        ).fetchall()
+        counts = db.execute(
+            "SELECT COUNT(*) as cnt, SUM(CASE WHEN status_code = 2 THEN 1 ELSE 0 END) as errors "
+            "FROM spans WHERE trace_id = ?",
+            (trace_id,),
+        ).fetchone()
Relevance

⭐⭐ Medium

No prior trace_server reviews about avoiding N+1 query loops; performance standards unclear.

PR-#516
PR-#556

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The implementation explicitly executes two separate db.execute(...) calls inside a for r in rows
loop, creating two extra round trips per trace.

trace_server/server.py[437-485]

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()` fetches a list of recent traces, then loops and runs two extra queries per trace. This is an N+1 query pattern.

### Issue Context
`limit` can be up to `MAX_LAST_TRACES`, so the per-trace queries can dominate response time.

### Fix Focus Areas
- trace_server/server.py[437-485]

### Suggested fix
- After fetching the initial `rows`, collect `trace_ids` and fetch:
 - issues for all trace_ids in one query (e.g., `GROUP_CONCAT(DISTINCT jira_issue)` grouped by `trace_id`), and
 - counts/errors for all trace_ids in one query (`GROUP BY trace_id`).
- Merge these precomputed maps into `results` instead of querying inside the loop.

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


View more (1)
5. Null jira_key drops traces 🐞 Bug ◔ Observability
Description
In MR consolidation queue mode, jira_key becomes None when source_issues is empty and is used
for both current_jira_issue and start_transaction, which prevents jira.issue from being
attached to spans. Those runs become unsearchable by issue in the trace server and lose per-issue
log correlation.
Code

ymir/agents/mr_consolidation_agent.py[R1277-1283]

+            jira_key = ",".join(job.source_issues) if job.source_issues else None
            current_jira_issue.set(jira_key)
            try:
                with span_processor.start_transaction(
-                    f"consolidation-{job.package}",
+                    jira_key,
                    workflow="mr_consolidation",
                ):
Relevance

⭐⭐ Medium

Closest precedent: rejected guarding None transaction fields in observability (PR #580); unclear for
None jira_key.

PR-#580
PR-#630

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The queue-mode path explicitly sets jira_key = None when there are no source issues, and the span
processor only sets jira.issue when the current context value is truthy, so spans will lack the
attribute in that case.

ymir/agents/mr_consolidation_agent.py[1269-1283]
ymir/agents/observability.py[30-53]

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

### Issue description
`jira_key` is set to `None` when `job.source_issues` is empty, which causes `AgentSpanProcessor.on_start()` to skip setting the `jira.issue` span attribute. This removes trace-server indexing/correlation for those jobs.

### Issue Context
`AgentSpanProcessor.on_start()` only sets `jira.issue` when the context value is truthy.

### Fix Focus Areas
- ymir/agents/mr_consolidation_agent.py[1275-1283]
- ymir/agents/observability.py[30-53]

### Suggested fix
- Keep the multi-issue join when `source_issues` is non-empty, but add a deterministic fallback when it’s empty (e.g., `f"{job.package}/{job.target_branch}"`), and pass that fallback into both `current_jira_issue.set(...)` and `start_transaction(...)`.
- Ensure the fallback is a non-empty string so `on_start()` reliably attaches `jira.issue`.

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



Informational

6. jira_issue mismatches query ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New
Description
Multi-issue discovery is done via span_issues, but query_spans() still returns the single
spans.jira_issue value, which is stored as only the first issue (jira_issues[0]). When querying
/traces/<issue> for a non-first issue, returned spans can show a different jira_issue than the
one requested.
Code

trace_server/server.py[R200-206]

            self.start_time,
            self.end_time,
            self.status_code,
-            self.jira_issue,
+            self.jira_issues[0] if self.jira_issues else None,
            self.agent_type,
            self.attributes,
        )
Relevance

⭐⭐ Medium

No prior reviews on multi-issue trace querying; trace_server is new (PRs #516/#533).

PR-#516
PR-#533

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new ingestion path stores only the first issue into spans.jira_issue, while the query response
still returns that field even though trace selection is multi-issue via span_issues.

trace_server/server.py[194-206]
trace_server/server.py[354-438]

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_spans()` discovers traces via `span_issues` (multi-issue), but the response payload still exposes only `spans.jira_issue`, which is persisted as `jira_issues[0]`. This makes the JSON response internally inconsistent for multi-issue traces.

## Issue Context
- `SpanRow.as_tuple()` persists only the first issue into `spans.jira_issue`.
- `query_spans()` returns `r["jira_issue"]` directly.

## Fix Focus Areas
- trace_server/server.py[194-206]
- trace_server/server.py[354-438]

## Implementation guidance
Pick one:
1) Extend API to return `jira_issues: [...]` (preferred) by looking up issues per trace (or per span) from `span_issues` and including it in each returned span (or at least once per trace).
2) Store a canonical comma-joined issue string in `spans.jira_issue` (and update any UI/consumers accordingly), so the single field reflects multi-issue reality.

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


7. Backfill skips issue splitting ✓ Resolved 🐞 Bug ≡ Correctness
Description
init_db() backfills span_issues from spans.jira_issue verbatim, so any historical rows with
comma-delimited issue strings remain unsplit in span_issues. After this PR, query_issues() and
query_spans() rely on span_issues, so those historical traces won’t be discoverable by
individual issue keys.
Code

trace_server/server.py[R122-134]

+    db.execute("""
+        CREATE TABLE IF NOT EXISTS span_issues (
+            trace_id TEXT NOT NULL,
+            span_id TEXT NOT NULL,
+            jira_issue TEXT NOT NULL,
+            PRIMARY KEY (trace_id, span_id, jira_issue)
+        )
+    """)
+    db.execute("CREATE INDEX IF NOT EXISTS idx_span_issues_issue ON span_issues(jira_issue)")
+    db.execute(
+        "INSERT OR IGNORE INTO span_issues (trace_id, span_id, jira_issue) "
+        "SELECT trace_id, span_id, jira_issue FROM spans WHERE jira_issue IS NOT NULL"
+    )
Relevance

⭐⭐ Medium

No historical review evidence about trace_server backfill splitting comma-delimited jira issues.

PR-#516
PR-#533

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The backfill inserts spans.jira_issue directly into span_issues, but the server now supports and
expects splitting comma-delimited values via _parse_jira_issues and uses span_issues for issue
listing.

trace_server/server.py[100-135]
trace_server/server.py[239-248]
trace_server/server.py[344-347]

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 multi-issue parsing logic is applied during ingest, but the DB backfill copies `spans.jira_issue` into `span_issues` without splitting comma-delimited values, leaving historical multi-issue rows unqueryable per-issue.

### Issue Context
After this PR, `query_issues()` reads distinct issues from `span_issues`, not `spans`.

### Fix Focus Areas
- trace_server/server.py[100-135]
- trace_server/server.py[239-248]
- trace_server/server.py[344-347]

### Suggested fix
- During `init_db()` (or as a one-time migration), rebuild/normalize `span_issues` from existing `spans` rows by splitting `spans.jira_issue` on commas (reuse `_parse_jira_issues`).
- Insert one row per (trace_id, span_id, individual_issue) instead of copying the raw combined string.

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


Grey Divider

Previous review results

Review updated until commit 3f83f88

Results up to commit 0eaf863


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


Remediation recommended
1. Unstable issue string ✓ Resolved 🐞 Bug ◔ Observability
Description
state.jira_issues_collected = list(set(all_jira)) produces nondeterministic ordering, and the
comma-joined value is used as current_jira_issue. This can fragment log buffering and transaction
naming/correlation across runs for the same set of issues.
Code

ymir/agents/mr_consolidation_agent.py[R468-472]

                all_jira.extend(_extract_jira_issues_from_description(mr.get("description", "")))
            state.jira_issues_collected = list(set(all_jira))
            state.jira_issue = state.jira_issues_collected[0] if state.jira_issues_collected else package
+            if state.jira_issues_collected:
+                current_jira_issue.set(",".join(state.jira_issues_collected))
Relevance

⭐⭐⭐ High

Team previously accepted order-preserving dedup of Jira issue lists (determinism concerns).

PR-#611

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The MR consolidation code builds the multi-issue string from a set, and the logging handler uses
the issue string as a dictionary key for buffering, so nondeterministic ordering causes multiple
distinct buffers/keys for the same issue set.

ymir/agents/mr_consolidation_agent.py[466-472]
ymir/common/logging_setup.py[37-67]

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

### Issue description
`list(set(all_jira))` yields an arbitrary order, so the resulting `current_jira_issue` string can vary run-to-run for identical inputs.

### Issue Context
`BufferedTaskHandler` buffers logs keyed by the exact `current_jira_issue` string, so different orderings create separate buffers for the same logical multi-issue set.

### Fix Focus Areas
- ymir/agents/mr_consolidation_agent.py[466-472]
- ymir/common/logging_setup.py[37-67]

### Suggested fix
- Replace `list(set(all_jira))` with `sorted(set(all_jira))` (or other stable ordering).
- Use the sorted list consistently for:
 - `state.jira_issues_collected`
 - `state.jira_issue = ...[0] ...`
 - `current_jira_issue.set(",".join(...))`

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


2. Recent traces N+1 queries ✓ Resolved 🐞 Bug ➹ Performance
Description
query_recent_traces() performs two additional SQL queries per returned trace (issues + counts),
which can become costly at higher limits and request rates. This increases latency and DB load for
/traces/recent.
Code

trace_server/server.py[R460-471]

+    results = []
+    for r in rows:
+        trace_id = r["trace_id"]
+        issues = db.execute(
+            "SELECT DISTINCT jira_issue FROM span_issues WHERE trace_id = ?",
+            (trace_id,),
+        ).fetchall()
+        counts = db.execute(
+            "SELECT COUNT(*) as cnt, SUM(CASE WHEN status_code = 2 THEN 1 ELSE 0 END) as errors "
+            "FROM spans WHERE trace_id = ?",
+            (trace_id,),
+        ).fetchone()
Relevance

⭐⭐ Medium

No prior trace_server reviews about avoiding N+1 query loops; performance standards unclear.

PR-#516
PR-#556

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The implementation explicitly executes two separate db.execute(...) calls inside a for r in rows
loop, creating two extra round trips per trace.

trace_server/server.py[437-485]

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()` fetches a list of recent traces, then loops and runs two extra queries per trace. This is an N+1 query pattern.

### Issue Context
`limit` can be up to `MAX_LAST_TRACES`, so the per-trace queries can dominate response time.

### Fix Focus Areas
- trace_server/server.py[437-485]

### Suggested fix
- After fetching the initial `rows`, collect `trace_ids` and fetch:
 - issues for all trace_ids in one query (e.g., `GROUP_CONCAT(DISTINCT jira_issue)` grouped by `trace_id`), and
 - counts/errors for all trace_ids in one query (`GROUP BY trace_id`).
- Merge these precomputed maps into `results` instead of querying inside the loop.

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


3. Null jira_key drops traces 🐞 Bug ◔ Observability
Description
In MR consolidation queue mode, jira_key becomes None when source_issues is empty and is used
for both current_jira_issue and start_transaction, which prevents jira.issue from being
attached to spans. Those runs become unsearchable by issue in the trace server and lose per-issue
log correlation.
Code

ymir/agents/mr_consolidation_agent.py[R1277-1283]

+            jira_key = ",".join(job.source_issues) if job.source_issues else None
            current_jira_issue.set(jira_key)
            try:
                with span_processor.start_transaction(
-                    f"consolidation-{job.package}",
+                    jira_key,
                    workflow="mr_consolidation",
                ):
Relevance

⭐⭐ Medium

Closest precedent: rejected guarding None transaction fields in observability (PR #580); unclear for
None jira_key.

PR-#580
PR-#630

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The queue-mode path explicitly sets jira_key = None when there are no source issues, and the span
processor only sets jira.issue when the current context value is truthy, so spans will lack the
attribute in that case.

ymir/agents/mr_consolidation_agent.py[1269-1283]
ymir/agents/observability.py[30-53]

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

### Issue description
`jira_key` is set to `None` when `job.source_issues` is empty, which causes `AgentSpanProcessor.on_start()` to skip setting the `jira.issue` span attribute. This removes trace-server indexing/correlation for those jobs.

### Issue Context
`AgentSpanProcessor.on_start()` only sets `jira.issue` when the context value is truthy.

### Fix Focus Areas
- ymir/agents/mr_consolidation_agent.py[1275-1283]
- ymir/agents/observability.py[30-53]

### Suggested fix
- Keep the multi-issue join when `source_issues` is non-empty, but add a deterministic fallback when it’s empty (e.g., `f"{job.package}/{job.target_branch}"`), and pass that fallback into both `current_jira_issue.set(...)` and `start_transaction(...)`.
- Ensure the fallback is a non-empty string so `on_start()` reliably attaches `jira.issue`.

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



Informational
4. Backfill skips issue splitting ✓ Resolved 🐞 Bug ≡ Correctness
Description
init_db() backfills span_issues from spans.jira_issue verbatim, so any historical rows with
comma-delimited issue strings remain unsplit in span_issues. After this PR, query_issues() and
query_spans() rely on span_issues, so those historical traces won’t be discoverable by
individual issue keys.
Code

trace_server/server.py[R122-134]

+    db.execute("""
+        CREATE TABLE IF NOT EXISTS span_issues (
+            trace_id TEXT NOT NULL,
+            span_id TEXT NOT NULL,
+            jira_issue TEXT NOT NULL,
+            PRIMARY KEY (trace_id, span_id, jira_issue)
+        )
+    """)
+    db.execute("CREATE INDEX IF NOT EXISTS idx_span_issues_issue ON span_issues(jira_issue)")
+    db.execute(
+        "INSERT OR IGNORE INTO span_issues (trace_id, span_id, jira_issue) "
+        "SELECT trace_id, span_id, jira_issue FROM spans WHERE jira_issue IS NOT NULL"
+    )
Relevance

⭐⭐ Medium

No historical review evidence about trace_server backfill splitting comma-delimited jira issues.

PR-#516
PR-#533

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The backfill inserts spans.jira_issue directly into span_issues, but the server now supports and
expects splitting comma-delimited values via _parse_jira_issues and uses span_issues for issue
listing.

trace_server/server.py[100-135]
trace_server/server.py[239-248]
trace_server/server.py[344-347]

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 multi-issue parsing logic is applied during ingest, but the DB backfill copies `spans.jira_issue` into `span_issues` without splitting comma-delimited values, leaving historical multi-issue rows unqueryable per-issue.

### Issue Context
After this PR, `query_issues()` reads distinct issues from `span_issues`, not `spans`.

### Fix Focus Areas
- trace_server/server.py[100-135]
- trace_server/server.py[239-248]
- trace_server/server.py[344-347]

### Suggested fix
- During `init_db()` (or as a one-time migration), rebuild/normalize `span_issues` from existing `spans` rows by splitting `spans.jira_issue` on commas (reuse `_parse_jira_issues`).
- Insert one row per (trace_id, span_id, individual_issue) instead of copying the raw combined string.

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


Qodo Logo

Comment thread ymir/agents/mr_consolidation_agent.py
Comment thread ymir/agents/mr_consolidation_agent.py
Comment thread trace_server/server.py Outdated
Comment thread trace_server/server.py Outdated
@nforro

nforro commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@nforro

nforro commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for multi-valued and propagated Jira issues across trace spans, backed by a new span_issues junction table in the SQLite database. It also adds a new /traces/recent endpoint to query recent root workflow spans, updates resource limits for the OpenShift OTEL collector, and allows egress traffic to Brave Search. Feedback focuses on ensuring data consistency by storing the full comma-separated list of issues in the legacy spans.jira_issue column, and providing a fallback value for jira_key in the MR consolidation agent when no source issues are present to avoid Sentry and tracing issues.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread trace_server/server.py Outdated
Comment thread ymir/agents/mr_consolidation_agent.py Outdated
Comment thread trace_server/server.py
Comment thread trace_server/server.py
Comment thread trace_server/server.py
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 026e1e3

@nforro

nforro commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@nforro

nforro commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several enhancements to tracing and database management, including a new /traces/recent endpoint, support for multi-valued Jira issues via a new span_issues junction table, and automated issue propagation across traces. It also adjusts OpenShift resource limits and egress rules. Feedback on these changes highlights a few critical areas: optimizing the database backfill migration to prevent startup performance bottlenecks, handling potential TypeError exceptions when parsing null array values, and ensuring fallback values are provided when a transaction's Jira key is null to prevent tracing failures.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread trace_server/server.py Outdated
Comment thread trace_server/server.py Outdated
Comment thread ymir/agents/mr_consolidation_agent.py
@nforro

nforro commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for multiple Jira issues per span by creating a new span_issues junction table, updating span ingestion and querying, and adding a /traces/recent endpoint. It also increases OpenShift OTel collector resource limits and allows egress traffic to Brave Search. The review feedback identifies three critical issues: a parameter binding mismatch in query_spans when combining filters, a potential sqlite3.IntegrityError during span ingestion due to duplicate entries, and downstream ValidationErrors caused by setting jira_issue to None where non-nullable strings are expected.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread trace_server/server.py Outdated
Comment thread trace_server/server.py
Comment thread ymir/agents/mr_consolidation_agent.py
@nforro

nforro commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the trace server and agents to support multiple Jira issues per trace/span, introducing a new span_issues junction table in SQLite and propagating issues across all spans in a trace. It also adds a new /traces/recent endpoint, introduces a SortedList validator for deterministic sorting of source issues, and updates OpenTelemetry collector resources and egress firewall rules. The reviewer suggested sorting the collected Jira issues in _resolve_source_issues to ensure deterministic behavior, matching the pattern used elsewhere in the codebase.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread ymir/agents/mr_consolidation_agent.py Outdated
@nforro

nforro commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new /traces/recent endpoint to query recent root workflow spans, refactors Jira issue tracking to support multiple issues per span via a new span_issues junction table, and updates agent workflows and Pydantic schemas to handle optional and multi-valued Jira issues. Feedback on these changes suggests removing unused columns from the SQL query in query_recent_traces, adding default=None to optional Pydantic fields to prevent validation errors, and deduplicating lists in the sort_list validator before sorting.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread trace_server/server.py
Comment thread ymir/common/models.py
Comment thread ymir/common/models.py
Comment thread ymir/common/validators.py Outdated
@nforro

nforro commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several updates to trace handling and agent workflows. Key changes include increasing resource limits for the OpenTelemetry collector, allowing egress traffic to Brave Search, and refactoring how Jira issues are stored and propagated. Specifically, a new span_issues junction table is introduced to support multiple Jira issues per span, along with a new /traces/recent endpoint to query recent root workflow spans. Additionally, several agent schemas and state models have been updated to allow jira_issue to be nullable and to enforce deduplicated, sorted lists of source issues. The feedback recommends wrapping the database query in the new /traces/recent endpoint within a try-except block to handle potential database exceptions gracefully and return a 500 error.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread trace_server/server.py
lbarcziova
lbarcziova previously approved these changes Jul 9, 2026

@lbarcziova lbarcziova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, couldn't spot anything problematic

cpu: "100m"
memory: 384Mi
cpu: "200m"
memory: 1Gi

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just out of curiosity, were we hitting significant issues with the lower resources? I see we still have some headroom in our namespace, so definitely let's accommodate to needs also for phoenix if needed

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only with trace server, when querying data for the past ~6 days. That should be improved by the new API endpoint, but I don't think raising the limits hurts anything.

@nforro

nforro commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the trace server to support multiple Jira issues per trace via a new span_issues junction table, propagates issues across spans in a trace, and adds a /traces/recent endpoint. It also updates OpenShift resource limits, allows null Jira issues in agent states, and enforces sorted, deduplicated issue lists. The review feedback highlights a potential Out-Of-Memory (OOM) issue during the database backfill in trace_server/server.py and suggests streaming and batching the query results instead of loading them all into memory with .fetchall().

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread trace_server/server.py Outdated
@nforro
nforro force-pushed the deployment branch 2 times, most recently from 616f0e1 to 908353f Compare July 9, 2026 07:51
@nforro

nforro commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for multi-valued and array-valued Jira issues, propagating them across all spans in a trace, and adds a new /traces/recent endpoint to query recent root workflow spans. It migrates the trace server database to use a new span_issues junction table, updates agent state models to allow nullable Jira issues, and introduces a UniqueSortedList validator for deterministic sorting and deduplication. Feedback on the changes highlights a critical issue in the SQL subquery joins in trace_server/server.py, where joining on span_id instead of trace_id can cause queries to fail for traces ingested across multiple batches.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread trace_server/server.py
@nforro

nforro commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for multiple and array-valued Jira issues per trace, propagating them across all spans in a trace and indexing them via a new span_issues junction table. It also adds a /traces/recent endpoint to query recent root workflow spans, updates agent schemas to support optional Jira issues, and increases resource limits for the OpenTelemetry collector. Feedback on the changes highlights a performance issue in trace_server/server.py where joining span_issues and spans solely on trace_id creates an inefficient Cartesian product; joining on both trace_id and span_id is recommended to optimize this to a 1-to-1 join.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread trace_server/server.py Outdated
Comment thread trace_server/server.py Outdated
Signed-off-by: Nikola Forró <nforro@redhat.com>
Assisted-by: Claude Opus 4.6 via Claude Code
@nforro

nforro commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several enhancements to trace and issue management. In trace_server/server.py, a new /traces/recent endpoint is added, and a span_issues junction table is introduced to support multiple Jira issues per span and propagate them across traces. The agent state and input schemas in ymir are updated to allow jira_issue to be optional (None) and to store source_issues as a deduplicated, sorted list (UniqueSortedList). Additionally, resource limits for the OpenShift OpenTelemetry collector are increased, and search.brave.com is added to the egress firewall rules. As there are no review comments, no further feedback is provided.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@nforro
nforro merged commit 3c3d1fc into packit:main Jul 9, 2026
11 checks passed
@nforro
nforro deleted the deployment branch July 9, 2026 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants