From ceee51ecc446b8936865c4426dafb4221ff2e69f Mon Sep 17 00:00:00 2001 From: Vikentiy Fesunov Date: Wed, 30 Sep 2026 14:35:51 +0200 Subject: [PATCH] AGTMETRICS-599 split tags containing commas With dogstatsd, it was possible to pass multiple tags as a single, comma-separated string. This happened to work by accident with dogstatsd because such tags are simply inserted into the on the-wire payload and later split into separate tags by the agent. Dogstatsd http preserves tags boundaries, so such strings are treated as a single tag, and then commas are stripped when the intake normalizes the tag. This patch adds tags splitting to avoid breaking clients relying on this behavior. Since scanning tags for commas adds overhead, we provide an opt-out. --- .../dogstatsd/http/DirectHttpClient.java | 67 +++++++++++++++-- .../dogstatsd/http/DirectHttpClientTest.java | 75 +++++++++++++++++++ 2 files changed, 136 insertions(+), 6 deletions(-) diff --git a/dogstatsd-http/core/src/main/java/com/datadoghq/dogstatsd/http/DirectHttpClient.java b/dogstatsd-http/core/src/main/java/com/datadoghq/dogstatsd/http/DirectHttpClient.java index 00266a8f..e05b6d76 100644 --- a/dogstatsd-http/core/src/main/java/com/datadoghq/dogstatsd/http/DirectHttpClient.java +++ b/dogstatsd-http/core/src/main/java/com/datadoghq/dogstatsd/http/DirectHttpClient.java @@ -22,6 +22,10 @@ /** * Simple Dogstatsd HTTP client for sending pre-aggregated metrics. * + *

By default, tags containing commas are split into separate tags and empty pieces are dropped, + * like the agent does for tags received over UDP or UDS. Splitting happens before the special tags + * below are handled. It can be disabled with {@link Builder#splitTags}. + * *

Two tags are given special treatment: their value is submitted as a property of the * timeseries. * @@ -45,6 +49,7 @@ public class DirectHttpClient { private final PayloadBuilder sketchesBuilder; private final Sketch sketchBuffer = new Sketch(); private final String prefix; + private final boolean splitTags; private static final int defaultInterval = 10; private static final String hostTagPrefix = "host:"; private static final String hostResourceType = "host"; @@ -69,6 +74,7 @@ private DirectHttpClient(final Builder builder) { } else { prefix = ""; } + splitTags = builder.splitTags; seriesBuilder = new PayloadBuilder( @@ -113,6 +119,7 @@ public static interface Forwarder { public static class Builder { private final Forwarder forwarder; private String prefix; + private boolean splitTags = true; private Builder(final Forwarder forwarder) { this.forwarder = Objects.requireNonNull(forwarder, "forwarder"); @@ -130,6 +137,18 @@ public Builder prefix(final String val) { return this; } + /** + * Sets whether tags containing commas are split into separate tags, as the agent does for + * tags received over UDP or UDS. Enabled by default. + * + * @param val true to split tags on commas, false to send every tag unchanged. + * @return this builder. + */ + public Builder splitTags(final boolean val) { + splitTags = val; + return this; + } + /** * Builds the client. * @@ -203,14 +222,50 @@ private String prefixed(final String name) { } /** - * Applies the tags to the metric, extracting the host tag into the host resource and the - * cardinality tag into the tags cardinality. The cardinality tag itself is kept in the tags. + * Applies the tags to the metric, splitting comma-separated tags if enabled, then extracting + * the host tag into the host resource and the cardinality tag into the tags cardinality. The + * cardinality tag itself is kept in the tags. */ - private static > T withTagsHostAndCardinality( + private > T withTagsHostAndCardinality( final T metric, final List tags) { - return metric.setTags(withoutHostTags(tags)) - .setResources(hostResource(hostTag(tags))) - .setTagsCardinality(cardinality(cardinalityTag(tags))); + final List t = splitTags ? splitOnComma(tags) : tags; + return metric.setTags(withoutHostTags(t)) + .setResources(hostResource(hostTag(t))) + .setTagsCardinality(cardinality(cardinalityTag(t))); + } + + /** + * Returns the tags with every tag containing a comma split into separate tags, dropping empty + * pieces, or the tags themselves if none contains a comma. + */ + static List splitOnComma(final List tags) { + if (tags == null) { + return null; + } + ArrayList split = null; + for (int i = 0; i < tags.size(); i++) { + final String tag = tags.get(i); + int end = tag.indexOf(','); + if (end >= 0) { + if (split == null) { + split = new ArrayList<>(tags.subList(0, i)); + } + int start = 0; + while (end >= 0) { + if (end > start) { + split.add(tag.substring(start, end)); + } + start = end + 1; + end = tag.indexOf(',', start); + } + if (start < tag.length()) { + split.add(tag.substring(start)); + } + } else if (split != null) { + split.add(tag); + } + } + return split == null ? tags : split; } /** Returns the value of the first host tag, or null if there is none. */ diff --git a/dogstatsd-http/core/src/test/java/com/datadoghq/dogstatsd/http/DirectHttpClientTest.java b/dogstatsd-http/core/src/test/java/com/datadoghq/dogstatsd/http/DirectHttpClientTest.java index 1cc699e7..38b02b35 100644 --- a/dogstatsd-http/core/src/test/java/com/datadoghq/dogstatsd/http/DirectHttpClientTest.java +++ b/dogstatsd-http/core/src/test/java/com/datadoghq/dogstatsd/http/DirectHttpClientTest.java @@ -203,6 +203,81 @@ public void withoutHostTagsRemovesEveryHostTag() { assertSame(card, DirectHttpClient.withoutHostTags(card)); } + @Test + public void splitOnCommaSplitsTags() { + assertNull(DirectHttpClient.splitOnComma(null)); + + List noComma = Arrays.asList("a:b", "", "c:d"); + assertSame(noComma, DirectHttpClient.splitOnComma(noComma)); + + assertEquals( + Arrays.asList("a:b", "c:d", "e:f"), + DirectHttpClient.splitOnComma(Arrays.asList("a:b,c:d", "e:f"))); + assertEquals( + Arrays.asList("x", "", "a", "b", "y", "z"), + DirectHttpClient.splitOnComma(Arrays.asList("x", "", "a,,b,", ",", ",y", "z"))); + } + + @Test + public void commaSeparatedTagsAreSplit() { + TestForwarder fwd = new TestForwarder(); + DirectHttpClient client = DirectHttpClient.builder(fwd).build(); + client.gauge("metric", 1.5, 100, Collections.singletonList("a:b,c:d")); + client.flush(); + + ArrayList expected = new ArrayList<>(); + PayloadBuilder b = builderInto(expected); + b.gauge("metric") + .setTags(Arrays.asList("a:b", "c:d")) + .setInterval(10) + .addPoint(100, 1.5) + .close(); + b.close(); + + assertSent(expected, fwd, seriesUri, "gauge with comma-separated tags"); + } + + @Test + public void hostAndCardinalityInsideCommaSeparatedTag() { + TestForwarder fwd = new TestForwarder(); + DirectHttpClient client = DirectHttpClient.builder(fwd).build(); + client.count( + "metric", 20, 100, Collections.singletonList("a:b,host:h1,dd.internal.card:low")); + client.flush(); + + ArrayList expected = new ArrayList<>(); + PayloadBuilder b = builderInto(expected); + b.rate("metric") + .setTags(Arrays.asList("a:b", "dd.internal.card:low")) + .setResources(Arrays.asList("host", "h1")) + .setTagsCardinality(TagsCardinality.LOW) + .setInterval(10) + .addPoint(100, 2) + .close(); + b.close(); + + assertSent(expected, fwd, seriesUri, "count with host and cardinality in a joined tag"); + } + + @Test + public void splitTagsCanBeDisabled() { + TestForwarder fwd = new TestForwarder(); + DirectHttpClient client = DirectHttpClient.builder(fwd).splitTags(false).build(); + client.gauge("metric", 1.5, 100, Collections.singletonList("a:b,host:h1")); + client.flush(); + + ArrayList expected = new ArrayList<>(); + PayloadBuilder b = builderInto(expected); + b.gauge("metric") + .setTags(Collections.singletonList("a:b,host:h1")) + .setInterval(10) + .addPoint(100, 1.5) + .close(); + b.close(); + + assertSent(expected, fwd, seriesUri, "gauge with splitting disabled"); + } + @Test public void hostResourceIsATypeNamePair() { assertNull(DirectHttpClient.hostResource(null));