Skip to content

Add trace filter tests [APMSP-2763]#6952

Open
Eldolfin wants to merge 22 commits into
mainfrom
oscarld/add-trace-filter-test
Open

Add trace filter tests [APMSP-2763]#6952
Eldolfin wants to merge 22 commits into
mainfrom
oscarld/add-trace-filter-test

Conversation

@Eldolfin

@Eldolfin Eldolfin commented May 15, 2026

Copy link
Copy Markdown
Contributor

APMSP-2763
APMSP-3048

Motivation

Changes

Add system-tests coverage for the CSS V1.2 spec trace filters (filter_tags, filter_tags_regex, ignore_resources).

TODO

  • add resources/tags normalization test (maybe in a follow up PR, this is an edge-edge case)

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed and the CI green, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • Anything but tests/ or manifests/ is modified ? I have the approval from R&P team
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added, removed or renamed?

@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

CODEOWNERS have been resolved as:

tests/parametric/test_trace_filters.py                                  @DataDog/system-tests-core @DataDog/apm-sdk-capabilities
manifests/cpp.yml                                                       @DataDog/dd-trace-cpp
manifests/dotnet.yml                                                    @DataDog/apm-dotnet @DataDog/asm-dotnet
manifests/golang.yml                                                    @DataDog/dd-trace-go-guild
manifests/java.yml                                                      @DataDog/asm-java @DataDog/apm-java
manifests/nodejs.yml                                                    @DataDog/dd-trace-js
manifests/php.yml                                                       @DataDog/apm-php @DataDog/asm-php
manifests/python.yml                                                    @DataDog/apm-python @DataDog/asm-python
manifests/ruby.yml                                                      @DataDog/ruby-guild @DataDog/asm-ruby
manifests/rust.yml                                                      @DataDog/apm-rust
tests/parametric/test_library_tracestats.py                             @DataDog/system-tests-core @DataDog/apm-sdk-capabilities

@Eldolfin Eldolfin changed the title feat: add trace filter tests (checked on stats) feat: add trace filter tests [APMSP-2763] May 15, 2026
@Eldolfin Eldolfin changed the title feat: add trace filter tests [APMSP-2763] Add trace filter tests [APMSP-2763] May 15, 2026
gh-worker-dd-mergequeue-cf854d Bot pushed a commit to DataDog/libdatadog that referenced this pull request May 19, 2026
… scheme (#2008)

These fields were renamed in version 7.79.0 of the agent for consistency.

# What does this PR do?

Allow both PascalCase and the newer snake_case format for these fields.

# Motivation

The trace exporter was crashing in a confusing way in new system tests I wrote for another feature: DataDog/system-tests#6952. I had an older agent.

# Additional Notes

Anything else we should know when reviewing?

# How to test the change?

Describe here in detail how the change can be validated.


Co-authored-by: oscar.ledauphin <oscar.ledauphin@datadoghq.com>
gh-worker-dd-mergequeue-cf854d Bot pushed a commit to DataDog/libdatadog that referenced this pull request Jun 16, 2026
# What does this PR do?

Implements the CSS trace-level filtering mechanism, that is applied before stats computation.

# Motivation

It's un unmet requirement from the spec.

# Additional Notes
~Please Ctrl+F `FIXME` when reviewing, I let some questions in the code that I'd like answers for before merging~

## TODO:

- [x] unit tests
- [x] system-tests DataDog/system-tests#6952
- [x] normalization before filtering
- [x] regex keys (currently only regex values are checked)
- [x] new spec or CSS spec RFC
- [x] telemetry ? Count dropped traces with the reason
- [x] trim spaces when parsing config
- [x] Move implementation to trace-utils
- [x] stop using v4 span but generic spans instead for php
- [x] move arcswap to the trace exporter (remove update_conf)
- [x] use test agent for snapshot tests

## Wont do
- [ ] ~advanced resource normalization (sql, redis etc...).~ No other implementation does that
- [ ] ~100% normalization edge cases (empty service field + non-normalized name + filter depending on that)~ Too specific edge case. Ignoring it for now
- [ ] ~check on meta_struct too~ Not in the spec


[APMSP-2763]

[APMSP-2763]: https://datadoghq.atlassian.net/browse/APMSP-2763?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Co-authored-by: oscar.ledauphin <oscar.ledauphin@datadoghq.com>
@datadog-official

datadog-official Bot commented Jun 17, 2026

Copy link
Copy Markdown

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 85ab44e | Docs | Datadog PR Page | Give us feedback!

@Eldolfin Eldolfin marked this pull request as ready for review July 2, 2026 12:54
@Eldolfin Eldolfin requested review from a team as code owners July 2, 2026 12:54
@Eldolfin Eldolfin requested review from a team as code owners July 2, 2026 12:54
@Eldolfin Eldolfin requested review from P403n1x87, brettlangdon, daniel-romano-DD, dubloom and manuel-alvarez-alvarez and removed request for a team July 2, 2026 12:54
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

assert interfaces.library.wait_for(wait_function, 30), f"Timed out waiting for request with status {status_code}"

P2 Badge Avoid aborting scenarios from setup waits

This helper is called from the new setup_* methods, and in this framework pytest_collection_finish invokes those setup methods before tests run and closes targets/re-raises on any setup exception (conftest.py lines 449-478). If a tracer fails to emit the control stats payload within 30s, this assertion aborts the entire scenario during setup instead of reporting only the affected test failure; the wait result should be recorded and asserted in the matching test_* method.


"tests/stats/test_trace_filters.py::Test_Trace_Filters_Regex_Require::test_filter_tags_regex_require": [
"TRACE_STATS_COMPUTATION_TRACE_FILTER_REGEX_REQUIRE"
],

P2 Badge Add the missing trace-filter scenarios to the map

compute_scenarios_by_files builds the scenarios for a modified test file solely from the nodeids present in this JSON map (utils/scripts/compute_libraries_and_scenarios.py lines 200-207). The added entries stop after the regex-require class, so changes to tests/stats/test_trace_filters.py will not select the three new edge-case scenarios (TRACE_STATS_COMPUTATION_TRACE_FILTER_REJECT_EDGE_CASES, TRACE_STATS_COMPUTATION_TRACE_FILTER_KEY_ONLY_REJECT, and TRACE_STATS_COMPUTATION_TRACE_FILTER_REQUIRE_EDGE_CASES) even though their tests were added in the same file.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread utils/_context/_scenarios/__init__.py Outdated
@Eldolfin Eldolfin requested a review from a team as a code owner July 3, 2026 18:37
@Eldolfin Eldolfin requested review from mabdinur and removed request for a team July 3, 2026 18:37
@Eldolfin Eldolfin force-pushed the oscarld/add-trace-filter-test branch from 97052c0 to 1a7794c Compare July 3, 2026 18:38
Comment thread manifests/cpp.yml Outdated
"""A require filter drops traces whose root span lacks the required tag."""
with test_library, test_library.dd_start_span(name="web.request"):
pass
assert not test_agent.traces(), "trace lacking the required tag should be dropped"

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.

nit: why not check for stats here ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These tests are focused on trace filters specifically, so they should not fail if CSS has a problem.

When I tried checking stats here using php it failed for the same reason as why CSS tests are disabled for php:

missing_feature (CSS v1.2.0: dd-trace-php ships CSS since v1.19.0, but the sidecar stats exporter retains buckets younger than ~20s (buffer_len=2 * bucket_size=10s) and dd_trace_synchronous_flush passes force=false, so short-lived parametric tests never observe a /v0.6/stats request)

Comment thread tests/parametric/test_trace_filters.py Outdated

@cbeauchesne cbeauchesne left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From framework usage, all good.

I let someone familiar with the feature validate the test logic and approve the PR

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