fix(dogstatsd): preserve cardinality for aggregated set metrics

[jaideeppyne]

What

When client-side aggregation is enabled, set metrics silently drop their cardinality tag on the wire, while count and gauge keep it.

statsd.gauge("g", 1, cardinality="high")      # -> g:1|g|card:high      ✅
statsd.increment("c", 2, cardinality="high")  # -> c:2|c|card:high      ✅
statsd.set("s", "v", cardinality="high")      # -> s:v|s   (card:high dropped)  ❌

Why

flush_aggregated_metrics() re-emits each aggregated metric with cardinality=m.cardinality. For count and gauge the flushed object is the original metric, so its cardinality survives. But SetMetric.get_data() rebuilds fresh MetricAggregators per value without passing cardinality, so it defaults to None and the |card:<value> tag is stripped:

# datadog/dogstatsd/metrics.py — SetMetric.get_data()
MetricAggregator(self.name, self.tags, self.rate, MetricType.SET, value)  # cardinality lost

This completes #929, which added cardinality=m.cardinality to the flush loops in base.py (intending cardinality to flow through all aggregated metrics) but did not cover the SetMetric.get_data() path — so set is the one metric type where the feature is broken.

How

Pass cardinality=self.cardinality when rebuilding the per-value aggregators:

-            MetricAggregator(self.name, self.tags, self.rate, MetricType.SET, value)
+            MetricAggregator(
+                self.name, self.tags, self.rate, MetricType.SET, value, cardinality=self.cardinality
+            )

After the fix the set packet is s:v|s|card:high.

Tests

Added test_aggregated_metrics_with_cardinality_when_aggregation_enabled (mirrors the existing sampled-metrics cardinality test): sends a gauge, count and set with cardinality="high" under aggregation and asserts all three packets carry |card:high. It fails on master (the set packet is set:value|s) and passes with the fix. The full tests/unit/dogstatsd/test_statsd.py suite stays green (126 passed, 1 skipped).

---

Disclosure: this change was prepared with AI assistance and reviewed/verified by me before submission.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b29c828199

ℹ️ 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".

[chatgpt-codex-connector]

Separate set values by cardinality before re-emitting

When two set() calls use the same metric name and tags but different per-call cardinalities during one aggregation window, Aggregator.get_context() places them in the same SetMetric; this line then assigns the first call's self.cardinality to every collected value. For example, values submitted with cardinality="low" and then cardinality="high" are both emitted as card:low, incorrectly controlling origin-tag enrichment for the second value. Include cardinality in the aggregation context or retain it per value rather than applying one cardinality to the entire set.

Useful? React with 👍 / 👎.

[jaideeppyne]

Good catch, thanks. Confirmed: Aggregator.get_context() keyed contexts on name+tags only, so two set() calls with the same name+tags but different per-call cardinalities in one aggregation window landed in the same SetMetric, and every collected value was re-emitted with whichever cardinality created the context first (low then high → both low).

Fixed in 934e484 by including the resolved cardinality in the aggregation context key, so distinct cardinalities form distinct metric objects and each flushes with its own cardinality. The key is unchanged when no cardinality is set, so existing contexts are preserved. Since the fix lives in the shared add_metric path, it also corrects the same latent collision for count/gauge. (The sampled path — histogram/distribution/timing — has the analogous keying but is out of scope for this PR.)

Added regression tests in test_aggregator.py: same name+tags with low then high now yields two SetMetrics and both cardinalities appear on the flushed lines (both fail without the fix, pass with it).

[jaideeppyne]

Thanks for the review. This is a pre-existing property of client-side aggregation rather than something this change introduces, so I've kept the fix scoped to the reported bug.

The aggregation context is keyed on name + tags only (Aggregator.get_context → "{name}:{tags}"), and add_metric applies the first call's cardinality to an existing context for every metric type — CountMetric.aggregate/GaugeMetric.aggregate likewise never update cardinality. So a second gauge(...)/increment(...) with a different cardinality but the same name+tags already keeps the first cardinality today; set behaves identically after this PR. In other words, this change makes set consistent with count/gauge (the intent of #929); it doesn't create a set-specific discrepancy.

Separating aggregated metrics by cardinality would mean adding cardinality to the aggregation context key in get_context, which affects all metric types and is a broader behavioral decision — happy to open a separate issue/PR for that if you'd like it, but it seems out of scope for restoring the dropped card: tag on sets.