diff --git a/CHANGES.md b/CHANGES.md index 379f2ac32..9392da0b2 100644 --- a/CHANGES.md +++ b/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 diff --git a/apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/MicrometerReceiverTracingHandlerInterceptor.java b/apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/MicrometerReceiverTracingHandlerInterceptor.java index 4f9ebafeb..d5347db3c 100644 --- a/apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/MicrometerReceiverTracingHandlerInterceptor.java +++ b/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 class MicrometerReceiverTracingHandlerInterceptor implements InstanceMeth 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())); diff --git a/apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/MicrometerSenderTracingHandlerInterceptor.java b/apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/MicrometerSenderTracingHandlerInterceptor.java index 1aeee1eee..47cf4dc2e 100644 --- a/apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/MicrometerSenderTracingHandlerInterceptor.java +++ b/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 class MicrometerSenderTracingHandlerInterceptor implements InstanceMethod SenderContext context = (SenderContext) 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 class MicrometerSenderTracingHandlerInterceptor implements InstanceMethod 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())); diff --git a/apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/SpanHelper.java b/apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/SpanHelper.java index f98fe3d82..c2a3a47b4 100644 --- a/apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/micrometer/SpanHelper.java +++ b/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 @@ package org.apache.skywalking.apm.toolkit.activation.micrometer; 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 @@ class SpanHelper { .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; } } } diff --git a/apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/test/java/org/apache/skywalking/apm/toolkit/activation/micrometer/SpanHelperTest.java b/apm-sniffer/apm-toolkit-activation/apm-toolkit-micrometer-activation/src/test/java/org/apache/skywalking/apm/toolkit/activation/micrometer/SpanHelperTest.java new file mode 100644 index 000000000..c8ef0c8b6 --- /dev/null +++ b/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")); + } + +}