Exit span doesn't include meaningful peer information when using micrometer as skywalking reporter

[wapkch]

Description

I tried to use Micrometer to report OpenFeign traces to Skywalking OAP server. maven dependencies are following:

<dependency>
      <groupId>org.apache.skywalking</groupId>
      <artifactId>apm-toolkit-micrometer-1.10</artifactId>
      <version>8.15.0</version>
   </dependency>

<dependency>
      <groupId>io.github.openfeign</groupId>
      <artifactId>feign-micrometer</artifactId>
      <version>12.1</version>
   </dependency>

<dependency>
      <groupId>org.springframework.cloud</groupId>
      <artifactId>spring-cloud-starter-openfeign</artifactId>
      <version>4.0.2</version>
   </dependency>

Feign Client definition:

@FeignClient(name = "jsonplaceholder", url = "http://jsonplaceholder.typicode.com")
public interface JsonPlaceholderService {

    @GetMapping("/posts")
    List<Post> loadPosts();

}

ObservationRegistryCustomizer:

@Bean
    public ObservationRegistryCustomizer<ObservationRegistry> observationRegistryCustomizer() {
        return registry -> {
            // Here we add a meter handler
            registry.observationConfig()
                .observationHandler(new ObservationHandler.FirstMatchingCompositeObservationHandler(
                    new SkywalkingMeterHandler(new SkywalkingMeterRegistry())
                ));
            // Here we add tracing handlers
            registry.observationConfig()
                .observationHandler(new ObservationHandler.FirstMatchingCompositeObservationHandler(
                    new SkywalkingSenderTracingHandler(), new SkywalkingReceiverTracingHandler(),
                    new SkywalkingDefaultTracingHandler()
                ));
        };
    }

When i start the application and invoke JsonPlaceholderService loadPosts method, the skywalking-api.log has errors:

ERROR 2023-05-10 17:14:34.584 http-nio-9000-exec-1 InstMethodsInter : class[class org.apache.skywalking.apm.toolkit.micrometer.observation.SkywalkingSenderTracingHandler] before method[onStart] intercept failure 
java.lang.IllegalStateException: Exit span doesn't include meaningful peer information.
    at org.apache.skywalking.apm.agent.core.context.TracingContext.inject(TracingContext.java:169)
    at org.apache.skywalking.apm.agent.core.context.TracingContext.inject(TracingContext.java:149)
    at org.apache.skywalking.apm.agent.core.context.ContextManager.createExitSpan(ContextManager.java:134)
    at org.apache.skywalking.apm.toolkit.activation.micrometer.MicrometerSenderTracingHandlerInterceptor.beforeMethod(MicrometerSenderTracingHandlerInterceptor.java:50)
    at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:76)
    at org.apache.skywalking.apm.toolkit.micrometer.observation.SkywalkingSenderTracingHandler.onStart(SkywalkingSenderTracingHandler.java)
    at org.apache.skywalking.apm.toolkit.micrometer.observation.SkywalkingSenderTracingHandler.onStart(SkywalkingSenderTracingHandler.java:25)
    at io.micrometer.observation.ObservationHandler$FirstMatchingCompositeObservationHandler.onStart(ObservationHandler.java:149)
    at io.micrometer.observation.SimpleObservation.notifyOnObservationStarted(SimpleObservation.java:227)
    at io.micrometer.observation.SimpleObservation.start(SimpleObservation.java:165)
    at feign.micrometer.MicrometerObservationCapability.lambda$enrich$1(MicrometerObservationCapability.java:50)
    at feign.SynchronousMethodHandler.executeAndDecode(SynchronousMethodHandler.java:102)
    at feign.SynchronousMethodHandler.invoke(SynchronousMethodHandler.java:72)
    at feign.ReflectiveFeign$FeignInvocationHandler.invoke(ReflectiveFeign.java:98)
    at jdk.proxy2/jdk.proxy2.$Proxy175.loadPosts(Unknown Source)
    at com.example.skywalkingdemo.HomeController.home(HomeController.java:32)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:207)

This is caused by getLowCardinalityKeyValues in DefaultFeignObservationConvention:

public class DefaultFeignObservationConvention implements FeignObservationConvention {
    @Override
  public KeyValues getLowCardinalityKeyValues(FeignContext context) {
    String templatedUrl = context.getCarrier().requestTemplate().methodMetadata().template().url();
    return KeyValues.of(
        FeignObservationDocumentation.HttpClientTags.METHOD
            .withValue(getMethodString(context.getCarrier())),
        FeignObservationDocumentation.HttpClientTags.URI
            .withValue(templatedUrl),
        FeignObservationDocumentation.HttpClientTags.STATUS
            .withValue(getStatusValue(context.getResponse())));
  }
}

The templatedUrl from context.getCarrier().requestTemplate().methodMetadata().template().url() will be /post, which isn't a valid url that skywalking agent can accept.

For now, i can define a CustomFeignObservationConvention and get the templatedUrl using context.getCarrier().requestTemplate().path().

But should we change the way for retrieving templatedUrl in DefaultFeignObservationConvention? @marcingrzejszczak

[marcingrzejszczak]

The templatedUrl from context.getCarrier().requestTemplate().methodMetadata().template().url() will be /post, which isn't a valid url that skywalking agent can accept.

So what is the valid URL? Isn't /post a valid templated URL ?

cc @jonatan-ivanov @shakuzen for example of using Skywalking with Micrometer Observation

[wapkch]

The templatedUrl from context.getCarrier().requestTemplate().methodMetadata().template().url() will be /post, which isn't a valid url that skywalking agent can accept.

So what is the valid URL? Isn't /post a valid templated URL ?

cc @jonatan-ivanov @shakuzen for example of using Skywalking with Micrometer Observation

/post is a valid templated URL, but it lacks of a host, so SpanHelper#tryToGetPeer in skywalking agent will return null:

class SpanHelper {

    static String tryToGetPeer(String remoteAddress, KeyValues allKeyValues) {
        if (remoteAddress != null) {
            return remoteAddress;
        }
        String result = allKeyValues
                               .stream()
                               .filter(keyValue -> "http.url".equalsIgnoreCase(
                                   keyValue.getKey()) || "uri".equalsIgnoreCase(keyValue.getKey())
                                   || keyValue.getKey().contains("uri") || keyValue.getKey().contains("url")
                               )
                               .findFirst()
                               .map(KeyValue::getValue)
                               .orElse("unknown");
        try {
            URI uri = URI.create(result);
            if (uri.getHost() == null) {
                return null;
            }
            return uri.getHost() + ":" + uri.getPort();
        } catch (Exception ex) {
            return null;
        }
    }
}

which result in ContextManager#createExitSpan's last param(remotePeer) being null, and then TracingContext#inject will check the peer is not empty and throw IllegalStateException:

package org.apache.skywalking.apm.agent.core.context;

public class TracingContext implements AbstractTracerContext {
    public void inject(AbstractSpan exitSpan, ContextCarrier carrier) {
        if (!exitSpan.isExit()) {
            throw new IllegalStateException("Inject can be done only in Exit Span");
        }

        ExitTypeSpan spanWithPeer = (ExitTypeSpan) exitSpan;
        String peer = spanWithPeer.getPeer();
        if (StringUtil.isEmpty(peer)) {
            throw new IllegalStateException("Exit span doesn't include meaningful peer information.");
        }
        ...
    }
}

[marcingrzejszczak]

File a PR in skywalking that fixes this, I don't think this is a problem with Feign. Apparently we should try to modify the way we tag this in skywalking

[wapkch]

File a PR in skywalking that fixes this, I don't think this is a problem with Feign. Apparently we should try to modify the way we tag this in skywalking

Thanks, i will try to do this.

[wapkch]

Since URI is inappropriate for peer(https://github.com/apache/skywalking-java/pull/531#discussion_r1197274921). I think remoteServiceName or remoteServiceAddress should be set to FeignContext, so we can use it instead of using templatedUrl as the peer in skywalking.

[marcingrzejszczak]

I don't think I follow. Can you create a draft PR of how you think this should be fixed?