From d19b014cdd37ec6bff22dce24fc963fb878ed278 Mon Sep 17 00:00:00 2001 From: Csaba Kos Date: Tue, 11 Feb 2020 11:01:18 -0600 Subject: [PATCH] Preserve case of local and remote service names (#1080) --- brave/src/main/java/brave/Tracing.java | 9 ++++----- .../src/main/java/brave/handler/MutableSpan.java | 7 +++---- brave/src/test/java/brave/TracingTest.java | 8 +++++++- .../test/java/brave/handler/MutableSpanTest.java | 16 +++++++++++++++- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/brave/src/main/java/brave/Tracing.java b/brave/src/main/java/brave/Tracing.java index 8e3943016e..570bb2c8c1 100644 --- 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 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". * *

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 index 1b41771803..b1bb3d3c65 100644 --- 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 index 1f602d6ec6..ec7bf20922 100644 --- 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 index b794cc1cb5..8fa4367814 100644 --- 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. *