Skip to content

Conversation

@buenaflor
Copy link
Contributor

@buenaflor buenaflor commented Jan 26, 2026

Summary

  • Adds implementation requirement specifying that ignoreSpans patterns must be evaluated before the span is started

🤖 Generated with Claude Code

Add implementation requirement specifying that ignoreSpans patterns
must be evaluated before the span is started to ensure ignored spans
are never created and avoid unnecessary overhead.

Co-Authored-By: Claude <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Jan 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
develop-docs Ready Ready Preview, Comment Jan 28, 2026 10:01am
1 Skipped Deployment
Project Deployment Review Updated (UTC)
sentry-docs Ignored Ignored Preview Jan 28, 2026 10:01am

Request Review

@buenaflor buenaflor changed the title docs(sdk): Clarify ignoreSpans must be evaluated before span starts feat(span-first): Clarify ignoreSpans must be evaluated before span starts Jan 26, 2026
Clarified implementation requirements for span filtering, including evaluation order and matching rules for attributes.
Clarified implementation requirements for ignoring spans in telemetry.
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

thanks!

Copy link
Contributor

@coolguyzone coolguyzone left a comment

Choose a reason for hiding this comment

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

Looks good!

buenaflor and others added 2 commits January 28, 2026 10:56
Co-authored-by: Alex Krawiec <alex.krawiec@sentry.io>
Co-authored-by: Alex Krawiec <alex.krawiec@sentry.io>
@buenaflor buenaflor enabled auto-merge (squash) January 28, 2026 09:56
Comment on lines +82 to 84
1. The `ignoreSpans` patterns MUST be evaluated **before** the span is started.
2. The `ignoreSpans` patterns MUST be applied to all spans, including the root or segment span.
- If a pattern matches the root span, the span and all its children MUST be ignored.
Copy link

Choose a reason for hiding this comment

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

Bug: The documentation requires ignoreSpans to be evaluated before a span starts, but provides examples that filter on attributes like http.status_code, which are only available after.
Severity: MEDIUM

Suggested Fix

Re-evaluate the ignoreSpans requirement. Either remove the strict "MUST be evaluated before the span is started" clause, or update the examples to only use attributes available at span creation. Alternatively, explicitly state that filtering on post-start attributes is not supported, or specify a multi-stage filtering process if it is intended to be supported.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: develop-docs/sdk/telemetry/spans/filtering.mdx#L82-L84

Potential issue: The documentation introduces a logical contradiction by requiring
`ignoreSpans` patterns to be evaluated before a span starts, while simultaneously
providing examples that filter on attributes only available after a span completes, such
as `http.status_code`. This creates an impossible implementation scenario for SDK
developers, forcing them to either ignore the new strict requirement or fail to support
the documented examples. This ambiguity will likely lead to inconsistent filtering
behavior across different SDKs, undermining the goal of a unified specification.

Did we get this right? 👍 / 👎 to inform future reviews.

@buenaflor buenaflor merged commit bb20b2a into master Jan 28, 2026
14 checks passed
@buenaflor buenaflor deleted the docs/clarify-ignorespans-evaluation-timing branch January 28, 2026 10:02
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.

4 participants