From 428323f4e7aacba129a6353a18a70d3e002f197f Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 18 Dec 2024 12:20:20 +0100 Subject: [PATCH 1/3] Improves ServiceNameCollector --- .../remoteconfig/ServiceNameCollector.java | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java b/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java index abc036f3ddb..cef9e3a28d5 100644 --- a/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java @@ -4,8 +4,10 @@ import datadog.trace.api.Config; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Locale; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import javax.annotation.Nullable; import org.slf4j.Logger; @@ -17,6 +19,9 @@ public class ServiceNameCollector { private static final int MAX_EXTRA_SERVICE = Config.get().getRemoteConfigMaxExtraServices(); + private static final String SERVICE_NAME_LOWERCASE = + Config.get().getServiceName().toLowerCase(Locale.ROOT); + // This is not final to allow mocking it on tests private static ServiceNameCollector INSTANCE = new ServiceNameCollector(); @@ -48,14 +53,30 @@ public void addService(final String serviceName) { } return; } - if (!Config.get().getServiceName().equalsIgnoreCase(serviceName)) { - services.put(serviceName.toLowerCase(Locale.ROOT), serviceName); - } + services.put(serviceName, serviceName); } + /** + * Get the list of unique services deduplicated by case. There is no locking on the addService map + * so, the method is not thread safe. + * + * @return + */ @Nullable public List getServices() { - return services.isEmpty() ? null : new ArrayList<>(services.values()); + if (services.isEmpty()) { + return null; + } + final Set uniques = new HashSet<>(services.size()); + // avoids reiterating again to convert a set to a list + final ArrayList ret = new ArrayList<>(services.size()); + for (String current : services.keySet()) { + String lowerCase = current.toLowerCase(Locale.ROOT); + if (!SERVICE_NAME_LOWERCASE.equals(lowerCase) && uniques.add(lowerCase)) { + ret.add(current); + } + } + return ret.isEmpty() ? null : ret; } public void clear() { From ab684c1b871c2ffb941d9f4119045a734348be25 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 18 Dec 2024 12:33:29 +0100 Subject: [PATCH 2/3] Update internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java Co-authored-by: Stuart McCulloch --- .../datadog/trace/api/remoteconfig/ServiceNameCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java b/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java index cef9e3a28d5..d38a324d643 100644 --- a/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java @@ -53,7 +53,7 @@ public void addService(final String serviceName) { } return; } - services.put(serviceName, serviceName); + services.putIfAbsent(serviceName, serviceName); } /** From da89e601637028cf430fe2d38aa582c79d4f1f3a Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 18 Dec 2024 17:52:24 +0100 Subject: [PATCH 3/3] apply suggestions --- .../remoteconfig/ServiceNameCollector.java | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java b/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java index d38a324d643..c4f78b22c58 100644 --- a/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/remoteconfig/ServiceNameCollector.java @@ -4,10 +4,9 @@ import datadog.trace.api.Config; import java.util.ArrayList; -import java.util.HashSet; import java.util.List; -import java.util.Locale; import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import javax.annotation.Nullable; import org.slf4j.Logger; @@ -19,9 +18,6 @@ public class ServiceNameCollector { private static final int MAX_EXTRA_SERVICE = Config.get().getRemoteConfigMaxExtraServices(); - private static final String SERVICE_NAME_LOWERCASE = - Config.get().getServiceName().toLowerCase(Locale.ROOT); - // This is not final to allow mocking it on tests private static ServiceNameCollector INSTANCE = new ServiceNameCollector(); @@ -67,16 +63,10 @@ public List getServices() { if (services.isEmpty()) { return null; } - final Set uniques = new HashSet<>(services.size()); - // avoids reiterating again to convert a set to a list - final ArrayList ret = new ArrayList<>(services.size()); - for (String current : services.keySet()) { - String lowerCase = current.toLowerCase(Locale.ROOT); - if (!SERVICE_NAME_LOWERCASE.equals(lowerCase) && uniques.add(lowerCase)) { - ret.add(current); - } - } - return ret.isEmpty() ? null : ret; + final Set uniqueNames = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + uniqueNames.addAll(services.keySet()); + uniqueNames.remove(Config.get().getServiceName()); + return uniqueNames.isEmpty() ? null : new ArrayList<>(uniqueNames); } public void clear() {