opentracing-contrib / java-spring-rabbitmq

OpenTracing RabbitMQ instrumentation
Apache License 2.0
18 stars 21 forks source link

Non-availability of TracingAspect for RabbitMQ Send #36

Closed KishoreKaushal closed 5 years ago

KishoreKaushal commented 5 years ago

It's not exactly an issue. I am interested to know why there is no tracing support for org.springframework.amqp.rabbit.core.RabbitTemplate#send. And what are the future plans for it?

ask4gilles commented 5 years ago

@KishoreKaushal Because most of the usage do not involve org.springframework.amqp.core.Message directly I guess, but use other types which are converted? Are you using this type directly?

KishoreKaushal commented 5 years ago

Yes, @ask4gilles; my organization have codes which uses template.send directly. Now they want tracing for it. I would like to know the status of the send aspect because if this dependency is (or will) not providing that then we might have to implement something by ourselves which may take more effort and time.

KishoreKaushal commented 5 years ago

@ask4gilles How the spanContext is extracted from the carrier in this code? I went through the source but it was very difficult for me to understand how it is rebuilding the spanContext and hence a child span on the receiving side more importantly when a @RabbitListener is present? Could you please explain it?

ask4gilles commented 5 years ago

@KishoreKaushal You mean here?

KishoreKaushal commented 5 years ago

@ask4gilles So what I want to do is implement a function which will be invoked whenever a message is received by a @RabbitListener which will enable me to extract the span context from the MessageProperties followed by building a child span. What part of the code is responsible for that? Since, I am new to Java and Spring it is quite difficult for me to identify those parts. What I understand basically is that somehow RabbitMqBeanPostProcessor and RabbitMqReceiveTracingInterceptor comes into picture whenever a message is received by a @RabbitListener.

ask4gilles commented 5 years ago

@KishoreKaushal Since @RabbitListener uses SimpleRabbitListenerContainerFactory behind the hood, this is also automatically instrumented by this library.

What I understand basically is that somehow RabbitMqBeanPostProcessor and RabbitMqReceiveTracingInterceptor comes into picture whenever a message is received by a @RabbitListener.

Indeed, the RabbitMqReceiveTracingInterceptor is added to the adviceChain of the SimpleRabbitListenerContainerFactory. It extracts the message and its properties and then uses https://github.com/opentracing-contrib/java-spring-rabbitmq/blob/89367dceffc30482f79d1553926319a97522d921/opentracing-spring-rabbitmq/src/main/java/io/opentracing/contrib/spring/rabbitmq/RabbitMqTracingUtils.java#L36 to build the span.

KishoreKaushal commented 5 years ago

@ask4gilles I tried implementing aspect for send method, I am getting the following error:

Unable to proxy interface-implementing method [public final void org.springframework.amqp.rabbit.core.RabbitTemplate.start()] because it is marked as final: Consider using interface-based JDK proxies instead!
Unable to proxy interface-implementing method [public final void org.springframework.amqp.rabbit.core.RabbitTemplate.stop()] because it is marked as final: Consider using interface-based JDK proxies instead!

Even though I annotated @EnableAspectJAutoProxy(proxyTargetClass = false) I am getting this error from CglibAopProxy framework.

ask4gilles commented 5 years ago

The error mentions start and stop methods, not send? I guess there is an error in your pointcut definition.

KishoreKaushal commented 5 years ago

@ask4gilles This is my pointcut definition:

@Around(value = "execution(* org.springframework.amqp.core.AmqpTemplate.send(..)) && 
args(exchange,routingKey, message)", argNames = "pjp,exchange,routingKey,message")
public Object traceAmqpSend(ProceedingJoinPoint pjp, String exchange, 
    String routingKey, Message message) throws Throwable {
      // ------- code ----------- //
}

Could you please explain where I am wrong?

ask4gilles commented 5 years ago

This looks fine.

KishoreKaushal commented 5 years ago

This looks fine.

But this is the one which is throwing the error.

ask4gilles commented 5 years ago

If you can share a minimal project somewhere, I could have a look at it.

KishoreKaushal commented 5 years ago

Given below is the code for the send tracing aspect:


@Aspect
@Configuration
public class AmqpSendTracingAspect {
    private final Tracer tracer;

    public AmqpSendTracingAspect(Tracer tracer) {
        this.tracer = tracer;
    }

    @Around(value = "execution(* org.springframework.amqp.core.AmqpTemplate.send(..)) " +
            "&& args(exchange,routingKey, message)",
            argNames = "pjp,exchange,routingKey,message")
    public Object traceAmqpSend(ProceedingJoinPoint pjp,
                                String exchange, String routingKey, Message message) throws Throwable {

        final Object[] args = pjp.getArgs();

        System.out.println("Aspect RUnning");

        final MessageProperties messageProperties = message.getMessageProperties();

        Scope scope = RabbitMqTracingUtils.buildSendSpan(tracer, messageProperties);
        tracer.inject(
                scope.span().context(),
                Format.Builtin.TEXT_MAP,
                new RabbitMqInjectAdapter(messageProperties));

        RabbitMqSpanDecorator spanDecorator = new RabbitMqSpanDecorator();
        spanDecorator.onSend(messageProperties, exchange, routingKey, scope.span());

        args[2] = message;

        try {
            return pjp.proceed(args);
        } catch (Exception ex) {
            spanDecorator.onError(ex, scope.span());
            throw ex;
        } finally {
            scope.close();
        }
    }
}

At present my project isn't doing anything special it is just sending a message using template.send. This is just to test whether this aspect is getting invoked or not.

Here is how I am testing these things:


@SpringBootApplication
@EnableAspectJAutoProxy(proxyTargetClass = false)
public class Demo1tracingApplication {

    @Autowired
    private JaegerTracer tracer;

    public static void main(String[] args) {

        SpringApplication.run(Demo1tracingApplication.class, args);

        ApplicationContext context =
                new AnnotationConfigApplicationContext(AmqpConfig.class);
        AmqpTemplate template = context.getBean(AmqpTemplate.class);
        Queue send_q = context.getBean(Queue.class);

        template.send("", send_q.getName(), MessageBuilder.withBody("test_message".getBytes()).build());

        System.out.println("Sent");
    }
}

I know it is a bad style of coding but at present I am only interested in whether that Aspect is invoked or not.

ask4gilles commented 5 years ago

It would be easier if you push your project sample on github somewhere to see your complete setup (dependencies etc). If you have the spring boot starter amqp on the classpath, just autowire a RabbitTemplate bean on a class (other than the main) and use it to send your message.

KishoreKaushal commented 5 years ago

@ask4gilles I tried using the convertAndSend function but the problem(Unable to proxy) remains same. I don't know what is going wrong. I think this is the case because convertAndSend internally calls the send which further invokes start method which is a final method.

Please find the attached zipped project file. Any help is greatly appreciated. Thank You.

anothertesting.zip

error

ask4gilles commented 5 years ago

I did some minor adaptations. The aspect is triggered. The message that you see is not an error but just a log info... anothertesting-adapted.zip

KishoreKaushal commented 5 years ago

Thanks a lot @ask4gilles

KishoreKaushal commented 5 years ago

@ask4gilles Could you please add the following code in your tracing aspect file? Then we wouldn't need two separate packages as dependencies.

@Around(value = "execution(* org.springframework.amqp.core.AmqpTemplate.send(..)) && args(exchange, " +
      "routingKey, message)", argNames = "pjp,exchange, routingKey, message"
  )
  public Object traceRabbitSend(
      ProceedingJoinPoint pjp, String exchange, String routingKey, Object message)
      throws Throwable {

      return createTracingHelper()
        .doWithTracingHeadersMessage(exchange, routingKey, message, (convertedMessage) ->
            proceedReplacingMessage(pjp, convertedMessage, 2));
  }

By the way thanks a lot.