The Wayback Machine - https://web.archive.org/web/20200625223624/https://github.com/opentracing-contrib/java-spring-cloud/issues/5
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trace messaging #5

Open
pavolloffay opened this issue Jul 12, 2017 · 31 comments
Open

Trace messaging #5

pavolloffay opened this issue Jul 12, 2017 · 31 comments

Comments

@pavolloffay
Copy link
Contributor

@pavolloffay pavolloffay commented Jul 12, 2017

Trace org.springframework.messaging.MessageChannel

@alesj alesj self-assigned this Jul 24, 2017
@alesj alesj mentioned this issue Aug 7, 2017
@pavolloffay
Copy link
Contributor Author

@pavolloffay pavolloffay commented Aug 8, 2017

This integration should trace all messaging platforms supported by Spring:

In sleuth messaging is reused for tracing WebSockets ( #25 )

@pavolloffay
Copy link
Contributor Author

@pavolloffay pavolloffay commented Aug 11, 2017

Reopened, it's not done completely.

@pavolloffay pavolloffay reopened this Aug 11, 2017
@pavolloffay
Copy link
Contributor Author

@pavolloffay pavolloffay commented Aug 30, 2017

Currently, only artemis and activemq are supported. We need to implement rabbitmq integration for RabbitTemplate and AmqpTemplate and also kafka.

@malafeev
Copy link
Collaborator

@malafeev malafeev commented Sep 25, 2017

if method with @JmsListener annotation has argument different than javax.jms.Message
then JmsListenerAspect doesn't intercept a call and tracing is not working for such consumer.

@pavolloffay
Copy link
Contributor Author

@pavolloffay pavolloffay commented Sep 25, 2017

@malafeev thanks, we should fix it. Can you provide examples of methods annotated with @JmsListener?

cc @alesj

@alesj
Copy link
Collaborator

@alesj alesj commented Sep 28, 2017

@pavolloffay @malafeev hmmm, that's a bit trickier to handle, as you need to somehow get your hands on the actual JMS Message instance.
But I'll have a look to see what can be done.

@ask4gilles
Copy link
Contributor

@ask4gilles ask4gilles commented Feb 23, 2018

Hi @pavolloffay
Any update on this? I'd like to be able to trace messages using Spring amqp/RabbitTemplate
Thank you!

@pavolloffay
Copy link
Contributor Author

@pavolloffay pavolloffay commented Feb 23, 2018

@gytis would the integration you have recently added (spring messaging) trace amqp/rabbit template?

@gytis
Copy link
Collaborator

@gytis gytis commented Feb 26, 2018

@pavolloffay I don't know, would have to run a test. It does trace Spring Cloud Stream with RabbitMQ, but not sure about Rabbit template.

@ask4gilles
Copy link
Contributor

@ask4gilles ask4gilles commented Feb 26, 2018

@gytis @pavolloffay Looking at io.opentracing.contrib.spring.integration.messaging.OpenTracingChannelInterceptor, it instruments org.springframework.messaging.MessageChannel which is not used by org.springframework.amqp.rabbit.core.RabbitTemplate. I think a "traced" implementation of org.springframework.amqp.core.AmqpTemplate would be needed?

@pavolloffay
Copy link
Contributor Author

@pavolloffay pavolloffay commented Feb 26, 2018

Yes it probably needs a dedicated insturmentation

@ask4gilles
Copy link
Contributor

@ask4gilles ask4gilles commented Apr 13, 2018

Hi @gytis and @pavolloffay, I'd like to contribute on the rabbitMq instrumentation.
Should this happen in the opentracing-spring-messaging repo or somewhere else?

@gytis
Copy link
Collaborator

@gytis gytis commented Apr 13, 2018

It should probably go to a separate repo similar to https://github.com/opentracing-contrib/java-jms. opentracing-spring-messaging is an instrumentation for massaging of Spring Integration i.e. ChannelInterceptor.

@ask4gilles
Copy link
Contributor

@ask4gilles ask4gilles commented Apr 13, 2018

Thanks @gytis , that's also the way I saw it.

@pavolloffay
Copy link
Contributor Author

@pavolloffay pavolloffay commented Apr 13, 2018

It depends on how complex it is and whether it can be reused in other projects. If it will just a wrapper around JMS it can be here.

Generally speaking you can submit a PR here and we will see whether it makes sense to move it somewhere else.

@ask4gilles
Copy link
Contributor

@ask4gilles ask4gilles commented Apr 17, 2018

@pavolloffay #126 is ready for review :)

@ask4gilles
Copy link
Contributor

@ask4gilles ask4gilles commented Jun 19, 2018

RabbitMQ instrumentation is located here.
https://github.com/opentracing-contrib/java-spring-rabbitmq
Its starter has been integrated here
#158

@bygui86
Copy link

@bygui86 bygui86 commented Jul 13, 2018

Hi @pavolloffay
Any update on kafka integration? I'd like to be able to trace messages using Spring Cloud Stream and Kafka.
Thank you!!

@pavolloffay
Copy link
Contributor Author

@pavolloffay pavolloffay commented Jul 13, 2018

hi @bygui86, not yet. Would you like to contribute it?

@bygui86
Copy link

@bygui86 bygui86 commented Jul 18, 2018

Thanks @pavolloffay, I will try to have a look and understand how to achieve that :)

@timtebeek
Copy link

@timtebeek timtebeek commented Jul 19, 2018

Hmm; wasn't aware of this issue; this seems related for passing tracing data between Kafka & Spring Cloud:
opentracing-contrib/java-kafka-client#26
Any chance you can improve the current integration? The proposed workaround at present seems too cumbersome to be practical.

@malafeev
Copy link
Collaborator

@malafeev malafeev commented Jul 20, 2018

Looking at JMS/RabbitMQ integrations looks like Kafka also requires some bean post processing hacks :)

@bygui86
Copy link

@bygui86 bygui86 commented Jul 20, 2018

Probably I found another way to integrate Spring Cloud Stream Kafka with OpenTracing.
I will try it today, if it's working I will post an example.

@bygui86
Copy link

@bygui86 bygui86 commented Jul 20, 2018

@pavolloffay I found a sort of work around to let everything work with Spring Cloud Stream, Kafka and Jaeger. Unfortunately as you can see from following issues there is a concurrency problem
jaegertracing/jaeger-client-java#363
jaegertracing/jaeger-client-java#334

And here is the repo with my example ;)
https://github.com/bygui86/spring-cloud-stream-kafka-jaeger

Let me know if you guys have the same exception and if you managed to solve it :)
Thanks

@pavolloffay
Copy link
Contributor Author

@pavolloffay pavolloffay commented Jul 20, 2018

@bygui86 thanks, I will have a look at it

@pavolloffay
Copy link
Contributor Author

@pavolloffay pavolloffay commented Jul 23, 2018

@bygui86 does kafka tracing work via spring stream without any changes in this repo? Do we need to provide some auto configs here?

@bygui86
Copy link

@bygui86 bygui86 commented Jul 23, 2018

@pavolloffay I'm still working on that side. As you can see for example in the class SinkTracingConfig, I had to redefine some beans from spring-kafka (DefaultKafkaConsumerFactory, ConcurrentKafkaListenerContainerFactory) and from opentracing-contrib-kafka (TracingConsumerFactory).
I will try to understand and better use the class "ConcurrentKafkaListenerContainerFactoryConfigurer" from spring-boot-autoconfigure, in order to avoid some bean redefinitions. I will try also to avoid redefine the TracingConsumerFactory bean.
If everything is working just with the ConcurrentKafkaListenerContainerFactoryConfigurer, I would say no need of auto-configs, otherwise could be useful something like TracingConsumerFactory.
So for now I'm not sure.

@ghilainm
Copy link
Contributor

@ghilainm ghilainm commented Mar 18, 2019

@pavolloffay any idea when KafkaTemplate will be supported?

@garimakemwal
Copy link

@garimakemwal garimakemwal commented May 11, 2020

@pavolloffay Can support for KafkaTemplate be assumed done with the PR referred to, above?
The comment here reflects it as open.

@pavolloffay
Copy link
Contributor Author

@pavolloffay pavolloffay commented May 12, 2020

Per #280 (comment) yes. I am marking it as done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.