Skip to content

chore: use libdatadog's trace filter implementation#3986

Draft
Eldolfin wants to merge 2 commits into
masterfrom
oscarld/use-libdatadog-trace-filter-implementation
Draft

chore: use libdatadog's trace filter implementation#3986
Eldolfin wants to merge 2 commits into
masterfrom
oscarld/use-libdatadog-trace-filter-implementation

Conversation

@Eldolfin

Copy link
Copy Markdown

Description

Removes the current trace filter implementation and use libdatadog's implementation instead which more closely matches the agent's behavior.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@Eldolfin Eldolfin force-pushed the oscarld/use-libdatadog-trace-filter-implementation branch from 5be0f9c to 64c0891 Compare June 15, 2026 19:24
@Eldolfin Eldolfin requested review from bwoebi and Copilot June 15, 2026 19:24
@Eldolfin Eldolfin changed the title chore: use libdatadog trace filter implementation chore: use libdatadog's trace filter implementation Jun 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR replaces the project’s custom trace-filtering logic with libdatadog’s TraceFilterer implementation to better match agent behavior, and updates the C/Rust FFI surface accordingly.

Changes:

  • Swap components-rs/trace_filter.rs from an in-tree regex/tag filter implementation to libdd_trace_utils::trace_filter::TraceFilterer.
  • Remove the slow-path meta-iterator callback from the FFI API and update call sites and headers.
  • Update agent-info config plumbing to pass the filter lists into the concentrator config application.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tracer/trace_filter.c Updates the C-side call into ddog_check_stats_trace_filter to the new signature (no iterator).
components-rs/trace_filter.rs Replaces custom filter compilation/evaluation with TraceFilterer and adapts the span lookup interface.
components-rs/datadog.h Updates the exported C header signature/documentation for ddog_check_stats_trace_filter.
components-rs/common.h Removes the no-longer-used meta-iteration callback typedefs from the public header.
components-rs/agent_info.rs Adjusts how filter config fields are extracted and passed into apply_concentrator_config.
Cargo.lock Adds a new Rust dependency entry related to trace normalization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tracer/trace_filter.c
Comment on lines 80 to 87
bool ddtrace_trace_passes_filter(ddtrace_span_data *span) {
zval *root_resource_zv = &span->root->property_resource;
ZVAL_DEREF(root_resource_zv);
ddog_CharSlice resource = Z_TYPE_P(root_resource_zv) == IS_STRING
? dd_zend_string_to_CharSlice(Z_STR_P(root_resource_zv))
: DDOG_CHARSLICE_C("");
return ddog_check_stats_trace_filter(resource, span, ddtrace_root_tag_value, ddtrace_meta_iter);
return ddog_check_stats_trace_filter(resource, span, ddtrace_root_tag_value);
}
Comment on lines +78 to +81
fn resource_normalized(&'a self) -> &'a str {
// FIXME: normalization: if resource is empty, name should be used instead
self.resource_str
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

true but the fix is very wrong

Comment thread components-rs/trace_filter.rs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@datadog-prod-us1-5

datadog-prod-us1-5 Bot commented Jun 15, 2026

Copy link
Copy Markdown

Pipelines  Tests

Fix all issues with BitsAI

⚠️ Warnings

🚦 16 Pipeline jobs failed

DataDog/apm-reliability/dd-trace-php | pecl tests: [8.2]   View in Datadog   GitLab

DataDog/apm-reliability/dd-trace-php | test_extension_ci: [7.1]   View in Datadog   GitLab

DataDog/apm-reliability/dd-trace-php | test_extension_ci: [7.2]   View in Datadog   GitLab

View all 16 failed jobs.

ℹ️ Info

No other issues found (see more)

🧪 All tests passed
❄️ No new flaky tests detected

🔄 Datadog auto-retried 1 job - 1 passed on retry View in Datadog

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 54.08% (-0.03%)

Useful? React with 👍 / 👎

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

@pr-commenter

pr-commenter Bot commented Jun 15, 2026

Copy link
Copy Markdown

Benchmarks [ tracer ]

Benchmark execution time: 2026-06-15 20:52:53

Comparing candidate commit 3f0f4b6 in PR branch oscarld/use-libdatadog-trace-filter-implementation with baseline commit a65b400 in branch master.

Found 0 performance improvements and 3 performance regressions! Performance is the same for 191 metrics, 0 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:PDOBench/benchPDOOverhead

  • 🟥 execution_time [+8.115µs; +10.171µs] or [+3.167%; +3.969%]

scenario:PDOBench/benchPDOOverheadWithDBM

  • 🟥 execution_time [+7.787µs; +10.608µs] or [+3.031%; +4.129%]

scenario:PHPRedisBench/benchRedisOverhead

  • 🟥 execution_time [+37.070µs; +53.647µs] or [+3.666%; +5.306%]

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