chore(agent-data-plane): tag_filterlist correctness test#1251
chore(agent-data-plane): tag_filterlist correctness test#1251rayz wants to merge 8 commits intoolivielpeau/tag-aggregation-pre-aggrfrom
tag_filterlist correctness test#1251Conversation
dd-octo-sts
bot
added
area/components
There was a problem hiding this comment.
Pull request overview
Adds a new correctness test case covering DogStatsD metric tag filterlist behavior and fixes ground-truth metric normalization so tag ordering differences don’t incorrectly split/duplicate aggregated metrics.
Changes:
- Add a new
dsd-tag-filterlistcorrectness scenario (Millstone corpus + Agent config + runner config) and wire it intomake test-correctness. - Normalize metric contexts (sort/dedup tags) before grouping/aggregating in ground-truth metric analysis, plus add a regression unit test.
- Minor import cleanup in the tag filterlist transform module.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/correctness/dsd-tag-filterlist/millstone.yaml | Defines a deterministic corpus that exercises include/exclude tag filtering and tag-order variation. |
| test/correctness/dsd-tag-filterlist/datadog.yaml | Configures DogStatsD UDS + tag filterlist rules for the scenario. |
| test/correctness/dsd-tag-filterlist/config.yaml | Registers the new correctness case (baseline vs comparison containers). |
| lib/saluki-components/src/transforms/tag_filterlist/mod.rs | Simplifies the Metric type import used by filter_metric_tags. |
| bin/correctness/ground-truth/src/analysis/metrics/types.rs | Fixes grouping by using normalized contexts so equivalent tag sets match regardless of order; adds unit test. |
| Makefile | Adds test-correctness-dsd-tag-filterlist and includes it in test-correctness. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Regression Detector (Agent Data Plane)This comment was omitted because it was over 65,536 characters.Please check the Gitlab Job logs to see its output. |
Binary Size Analysis (Agent Data Plane)Target: 1d50af4 (baseline) vs b9edd70 (comparison) diff
|
| Module | File Size | Symbols |
|---|---|---|
core |
+29.56 KiB | 8776 |
saluki_components::transforms::tag_filterlist |
+26.19 KiB | 18 |
serde_core |
+8.78 KiB | 338 |
[sections] |
+6.92 KiB | 7 |
agent_data_plane::cli::run |
+6.58 KiB | 70 |
hashbrown |
+5.59 KiB | 283 |
[Unmapped] |
+2.73 KiB | 1 |
saluki_config::dynamic::watcher |
-2.34 KiB | 2 |
saluki_context::hash::hash_context |
+2.15 KiB | 1 |
tokio |
+1.69 KiB | 2123 |
saluki_core::topology::blueprint |
+1.19 KiB | 27 |
saluki_context::context::Context |
+894 B | 4 |
smallvec |
+753 B | 69 |
saluki_context::resolver::TagsResolver |
-574 B | 13 |
saluki_components::encoders::datadog |
+379 B | 288 |
serde_json |
+281 B | 171 |
saluki_config::ConfigurationLoader::from_environment |
+249 B | 1 |
saluki_components::common::datadog |
-240 B | 326 |
saluki_components::sources::otlp |
-169 B | 164 |
saluki_common::task::instrument |
-165 B | 76 |
Detailed Symbol Changes
FILE SIZE VM SIZE
-------------- --------------
[NEW] +1.79Mi [NEW] +1.79Mi std::thread::local::LocalKey<T>::with::hf947cb2a7d030d57
[NEW] +121Ki [NEW] +120Ki agent_data_plane::cli::run::create_topology::_{{closure}}::hc39e8af12512af57
[NEW] +84.6Ki [NEW] +84.5Ki agent_data_plane::internal::control_plane::spawn_control_plane::_{{closure}}::h5be1ddd17aeb8c4f
+0.6% +83.5Ki +0.7% +70.3Ki [22545 Others]
[NEW] +64.2Ki [NEW] +64.0Ki saluki_components::common::datadog::io::run_endpoint_io_loop::_{{closure}}::h8ea6d1e501a9b09a
[NEW] +57.8Ki [NEW] +57.7Ki agent_data_plane::cli::run::handle_run_command::_{{closure}}::h663761cb14f422f3
[NEW] +49.5Ki [NEW] +49.3Ki saluki_app::bootstrap::AppBootstrapper::bootstrap::_{{closure}}::h6bb4e73bcd65b796
[NEW] +46.4Ki [NEW] +46.3Ki h2::proto::connection::Connection<T,P,B>::poll::h35223740d8685346
[NEW] +46.1Ki [NEW] +45.9Ki _<saluki_components::forwarders::otlp::OtlpForwarder as saluki_core::components::forwarders::Forwarder>::run::_{{closure}}::heae6862569bb613c
[NEW] +45.9Ki [NEW] +45.7Ki _<saluki_components::destinations::prometheus::Prometheus as saluki_core::components::destinations::Destination>::run::_{{closure}}::hb3bed4ad4a0f4d9e
[NEW] +44.4Ki [NEW] +44.2Ki _<saluki_components::transforms::aggregate::Aggregate as saluki_core::components::transforms::Transform>::run::_{{closure}}::h3292df23d23f2b59
[DEL] -44.4Ki [DEL] -44.2Ki _<saluki_components::transforms::aggregate::Aggregate as saluki_core::components::transforms::Transform>::run::_{{closure}}::hb4d1ad0684099c4c
[DEL] -46.0Ki [DEL] -45.8Ki _<saluki_components::destinations::prometheus::Prometheus as saluki_core::components::destinations::Destination>::run::_{{closure}}::hea5618e9afd08b63
[DEL] -46.1Ki [DEL] -45.9Ki _<saluki_components::forwarders::otlp::OtlpForwarder as saluki_core::components::forwarders::Forwarder>::run::_{{closure}}::hb0c627893c271e2c
[DEL] -46.4Ki [DEL] -46.3Ki h2::proto::connection::Connection<T,P,B>::poll::h100071ec98c95c21
[DEL] -49.5Ki [DEL] -49.4Ki saluki_app::bootstrap::AppBootstrapper::bootstrap::_{{closure}}::h15f926ef5659e580
[DEL] -57.8Ki [DEL] -57.7Ki agent_data_plane::cli::run::handle_run_command::_{{closure}}::hebabd4c0d26708e2
[DEL] -64.2Ki [DEL] -64.0Ki saluki_components::common::datadog::io::run_endpoint_io_loop::_{{closure}}::h1c7964dcd8ecbead
[DEL] -84.6Ki [DEL] -84.5Ki agent_data_plane::internal::control_plane::spawn_control_plane::_{{closure}}::h78c38079d35f4f77
[DEL] -114Ki [DEL] -114Ki agent_data_plane::cli::run::create_topology::_{{closure}}::h4a1b0b37b7e2d03e
[DEL] -1.79Mi [DEL] -1.79Mi std::thread::local::LocalKey<T>::with::he517c09e5477efe1
+0.3% +90.6Ki +0.3% +77.4Ki TOTAL
rayz
changed the title
tag_filterlist correctness test
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 20 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| additional_env_vars: | ||
| - DD_API_KEY=correctness-test | ||
| comparison: | ||
| image: registry.datadoghq.com/agent-dev:ca27cfe6-py3-jmx |
There was a problem hiding this comment.
comparison.image is pinned to a specific registry.datadoghq.com/agent-dev:<sha> image. In CI this value is overridden by GROUND_TRUTH_COMPARISON__IMAGE (see .gitlab/e2e.yml), so the pin only affects local runs and may break if the agent-dev tag is garbage-collected or if a developer lacks access. Consider switching this to the usual placeholder saluki-images/datadog-agent:testing-release (as in test/correctness/dsd-plain/config.yaml:9-16 / otlp-metrics/config.yaml:9-17) and rely on env-var overrides when you explicitly want to test an agent-dev build.
| image: registry.datadoghq.com/agent-dev:ca27cfe6-py3-jmx | |
| image: saluki-images/datadog-agent:testing-release |
| additional_env_vars: | ||
| - DD_API_KEY=correctness-test | ||
| comparison: | ||
| image: registry.datadoghq.com/agent-dev:aef811fd-py3-jmx |
There was a problem hiding this comment.
comparison.image is pinned to a specific registry.datadoghq.com/agent-dev:<sha> image. In CI this value is overridden by GROUND_TRUTH_COMPARISON__IMAGE (see .gitlab/e2e.yml), so the pin only affects local runs and may break if the agent-dev tag is removed or if a developer lacks registry access. Consider using the standard placeholder saluki-images/datadog-agent:testing-release (e.g. test/correctness/dsd-plain/config.yaml) and overriding via env vars when you need an agent-dev comparison.
| image: registry.datadoghq.com/agent-dev:aef811fd-py3-jmx | |
| image: saluki-images/datadog-agent:testing-release |
| additional_env_vars: | ||
| - DD_API_KEY=correctness-test | ||
| comparison: | ||
| image: registry.datadoghq.com/agent-dev:ff848d05-py3-jmx |
There was a problem hiding this comment.
comparison.image is pinned to a specific registry.datadoghq.com/agent-dev:<sha> tag. This can make local make test-correctness-* runs brittle (tag GC / missing registry creds) while CI will override the value via GROUND_TRUTH_COMPARISON__IMAGE anyway. Recommend switching to the repo’s usual placeholder saluki-images/datadog-agent:testing-release and using env-var overrides only for one-off agent-dev comparisons.
| image: registry.datadoghq.com/agent-dev:ff848d05-py3-jmx | |
| image: saluki-images/datadog-agent:testing-release |
| additional_env_vars: | ||
| - DD_API_KEY=correctness-test | ||
| comparison: | ||
| image: registry.datadoghq.com/agent-dev:62567276-py3-jmx |
There was a problem hiding this comment.
comparison.image is pinned to a specific registry.datadoghq.com/agent-dev:<sha> image. Since CI overrides comparison.image via GROUND_TRUTH_COMPARISON__IMAGE, this pin only impacts local runs and is prone to breaking when the tag is deleted/GC’d. Consider using the standard local placeholder saluki-images/datadog-agent:testing-release (consistent with other correctness configs) and overriding via env vars when you explicitly need an agent-dev build.
| image: registry.datadoghq.com/agent-dev:62567276-py3-jmx | |
| image: saluki-images/datadog-agent:testing-release |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 23 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
test/correctness/dsd-additional-optimized-tag-filterlist-fix/config.yaml:20
- This correctness case directory appears to be unused: it is not referenced by any
make test-correctness-*target nor by.gitlab/e2e.yml, so it will likely bit-rot and confuse future readers. Either wire it into the Makefile/CI (if it’s meant to be a supported test case) or remove it / fold it into the intended case (e.g., document it as a local-only reproduction and keep it out of the main suite).
analysis_mode: metrics
millstone:
image: saluki-images/millstone:latest
config_path: millstone.yaml
datadog_intake:
image: saluki-images/datadog-intake:latest
config_path: ../datadog-intake.yaml
baseline:
image: registry.datadoghq.com/agent:7.76.2-jmx
files:
- datadog.yaml:/etc/datadog-agent/datadog.yaml
additional_env_vars:
- DD_API_KEY=correctness-test
comparison:
image: datadog/agent-dev:efcf783f-py3-jmx
files:
- datadog.yaml:/etc/datadog-agent/datadog.yaml
additional_env_vars:
- DD_API_KEY=correctness-test
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| files: | ||
| - datadog.yaml:/etc/datadog-agent/datadog.yaml | ||
| additional_env_vars: | ||
| - DD_API_KEY=correctness-test |
There was a problem hiding this comment.
In CI, .test-correctness-definition overrides comparison.image via GROUND_TRUTH_COMPARISON__IMAGE (bundled ADP image), but this config’s comparison.additional_env_vars does not enable ADP/DogStatsD data plane. As a result, the comparison target will run without the tag_filterlist transform and the test won’t exercise the intended behavior. Add the same DD_DATA_PLANE_ENABLED=true, DD_DATA_PLANE_DOGSTATSD_ENABLED=true (and any other required ADP vars like DD_AGGREGATE_CONTEXT_LIMIT) to comparison.additional_env_vars, or adjust the GitLab job to avoid overriding the comparison image for this case.
| - DD_API_KEY=correctness-test | |
| - DD_API_KEY=correctness-test | |
| - DD_DATA_PLANE_ENABLED=true | |
| - DD_DATA_PLANE_DOGSTATSD_ENABLED=true | |
| - DD_AGGREGATE_CONTEXT_LIMIT=100000 |
| files: | ||
| - datadog.yaml:/etc/datadog-agent/datadog.yaml | ||
| additional_env_vars: | ||
| - DD_API_KEY=correctness-test |
There was a problem hiding this comment.
This correctness case is wired into CI with .test-correctness-definition, which overrides comparison.image to the bundled ADP image. However, comparison.additional_env_vars here does not enable ADP/DogStatsD data plane, so in CI the comparison target will behave like a regular Agent and won’t apply the tag filterlist logic under test. Add the ADP enablement env vars under comparison.additional_env_vars (and keep DD_AGGREGATE_CONTEXT_LIMIT if needed), or change the CI job so this case uses the image declared in the YAML instead of the overridden one.
| - DD_API_KEY=correctness-test | |
| - DD_API_KEY=correctness-test | |
| - DD_ADP_ENABLED=true | |
| - DD_DOGSTATSD_ADP_ENABLED=true |
| files: | ||
| - datadog.yaml:/etc/datadog-agent/datadog.yaml | ||
| additional_env_vars: | ||
| - DD_API_KEY=correctness-test |
There was a problem hiding this comment.
CI overrides comparison.image for correctness tests (via GROUND_TRUTH_COMPARISON__IMAGE), but this case’s comparison.additional_env_vars only sets DD_API_KEY. Without DD_DATA_PLANE_ENABLED=true + DD_DATA_PLANE_DOGSTATSD_ENABLED=true (and any required aggregation settings), the comparison target won’t run the ADP path and the tag_filterlist behavior won’t be validated. Add the ADP enablement env vars here, or adjust the CI job so it doesn’t override the comparison image for this specific test case.
| - DD_API_KEY=correctness-test | |
| - DD_API_KEY=correctness-test | |
| - DD_DATA_PLANE_ENABLED=true | |
| - DD_DATA_PLANE_DOGSTATSD_ENABLED=true |
| files: | ||
| - datadog.yaml:/etc/datadog-agent/datadog.yaml | ||
| additional_env_vars: | ||
| - DD_API_KEY=correctness-test |
There was a problem hiding this comment.
Like the other newly-added tag-filterlist correctness cases, this config is run in CI with .test-correctness-definition, which overrides comparison.image to the bundled ADP image. comparison.additional_env_vars here doesn’t enable ADP/DogStatsD data plane, so the comparison run in CI won’t exercise the tag_filterlist transform. Add DD_DATA_PLANE_ENABLED=true / DD_DATA_PLANE_DOGSTATSD_ENABLED=true (and any other required ADP env vars) to comparison.additional_env_vars, or modify the CI job so this case uses the image specified in the YAML instead of the overridden one.
| - DD_API_KEY=correctness-test | |
| - DD_API_KEY=correctness-test | |
| - DD_DATA_PLANE_ENABLED=true | |
| - DD_DATA_PLANE_DOGSTATSD_ENABLED=true |
Summary
This pr adds a new correctness test for tag aggregation implemented in #1247
Change Type
How did you test this PR?
CI
References