Skip to content

Fix span_name signal trigger not matching spans from previous batches#1621

Merged
Rainhunter13 merged 1 commit intodevfrom
agent/lam-1437/trigger-for-span-name-doesn-t-work/811a19
Apr 9, 2026
Merged

Fix span_name signal trigger not matching spans from previous batches#1621
Rainhunter13 merged 1 commit intodevfrom
agent/lam-1437/trigger-for-span-name-doesn-t-work/811a19

Conversation

@laminar-coding-agent
Copy link
Copy Markdown
Contributor

@laminar-coding-agent laminar-coding-agent bot commented Apr 9, 2026

Summary

  • Bug: Signal triggers with "contains span with name" filter only checked spans in the current processing batch, not spans from previous batches stored in the database. When a child span (e.g. "GitHub") arrived in a different batch than the root span, the trigger would never fire.
  • Fix: The span_name filter in evaluate_single_filter now also checks the accumulated span_names from the database trace record (merged across all batches via JSONB || on upsert), in addition to the current batch's spans.
  • Tests: Added 3 unit tests covering the cross-batch scenario, same-batch matching, and Ne operator with accumulated names.

Root cause

Spans arrive at the app-server in batches. check_and_push_signals() runs after each batch is processed. The span_name filter was only scanning the raw spans array from the current batch:

// Before (broken): only checks current batch
let has_span = spans.iter()
    .filter(|s| s.trace_id == self.id)
    .any(|s| s.name == target_name);

When child span "GitHub" arrived in batch 1 and the root span in batch 2:

  1. Batch 1: root_span_finished = false (no root yet) → trigger skipped
  2. Batch 2: root_span_finished = true, but "GitHub" not in this batch → trigger skipped

The fix checks the DB-accumulated span_names (which merges names from all batches):

// After (fixed): checks accumulated DB names + current batch
let has_span = self.span_names().iter().any(|n| n == target_name)
    || spans.iter()
        .filter(|s| s.trace_id == self.id)
        .any(|s| s.name == target_name);

Test plan

  • cargo check passes
  • 3 new unit tests pass: cargo test --bin app-server db::trace::tests
    • test_span_name_filter_uses_accumulated_db_span_names — the exact bug scenario
    • test_span_name_filter_matches_current_batch — regression: same-batch still works
    • test_span_name_ne_filter_with_accumulated_names — Ne operator correctness

🤖 Generated with Claude Code


Note

Medium Risk
Moderate risk because it changes signal trigger evaluation semantics in the Rust backend, potentially affecting when/which signals fire; logic is straightforward and covered by new unit tests.

Overview
Fixes span_name signal trigger filters to consider DB-accumulated span names (traces.span_names) in addition to spans from the current processing batch, so triggers can fire when relevant spans arrive in different batches.

Adds unit tests for cross-batch matching, same-batch regression, and Ne operator behavior, and documents the trigger evaluation flow and batch/DB considerations in CLAUDE.md.

Reviewed by Cursor Bugbot for commit fae96b0. Bugbot is set up for automated code reviews on this repo. Configure here.

The span_name filter in signal trigger evaluation only checked spans in
the current processing batch, missing spans that arrived in earlier
batches. When a child span (e.g. "GitHub") arrived before the root span,
the trigger would never fire: the first batch lacked root_span_finished,
and the second batch lacked the child span name.

Fix: also check the accumulated span_names from the database trace
record, which merges span names across all batches via JSONB || on
upsert.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 9, 2026

Greptile Summary

This PR fixes a bug where the span_name signal trigger filter only checked spans in the current processing batch, causing triggers to silently miss traces where the target span arrived in an earlier batch than the root span. The fix augments evaluate_single_filter to also check the DB-accumulated span_names JSONB field (which is merged across all batches via the || upsert operator), in addition to the current-batch spans array.

Key points:

  • Root cause correctly identified: upsert_trace_statistics_batch already merges span_names across batches and returns the full merged trace via RETURNING, so self.span_names() is the authoritative accumulated set at trigger evaluation time.
  • Duplicate-fire concern is pre-handled: The existing SIGNAL_TRIGGER_LOCK_CACHE_KEY lock (1-hour TTL, keyed by project_id + signal_id + trace_id) in check_and_push_signals prevents the same trigger from firing multiple times per trace within the TTL window.
  • Ne operator is semantically correct: Since span_names only grows (JSONB || merge), once a span name is present the Ne filter permanently returns false, which is the intended behaviour.
  • Current-batch fallback (|| spans.iter()...) is technically redundant after the upsert returns the merged trace, but is a harmless safety net.
  • Three unit tests cover the exact bug scenario, the same-batch regression, and the Ne operator.

Confidence Score: 5/5

Safe to merge — targeted, correct bug fix with comprehensive tests and no new risk vectors.

The change is minimal and surgical: one OR-clause added to a single filter branch, three unit tests that directly validate the bug scenario and edge cases. The potential re-fire concern from persistent span_names is already mitigated by the pre-existing 1-hour cache lock in check_and_push_signals. The RETURNING-based upsert ensures the accumulated state is authoritative before filter evaluation. No security, data-loss, or reliability regressions identified.

No files require special attention.

Vulnerabilities

No security concerns identified. The change is limited to filter evaluation logic using pre-fetched DB data with no new input paths or privilege boundaries.

Important Files Changed

Filename Overview
app-server/src/db/trace.rs Adds accumulated span_names check to span_name filter evaluation and three unit tests; logic is correct and deduplication is handled upstream by the existing signal trigger lock.

Reviews (1): Last reviewed commit: "Fix span_name signal trigger not matchin..." | Re-trigger Greptile

@laminar-coding-agent
Copy link
Copy Markdown
Contributor Author

This comment is an automated summary/review from the Greptile bot, not a change request. It gives the PR a 5/5 confidence score and concludes "Safe to merge" with no issues identified and no files requiring special attention. There are no actionable code changes requested — it's purely an informational summary of the PR's correctness.

No changes needed.

@Rainhunter13 Rainhunter13 merged commit d9d99c9 into dev Apr 9, 2026
4 checks passed
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.

2 participants