Fix possible IllegalStateException when using Micrometer (#531)

Author: wapkch

CHANGES.md

@@ -16,6 +16,7 @@ Release Notes.
 * Fix Jedis-2.x plugin can not get host info in jedis 3.3.x+
 * Change the classloader to locate the agent path in AgentPackagePath, from `SystemClassLoader` to AgentPackagePath's loader.
 * Support Grizzly Trace
+* Fix possible IllegalStateException when using Micrometer.
 
 #### Documentation
 

apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/MicrometerReceiverTracingHandlerInterceptor.java

@@ -60,7 +60,8 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
             SpanLayer spanLayer = TaggingHelper.toLayer(context.getAllKeyValues());
             AbstractSpan abstractSpan = ContextManager.activeSpan();
             abstractSpan
-                .setPeer(SpanHelper.tryToGetPeer(context.getRemoteServiceAddress(), context.getAllKeyValues()))
+                .setPeer(SpanHelper.tryToGetPeer(context.getRemoteServiceAddress(), context.getRemoteServiceName(),
+                    context.getAllKeyValues()))
                 .setOperationName(getOperationName(context));
             context.getAllKeyValues()
                    .forEach(keyValue -> abstractSpan.tag(Tags.ofKey(keyValue.getKey()), keyValue.getValue()));

apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/MicrometerSenderTracingHandlerInterceptor.java

@@ -48,7 +48,8 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
             SenderContext<Object> context = (SenderContext<Object>) allArguments[0];
             final ContextCarrier contextCarrier = new ContextCarrier();
             AbstractSpan span = ContextManager.createExitSpan(
-                getOperationName(context), contextCarrier, SpanHelper.tryToGetPeer(context.getRemoteServiceAddress(), context.getAllKeyValues()));
+                getOperationName(context), contextCarrier, SpanHelper.tryToGetPeer(context.getRemoteServiceAddress(),
+                    context.getRemoteServiceName(), context.getAllKeyValues()));
             CarrierItem next = contextCarrier.items();
             while (next.hasNext()) {
                 next = next.next();
@@ -60,7 +61,8 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
             SpanLayer spanLayer = TaggingHelper.toLayer(context.getAllKeyValues());
             AbstractSpan abstractSpan = ContextManager.activeSpan();
             abstractSpan
-                .setPeer(SpanHelper.tryToGetPeer(context.getRemoteServiceAddress(), context.getAllKeyValues()))
+                .setPeer(SpanHelper.tryToGetPeer(context.getRemoteServiceAddress(), context.getRemoteServiceName(),
+                    context.getAllKeyValues()))
                 .setOperationName(getOperationName(context));
             context.getAllKeyValues()
                    .forEach(keyValue -> abstractSpan.tag(Tags.ofKey(keyValue.getKey()), keyValue.getValue()));

apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/SpanHelper.java

@@ -20,11 +20,12 @@
 
 import io.micrometer.common.KeyValue;
 import io.micrometer.common.KeyValues;
+
 import java.net.URI;
 
 class SpanHelper {
 
-    static String tryToGetPeer(String remoteAddress, KeyValues allKeyValues) {
+    static String tryToGetPeer(String remoteAddress, String remoteName, KeyValues allKeyValues) {
         if (remoteAddress != null) {
             return remoteAddress;
         }
@@ -39,12 +40,12 @@ static String tryToGetPeer(String remoteAddress, KeyValues allKeyValues) {
                                .orElse("unknown");
         try {
             URI uri = URI.create(result);
-            if (uri.getHost() == null) {
-                return null;
+            if (uri.getHost() != null) {
+                return uri.getHost() + ":" + uri.getPort();
             }
-            return uri.getHost() + ":" + uri.getPort();
+            return remoteName;
         } catch (Exception ex) {
-            return null;
+            return remoteName;
         }
     }
 }

apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/test/java/org/apache/skywalking/apm/toolkit/activation/micrometer/SpanHelperTest.java

@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.toolkit.activation.micrometer;
+
+import io.micrometer.common.KeyValue;
+import io.micrometer.common.KeyValues;
+
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+public class SpanHelperTest {
+
+    @Test
+    public void testTryToGetPeer() {
+        String remoteName = SpanHelper.tryToGetPeer("http://localhost:8080", "remoteName",
+            KeyValues.of(KeyValue.of("http.url", "http://localhost:8081")));
+        assertThat(remoteName, is("http://localhost:8080"));
+    }
+
+    @Test
+    public void testTryToGetPeerWhenRemoteAddressIsNull() {
+        String remoteName = SpanHelper.tryToGetPeer(null, "remoteName",
+            KeyValues.of(KeyValue.of("http.url", "http://localhost:8080")));
+        assertThat(remoteName, is("localhost:8080"));
+    }
+
+    @Test
+    public void testTryToGetPeerWhenURIWithQueryString() {
+        String remoteName = SpanHelper.tryToGetPeer(null, "remoteName",
+            KeyValues.of(KeyValue.of("http.url", "http://localhost:8080?a=b")));
+        assertThat(remoteName, is("localhost:8080"));
+    }
+
+    @Test
+    public void testTryToGetPeerWhenLackOfURIComponents() {
+        String remoteName = SpanHelper.tryToGetPeer(null, "remoteName",
+            KeyValues.empty());
+        assertThat(remoteName, is("remoteName"));
+    }
+
+    @Test
+    public void testTryToGetPeerWhenURIViolatesRFC2396() {
+        String remoteName = SpanHelper.tryToGetPeer(null, "remoteName",
+            KeyValues.of(KeyValue.of("http.url", "https://www.example.com/path/with spaces")));
+        assertThat(remoteName, is("remoteName"));
+    }
+
+}