From 4b335f30928d65b6892941955102d3b1d0a0f3f8 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Thu, 17 Jul 2025 10:43:57 +0530 Subject: [PATCH 1/6] Handle null dimension values in ServiceMetricEvent.copy --- .../emitter/service/ServiceMetricEvent.java | 21 +++++++++++++++++-- .../service/ServiceMetricEventTest.java | 17 +++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java b/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java index f7f3549ecf7d..d62596f63239 100644 --- a/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java +++ b/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java @@ -33,6 +33,7 @@ import java.util.Arrays; import java.util.Map; import java.util.TreeMap; +import java.util.stream.Collectors; /** * @@ -92,7 +93,7 @@ public String getHost() public Map getUserDims() { - return ImmutableMap.copyOf(userDims); + return nonNullCopyUserDims(); } public String getMetric() @@ -130,7 +131,23 @@ public EventMap toMap() */ public ServiceMetricEvent copy() { - return new ServiceMetricEvent(createdTime, serviceDims, Map.copyOf(userDims), feed, metric, value); + return new ServiceMetricEvent(createdTime, serviceDims, nonNullCopyUserDims(), feed, metric, value); + } + + /** + * Creates a non-null copy of {@link #userDims} containing only non-null keys + * and values. + */ + private Map nonNullCopyUserDims() + { + if (userDims == null) { + return Map.of(); + } + + return userDims.entrySet() + .stream() + .filter(entry -> entry.getKey() != null && entry.getValue() != null) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); } /** diff --git a/processing/src/test/java/org/apache/druid/java/util/emitter/service/ServiceMetricEventTest.java b/processing/src/test/java/org/apache/druid/java/util/emitter/service/ServiceMetricEventTest.java index cf59f0f3c40e..2b0962b4427c 100644 --- a/processing/src/test/java/org/apache/druid/java/util/emitter/service/ServiceMetricEventTest.java +++ b/processing/src/test/java/org/apache/druid/java/util/emitter/service/ServiceMetricEventTest.java @@ -344,4 +344,21 @@ public void test_copy_returnsAnImmutableInstance() // But the event copy still has the original dimensions Assert.assertEquals(Map.of("dim1", "v1"), event1Copy.getUserDims()); } + + @Test + public void test_copy_doesNotContainNullDimValues() + { + final ServiceMetricEvent.Builder eventBuilder = ServiceMetricEvent + .builder() + .setDimension("dim1", (Object) null) + .setMetric("m1", 100); + + final ServiceMetricEvent event1 = eventBuilder.build("coordinator", "localhost"); + final ServiceMetricEvent event1Copy = event1.copy(); + + Assert.assertTrue(event1.toMap().containsKey("dim1")); + Assert.assertNull(event1.toMap().get("dim1")); + Assert.assertEquals(Map.of(), event1.getUserDims()); + Assert.assertEquals(Map.of(), event1Copy.getUserDims()); + } } From 4845d7dcb37d0a548482de08f363536b80b63abe Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Thu, 17 Jul 2025 13:26:04 +0530 Subject: [PATCH 2/6] Fix test --- .../org/apache/druid/emitter/influxdb/InfluxdbEmitterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions-contrib/influxdb-emitter/src/test/java/org/apache/druid/emitter/influxdb/InfluxdbEmitterTest.java b/extensions-contrib/influxdb-emitter/src/test/java/org/apache/druid/emitter/influxdb/InfluxdbEmitterTest.java index cc66cc2c5d9e..5240899b555c 100644 --- a/extensions-contrib/influxdb-emitter/src/test/java/org/apache/druid/emitter/influxdb/InfluxdbEmitterTest.java +++ b/extensions-contrib/influxdb-emitter/src/test/java/org/apache/druid/emitter/influxdb/InfluxdbEmitterTest.java @@ -216,7 +216,7 @@ public void testMetricIsInDefaultDimensionWhitelist() null ); InfluxdbEmitter influxdbEmitter = new InfluxdbEmitter(config); - String expected = "druid_metric,service=druid/historical,hostname=localhost,dataSource=wikipedia,taskType=index druid_time=1234 1509357600000000000" + String expected = "druid_metric,service=druid/historical,hostname=localhost,taskType=index,dataSource=wikipedia druid_time=1234 1509357600000000000" + "\n"; String actual = influxdbEmitter.transformForInfluxSystems(event); Assert.assertEquals(expected, actual); From e6f1f6cf93cc9e3b739b699447a68cfe1e44f496 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Thu, 17 Jul 2025 18:08:20 +0530 Subject: [PATCH 3/6] Do not allow null values, make event immutable --- .../emitter/service/ServiceMetricEvent.java | 58 ++++++------------- .../service/ServiceMetricEventTest.java | 36 ++++++------ .../java/util/metrics/StubServiceEmitter.java | 2 +- 3 files changed, 37 insertions(+), 59 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java b/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java index d62596f63239..814d2a2333b9 100644 --- a/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java +++ b/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java @@ -22,9 +22,9 @@ import com.fasterxml.jackson.annotation.JsonValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Maps; import org.apache.druid.guice.annotations.PublicApi; import org.apache.druid.java.util.common.DateTimes; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.emitter.core.Event; import org.apache.druid.java.util.emitter.core.EventMap; @@ -33,10 +33,9 @@ import java.util.Arrays; import java.util.Map; import java.util.TreeMap; -import java.util.stream.Collectors; /** - * + * Immutable metric event emitted by a Druid {@link ServiceEmitter}. */ @PublicApi public class ServiceMetricEvent implements Event @@ -53,6 +52,9 @@ public static Builder builder() private final String metric; private final Number value; + /** + * Creates an immutable metric event. + */ private ServiceMetricEvent( DateTime createdTime, ImmutableMap serviceDims, @@ -64,7 +66,7 @@ private ServiceMetricEvent( { this.createdTime = createdTime != null ? createdTime : DateTimes.nowUtc(); this.serviceDims = serviceDims; - this.userDims = userDims; + this.userDims = userDims == null ? Map.of() : Map.copyOf(userDims); this.feed = feed; this.metric = metric; this.value = value; @@ -93,7 +95,7 @@ public String getHost() public Map getUserDims() { - return nonNullCopyUserDims(); + return userDims; } public String getMetric() @@ -117,39 +119,10 @@ public EventMap toMap() .putAll(serviceDims) .put("metric", metric) .put("value", value) - .putAll( - Maps.filterEntries( - userDims, - input -> input.getKey() != null - ) - ) + .putAll(userDims) .build(); } - /** - * Creates an immutable copy of this metric event. This is used only in tests. - */ - public ServiceMetricEvent copy() - { - return new ServiceMetricEvent(createdTime, serviceDims, nonNullCopyUserDims(), feed, metric, value); - } - - /** - * Creates a non-null copy of {@link #userDims} containing only non-null keys - * and values. - */ - private Map nonNullCopyUserDims() - { - if (userDims == null) { - return Map.of(); - } - - return userDims.entrySet() - .stream() - .filter(entry -> entry.getKey() != null && entry.getValue() != null) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - } - /** * Builder for a {@link ServiceMetricEvent}. This builder can be used for * building only one event. @@ -170,22 +143,29 @@ public Builder setFeed(String feed) public Builder setDimension(String dim, String[] values) { + if (dim == null) { + throw new IAE("Dimension name cannot be null"); + } + userDims.put(dim, Arrays.asList(values)); return this; } public Builder setDimensionIfNotNull(String dim, Object value) { - if (value != null) { - userDims.put(dim, value); + if (dim == null) { + throw new IAE("Dimension name cannot be null"); + } else if (value == null) { + throw new IAE("Value of dimension[%s] cannot be null", dim); } + + userDims.put(dim, value); return this; } public Builder setDimension(String dim, Object value) { - userDims.put(dim, value); - return this; + return setDimensionIfNotNull(dim, value); } public Object getDimension(String dim) diff --git a/processing/src/test/java/org/apache/druid/java/util/emitter/service/ServiceMetricEventTest.java b/processing/src/test/java/org/apache/druid/java/util/emitter/service/ServiceMetricEventTest.java index 2b0962b4427c..2b9428708020 100644 --- a/processing/src/test/java/org/apache/druid/java/util/emitter/service/ServiceMetricEventTest.java +++ b/processing/src/test/java/org/apache/druid/java/util/emitter/service/ServiceMetricEventTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableMap; import org.apache.druid.java.util.common.DateTimes; +import org.apache.druid.java.util.common.IAE; import org.junit.Assert; import org.junit.Test; @@ -319,7 +320,7 @@ public void testSetDimensionIfNotNullShouldNotSetNullDimension() } @Test - public void test_copy_returnsAnImmutableInstance() + public void test_builder_createsImmutableEvents() { final ServiceMetricEvent.Builder eventBuilder = ServiceMetricEvent .builder() @@ -327,10 +328,8 @@ public void test_copy_returnsAnImmutableInstance() .setMetric("m1", 100); final ServiceMetricEvent event1 = eventBuilder.build("coordinator", "localhost"); - final ServiceMetricEvent event1Copy = event1.copy(); Assert.assertEquals(Map.of("dim1", "v1"), event1.getUserDims()); - Assert.assertEquals(Map.of("dim1", "v1"), event1Copy.getUserDims()); final ServiceMetricEvent event2 = eventBuilder .setDimension("dim2", "v2") @@ -339,26 +338,25 @@ public void test_copy_returnsAnImmutableInstance() // Verify that the original event gets changed dimensions Assert.assertEquals(Map.of("dim1", "v1", "dim2", "v2"), event2.getUserDims()); - Assert.assertEquals(Map.of("dim1", "v1", "dim2", "v2"), event1.getUserDims()); - - // But the event copy still has the original dimensions - Assert.assertEquals(Map.of("dim1", "v1"), event1Copy.getUserDims()); + Assert.assertEquals(Map.of("dim1", "v1"), event1.getUserDims()); } @Test - public void test_copy_doesNotContainNullDimValues() + public void test_builder_throwsException_ifDimNameOrValueIsNull() { - final ServiceMetricEvent.Builder eventBuilder = ServiceMetricEvent - .builder() - .setDimension("dim1", (Object) null) - .setMetric("m1", 100); + final ServiceMetricEvent.Builder eventBuilder = ServiceMetricEvent.builder(); - final ServiceMetricEvent event1 = eventBuilder.build("coordinator", "localhost"); - final ServiceMetricEvent event1Copy = event1.copy(); - - Assert.assertTrue(event1.toMap().containsKey("dim1")); - Assert.assertNull(event1.toMap().get("dim1")); - Assert.assertEquals(Map.of(), event1.getUserDims()); - Assert.assertEquals(Map.of(), event1Copy.getUserDims()); + Assert.assertThrows( + IAE.class, + () -> eventBuilder.setDimension("dim1", (Object) null) + ); + Assert.assertThrows( + IAE.class, + () -> eventBuilder.setDimension(null, null) + ); + Assert.assertThrows( + IAE.class, + () -> eventBuilder.setDimension(null, new String[]{"a"}) + ); } } diff --git a/processing/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java b/processing/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java index b330573c3621..fac548afb468 100644 --- a/processing/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java +++ b/processing/src/test/java/org/apache/druid/java/util/metrics/StubServiceEmitter.java @@ -59,7 +59,7 @@ public void emit(Event event) if (event instanceof ServiceMetricEvent) { ServiceMetricEvent metricEvent = (ServiceMetricEvent) event; metricEvents.computeIfAbsent(metricEvent.getMetric(), name -> new ConcurrentLinkedDeque<>()) - .add(metricEvent.copy()); + .add(metricEvent); } else if (event instanceof AlertEvent) { alertEvents.add((AlertEvent) event); } From 7c2ef004c3b23ab8590096579482d1522731fc79 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Thu, 17 Jul 2025 19:24:13 +0530 Subject: [PATCH 4/6] Do not throw exception in addDimensionIfNotNull --- .../emitter/service/ServiceMetricEvent.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java b/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java index 814d2a2333b9..d2a53a3c90f9 100644 --- a/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java +++ b/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java @@ -151,20 +151,34 @@ public Builder setDimension(String dim, String[] values) return this; } + /** + * Adds a dimension to be emitted with this metric event, only if the given + * value is not null. + * + * @throws IAE if the dimension name is null. + */ public Builder setDimensionIfNotNull(String dim, Object value) { if (dim == null) { throw new IAE("Dimension name cannot be null"); - } else if (value == null) { - throw new IAE("Value of dimension[%s] cannot be null", dim); } - userDims.put(dim, value); + if (value != null) { + userDims.put(dim, value); + } return this; } + /** + * Adds a dimension to be emitted with this metric event. + * + * @throws IAE if the dimension name or the given value is null. + */ public Builder setDimension(String dim, Object value) { + if (value == null) { + throw new IAE("Value of dimension[%s] cannot be null", dim); + } return setDimensionIfNotNull(dim, value); } From 713ee5a966643ec2af07a54216c2f5c468cd3278 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Thu, 17 Jul 2025 22:06:26 +0530 Subject: [PATCH 5/6] Fix up tests --- .../emitter/influxdb/InfluxdbEmitter.java | 4 +++- .../emitter/influxdb/InfluxdbEmitterTest.java | 2 +- .../apache/druid/indexer/HadoopTaskTest.java | 2 +- .../common/task/IndexTaskUtilsTest.java | 18 ++++++------------ .../SeekableStreamIndexTaskRunnerAuthTest.java | 2 +- .../druid/query/DefaultQueryMetricsTest.java | 2 +- 6 files changed, 13 insertions(+), 17 deletions(-) diff --git a/extensions-contrib/influxdb-emitter/src/main/java/org/apache/druid/emitter/influxdb/InfluxdbEmitter.java b/extensions-contrib/influxdb-emitter/src/main/java/org/apache/druid/emitter/influxdb/InfluxdbEmitter.java index bb1c72b03141..a6b28811e610 100644 --- a/extensions-contrib/influxdb-emitter/src/main/java/org/apache/druid/emitter/influxdb/InfluxdbEmitter.java +++ b/extensions-contrib/influxdb-emitter/src/main/java/org/apache/druid/emitter/influxdb/InfluxdbEmitter.java @@ -41,6 +41,8 @@ import java.io.IOException; import java.security.KeyStore; import java.util.Arrays; +import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -150,7 +152,7 @@ public String transformForInfluxSystems(ServiceMetricEvent event) String metricTag = parts.length == 2 ? "" : ",metric=druid_" + metric; tag.append(metricTag); tag.append(StringUtils.format(",hostname=%s", getValue("host", event).split(":")[0])); - ImmutableSet dimNames = ImmutableSet.copyOf(event.getUserDims().keySet()); + Set dimNames = new TreeSet<>(event.getUserDims().keySet()); for (String dimName : dimNames) { if (this.dimensionWhiteList.contains(dimName)) { tag.append(StringUtils.format(",%1$s=%2$s", dimName, sanitize(String.valueOf(event.getUserDims().get(dimName))))); diff --git a/extensions-contrib/influxdb-emitter/src/test/java/org/apache/druid/emitter/influxdb/InfluxdbEmitterTest.java b/extensions-contrib/influxdb-emitter/src/test/java/org/apache/druid/emitter/influxdb/InfluxdbEmitterTest.java index 5240899b555c..cc66cc2c5d9e 100644 --- a/extensions-contrib/influxdb-emitter/src/test/java/org/apache/druid/emitter/influxdb/InfluxdbEmitterTest.java +++ b/extensions-contrib/influxdb-emitter/src/test/java/org/apache/druid/emitter/influxdb/InfluxdbEmitterTest.java @@ -216,7 +216,7 @@ public void testMetricIsInDefaultDimensionWhitelist() null ); InfluxdbEmitter influxdbEmitter = new InfluxdbEmitter(config); - String expected = "druid_metric,service=druid/historical,hostname=localhost,taskType=index,dataSource=wikipedia druid_time=1234 1509357600000000000" + String expected = "druid_metric,service=druid/historical,hostname=localhost,dataSource=wikipedia,taskType=index druid_time=1234 1509357600000000000" + "\n"; String actual = influxdbEmitter.transformForInfluxSystems(event); Assert.assertEquals(expected, actual); diff --git a/indexing-hadoop/src/test/java/org/apache/druid/indexer/HadoopTaskTest.java b/indexing-hadoop/src/test/java/org/apache/druid/indexer/HadoopTaskTest.java index 02679387e88d..c51c8fd7e4f8 100644 --- a/indexing-hadoop/src/test/java/org/apache/druid/indexer/HadoopTaskTest.java +++ b/indexing-hadoop/src/test/java/org/apache/druid/indexer/HadoopTaskTest.java @@ -58,7 +58,7 @@ public void testBuildClassLoader() throws Exception @Override public String getType() { - return null; + return "hadoop-test"; } @Override diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskUtilsTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskUtilsTest.java index 025b3ee09ea9..0e5839c87d80 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskUtilsTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskUtilsTest.java @@ -38,7 +38,6 @@ public class IndexTaskUtilsTest private static final Map METRIC_TAGS = ImmutableMap.of("k1", "v1", "k2", 20); private static final String GROUP_ID = "groupId123"; - @Mock private Task task; @Mock private AbstractTask abstractTask; @@ -48,10 +47,13 @@ public class IndexTaskUtilsTest public void setUp() { metricBuilder = ServiceMetricEvent.builder(); - Mockito.when(task.getContextValue(DruidMetrics.TAGS)).thenReturn(METRIC_TAGS); - Mockito.when(task.getGroupId()).thenReturn(GROUP_ID); + task = new NoopTask(GROUP_ID, GROUP_ID, "wiki", 1L, 0L, Map.of(DruidMetrics.TAGS, METRIC_TAGS)); Mockito.when(abstractTask.getContextValue(DruidMetrics.TAGS)).thenReturn(METRIC_TAGS); Mockito.when(abstractTask.getGroupId()).thenReturn(GROUP_ID); + Mockito.when(abstractTask.getId()).thenReturn(GROUP_ID); + Mockito.when(abstractTask.getType()).thenReturn("test"); + Mockito.when(abstractTask.getDataSource()).thenReturn("wiki"); + Mockito.when(abstractTask.getIngestionMode()).thenReturn(AbstractTask.IngestionMode.APPEND); } @Test @@ -71,7 +73,7 @@ public void testSetTaskDimensionsForAbstractTaskWithContextTagsShouldSetTags() @Test public void testSetTaskDimensionsWithoutTagsShouldNotSetTags() { - Mockito.when(task.getContextValue(DruidMetrics.TAGS)).thenReturn(null); + task = new NoopTask(null, null, "wiki", 1L, 0L, null); IndexTaskUtils.setTaskDimensions(metricBuilder, task); Assert.assertNull(metricBuilder.getDimension(DruidMetrics.TAGS)); } @@ -91,14 +93,6 @@ public void testSetTaskDimensionsWithGroupIdShouldSetGroupId() Assert.assertEquals(GROUP_ID, metricBuilder.getDimension(DruidMetrics.GROUP_ID)); } - @Test - public void testSetTaskDimensionsWithoutGroupIdShouldNotSetGroupId() - { - Mockito.when(task.getGroupId()).thenReturn(null); - IndexTaskUtils.setTaskDimensions(metricBuilder, task); - Assert.assertNull(metricBuilder.getDimension(DruidMetrics.GROUP_ID)); - } - @Test public void testSetTaskDimensionsForAbstractTaskWithGroupIdShouldSetGroupId() { diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunnerAuthTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunnerAuthTest.java index 92c828c69850..ec71cec89c18 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunnerAuthTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunnerAuthTest.java @@ -381,7 +381,7 @@ public TestSeekableStreamIndexTask( @Override public String getType() { - return null; + return "test"; } @Override diff --git a/processing/src/test/java/org/apache/druid/query/DefaultQueryMetricsTest.java b/processing/src/test/java/org/apache/druid/query/DefaultQueryMetricsTest.java index f7cd6887e7e0..ee82f206f79c 100644 --- a/processing/src/test/java/org/apache/druid/query/DefaultQueryMetricsTest.java +++ b/processing/src/test/java/org/apache/druid/query/DefaultQueryMetricsTest.java @@ -66,11 +66,11 @@ public void testDefaultQueryMetricsQuery() .context(ImmutableMap.of("testKey", "testValue")) .build(); queryMetrics.query(query); - queryMetrics.reportQueryTime(0).emit(serviceEmitter); // No way to verify this right now since DefaultQueryMetrics implements a no-op for sqlQueryId(String) and queryId(String) // This change is done to keep the code coverage tool happy by exercising the implementation queryMetrics.sqlQueryId("dummy"); queryMetrics.queryId("dummy"); + queryMetrics.reportQueryTime(0).emit(serviceEmitter); Map actualEvent = serviceEmitter.getEvents().get(0).toMap(); Assert.assertEquals(13, actualEvent.size()); Assert.assertTrue(actualEvent.containsKey("feed")); From 4717411fab9c97e2a9cbc69c33648861f115dfa3 Mon Sep 17 00:00:00 2001 From: Kashif Faraz Date: Fri, 18 Jul 2025 08:10:16 +0530 Subject: [PATCH 6/6] Clean up ServiceMetricEvent methods --- .../emitter/influxdb/InfluxdbEmitter.java | 4 +-- .../emitter/service/ServiceMetricEvent.java | 25 ++++++++----------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/extensions-contrib/influxdb-emitter/src/main/java/org/apache/druid/emitter/influxdb/InfluxdbEmitter.java b/extensions-contrib/influxdb-emitter/src/main/java/org/apache/druid/emitter/influxdb/InfluxdbEmitter.java index a6b28811e610..bb1c72b03141 100644 --- a/extensions-contrib/influxdb-emitter/src/main/java/org/apache/druid/emitter/influxdb/InfluxdbEmitter.java +++ b/extensions-contrib/influxdb-emitter/src/main/java/org/apache/druid/emitter/influxdb/InfluxdbEmitter.java @@ -41,8 +41,6 @@ import java.io.IOException; import java.security.KeyStore; import java.util.Arrays; -import java.util.Set; -import java.util.TreeSet; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -152,7 +150,7 @@ public String transformForInfluxSystems(ServiceMetricEvent event) String metricTag = parts.length == 2 ? "" : ",metric=druid_" + metric; tag.append(metricTag); tag.append(StringUtils.format(",hostname=%s", getValue("host", event).split(":")[0])); - Set dimNames = new TreeSet<>(event.getUserDims().keySet()); + ImmutableSet dimNames = ImmutableSet.copyOf(event.getUserDims().keySet()); for (String dimName : dimNames) { if (this.dimensionWhiteList.contains(dimName)) { tag.append(StringUtils.format(",%1$s=%2$s", dimName, sanitize(String.valueOf(event.getUserDims().get(dimName))))); diff --git a/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java b/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java index d2a53a3c90f9..6644886497df 100644 --- a/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java +++ b/processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java @@ -47,7 +47,7 @@ public static Builder builder() private final DateTime createdTime; private final ImmutableMap serviceDims; - private final Map userDims; + private final ImmutableMap userDims; private final String feed; private final String metric; private final Number value; @@ -58,7 +58,7 @@ public static Builder builder() private ServiceMetricEvent( DateTime createdTime, ImmutableMap serviceDims, - Map userDims, + ImmutableMap userDims, String feed, String metric, Number value @@ -66,7 +66,7 @@ private ServiceMetricEvent( { this.createdTime = createdTime != null ? createdTime : DateTimes.nowUtc(); this.serviceDims = serviceDims; - this.userDims = userDims == null ? Map.of() : Map.copyOf(userDims); + this.userDims = userDims; this.feed = feed; this.metric = metric; this.value = value; @@ -159,14 +159,7 @@ public Builder setDimension(String dim, String[] values) */ public Builder setDimensionIfNotNull(String dim, Object value) { - if (dim == null) { - throw new IAE("Dimension name cannot be null"); - } - - if (value != null) { - userDims.put(dim, value); - } - return this; + return value == null ? this : setDimension(dim, value); } /** @@ -176,10 +169,14 @@ public Builder setDimensionIfNotNull(String dim, Object value) */ public Builder setDimension(String dim, Object value) { - if (value == null) { + if (dim == null) { + throw new IAE("Dimension name cannot be null"); + } else if (value == null) { throw new IAE("Value of dimension[%s] cannot be null", dim); } - return setDimensionIfNotNull(dim, value); + + userDims.put(dim, value); + return this; } public Object getDimension(String dim) @@ -216,7 +213,7 @@ public ServiceMetricEvent build(ImmutableMap serviceDimensions) return new ServiceMetricEvent( createdTime, serviceDimensions, - userDims, + ImmutableMap.copyOf(userDims), feed, metric, value