Preserve case of local and remote service names (#1080)

openzipkin · Feb 11, 2020 · d19b014 · d19b014
1 parent 3b434dc
commit d19b014
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 11 deletions.
diff --git a/brave/src/main/java/brave/Tracing.java b/brave/src/main/java/brave/Tracing.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2019 The OpenZipkin Authors
+ * Copyright 2013-2020 The OpenZipkin Authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License. You may obtain a copy of the License at
@@ -30,7 +30,6 @@
 import java.io.Closeable;
 import java.util.ArrayList;
 import java.util.LinkedHashSet;
-import java.util.Locale;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
@@ -152,8 +151,8 @@ public static final class Builder {
     Set<FinishedSpanHandler> finishedSpanHandlers = new LinkedHashSet<>(); // dupes not ok
 
     /**
-     * Lower-case label of the remote node in the service graph, such as "favstar". Avoid names with
-     * variables or unique identifiers embedded. Defaults to "unknown".
+     * Label of the remote node in the service graph, such as "favstar". Avoid names with variables
+     * or unique identifiers embedded. Defaults to "unknown".
      *
      * <p>This is a primary label for trace lookup and aggregation, so it should be intuitive and
      * consistent. Many use a name from service discovery.
@@ -164,7 +163,7 @@ public Builder localServiceName(String localServiceName) {
       if (localServiceName == null || localServiceName.isEmpty()) {
         throw new IllegalArgumentException(localServiceName + " is not a valid serviceName");
       }
-      this.localServiceName = localServiceName.toLowerCase(Locale.ROOT);
+      this.localServiceName = localServiceName;
       return this;
     }
 

diff --git a/brave/src/main/java/brave/handler/MutableSpan.java b/brave/src/main/java/brave/handler/MutableSpan.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2019 The OpenZipkin Authors
+ * Copyright 2013-2020 The OpenZipkin Authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License. You may obtain a copy of the License at
@@ -19,7 +19,6 @@
 import brave.internal.Nullable;
 import brave.propagation.TraceContext;
 import java.util.ArrayList;
-import java.util.Locale;
 
 /**
  * This represents a span except for its {@link TraceContext}. It is mutable, for late adjustments.
@@ -148,7 +147,7 @@ public void localServiceName(String localServiceName) {
     if (localServiceName == null || localServiceName.isEmpty()) {
       throw new NullPointerException("localServiceName is empty");
     }
-    this.localServiceName = localServiceName.toLowerCase(Locale.ROOT);
+    this.localServiceName = localServiceName;
   }
 
   /** When null {@link brave.Tracing.Builder#localIp(String) default} will be used for zipkin. */
@@ -184,7 +183,7 @@ public void remoteServiceName(String remoteServiceName) {
     if (remoteServiceName == null || remoteServiceName.isEmpty()) {
       throw new NullPointerException("remoteServiceName is empty");
     }
-    this.remoteServiceName = remoteServiceName.toLowerCase(Locale.ROOT);
+    this.remoteServiceName = remoteServiceName;
   }
 
   /**

diff --git a/brave/src/test/java/brave/TracingTest.java b/brave/src/test/java/brave/TracingTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2019 The OpenZipkin Authors
+ * Copyright 2013-2020 The OpenZipkin Authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License. You may obtain a copy of the License at
@@ -69,6 +69,12 @@ public class TracingTest {
     assertThat(spans).isEmpty();
   }
 
+  @Test public void localServiceNamePreservesCase() {
+    String expectedLocalServiceName = "FavStar";
+    Tracing.Builder builder = Tracing.newBuilder().localServiceName(expectedLocalServiceName);
+    assertThat(builder).extracting("localServiceName").isEqualTo(expectedLocalServiceName);
+  }
+
   @Test public void spanReporter_getsLocalEndpointInfo() {
     String expectedLocalServiceName = "favistar", expectedLocalIp = "1.2.3.4";
     int expectedLocalPort = 80;

diff --git a/brave/src/test/java/brave/handler/MutableSpanTest.java b/brave/src/test/java/brave/handler/MutableSpanTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2019 The OpenZipkin Authors
+ * Copyright 2013-2020 The OpenZipkin Authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License. You may obtain a copy of the License at
@@ -178,6 +178,20 @@ class Tag {
     );
   }
 
+  @Test public void localServiceNamePreservesCase() {
+    String expectedLocalServiceName = "FavStar";
+    MutableSpan span = new MutableSpan();
+    span.localServiceName(expectedLocalServiceName);
+    assertThat(span.localServiceName()).isEqualTo(expectedLocalServiceName);
+  }
+
+  @Test public void remoteServiceNamePreservesCase() {
+    String expectedRemoteServiceName = "FavStar";
+    MutableSpan span = new MutableSpan();
+    span.remoteServiceName(expectedRemoteServiceName);
+    assertThat(span.remoteServiceName()).isEqualTo(expectedRemoteServiceName);
+  }
+
   /**
    * {@link brave.Span#kind(Span.Kind)} is nullable, so setting kind to null should work.
    *