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

[wapkch]

I tried to use Micrometer to report datasource 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>net.ttddyy.observation</groupId>
      <artifactId>datasource-micrometer-spring-boot</artifactId>
      <version>1.0.0</version>
   </dependency>

<dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-data-jpa</artifactId>
      <version>3.0.6</version>
   </dependency>

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 repository.saveAll(list);, the skywalking-api.log has errors:

ERROR 2023-05-10 21:59:06.892 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 net.ttddyy.observation.tracing.DataSourceObservationListener.createAndStartObservation(DataSourceObservationListener.java:119)
    at net.ttddyy.observation.tracing.DataSourceObservationListener.startQueryObservation(DataSourceObservationListener.java:106)
    at net.ttddyy.observation.tracing.DataSourceObservationListener.beforeQuery(DataSourceObservationListener.java:93)
    at net.ttddyy.dsproxy.listener.ChainListener.beforeQuery(ChainListener.java:20)

This is because the new QueryContext() in DataSourceObservationListener has no remoteServiceAddress and neither keyValues in QueryObservationConvention, which skywalking agent can not accept.

public class MicrometerSenderTracingHandlerInterceptor implements InstanceMethodsAroundInterceptor {

    @Override
    public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
                             MethodInterceptResult result) throws Throwable {
        String methodName = method.getName();
        if ("onStart".equals(methodName)) {
            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()));
            ...
    }
}

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;
        }
    }
}

[ttddyy]

SenderContext#remoteServiceAddress is nullable and datasource-micrometer does not populate this, nor the http.uri or uri tags. On the other hand, SenderContext#remoteServiceName is populated with the datasource name by default. So, instead of remoteServiceAddress, I suggest using the remoteServiceName in your logic. Or, you may add an ObservationFilter that populates remoteServiceAddress.

[wapkch]

So, instead of remoteServiceAddress, I suggest using the remoteServiceName in your logic.

Actually, it's the logic of apm-toolkit-micrometer-activation in skywalking java agent. cc @marcingrzejszczak Is there a possibility that we can just use the remoteServiceName in SpanHelper#tryToGetPeer?

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;
        }
    }
}

Or, you may add an ObservationFilter that populates remoteServiceAddress.

ObservationFilter only modify the Observation.Context on stopping the Observation. But it happens here on starting the Observation

public class MicrometerSenderTracingHandlerInterceptor implements InstanceMethodsAroundInterceptor {

    @Override
    public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
                             MethodInterceptResult result) throws Throwable {
        String methodName = method.getName();
        if ("onStart".equals(methodName)) {
            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()));
            CarrierItem next = contextCarrier.items();
            while (next.hasNext()) {
                next = next.next();
                context.getSetter().set(context.getCarrier(), next.getHeadKey(), next.getHeadValue());
            }
            span.setComponent(ComponentsDefine.MICROMETER);
        }
       ...
    }
}

[marcingrzejszczak]

Actually, it's the logic of apm-toolkit-micrometer-activation in skywalking java agent. cc @marcingrzejszczak Is there a possibility that we can just use the remoteServiceName in SpanHelper#tryToGetPeer?

Sure, file a PR in skywalking and mention me there so I can review it