Support tracking the message listener of spring jms #706

hyhyf · 2024-07-19T09:25:27Z

If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
Update the CHANGES log.

hyhyf · 2024-07-19T09:39:58Z

I recommited the code and successfully passed the github action job

test/plugin/scenarios/spring-jms-5.x-scenario/support-version.list

.../java/org/apache/skywalking/apm/plugin/spring/jms/define/MessageListenerInstrumentation.java

docs/en/setup/service-agent/java-agent/Supported-list.md

CHANGES.md

wu-sheng · 2024-07-20T09:22:15Z

Your test scenarios failed. Please test them locally, make sure they are passed and stable.

wu-sheng · 2024-07-21T04:17:33Z

Why did you remove 5.0?

hyhyf · 2024-07-21T04:37:03Z

Why did you remove 5.0?

5.0 spring version too low lead to test scenarios failed

wu-sheng · 2024-07-21T04:40:15Z

My question is about why. We should be clear about supported versions.
5.x should cover 5.0.

Could you check the codes and explain why?

hyhyf · 2024-07-21T04:54:08Z

My question is about why. We should be clear about supported versions. 5.x should cover 5.0.

Could you check the codes and explain why?

The plugin can support 5.0, but I'm still figuring out how to make it pass the test scenario

wu-sheng · 2024-07-21T04:58:12Z

Take your time. We should cover all 5.x versions.

wu-sheng · 2024-07-25T07:54:22Z

Any update? I am planning to cut the 9.3.0 release

# Conflicts: # CHANGES.md # docs/en/setup/service-agent/java-agent/Plugin-list.md

wu-sheng · 2024-07-28T08:55:37Z

Please resolve the conflicts.

apm-sniffer/apm-agent-core/pom.xml

...in/src/main/java/org/apache/skywalking/apm/plugin/spring/jms/MessageListenerInterceptor.java

test/plugin/scenarios/spring-jms-5.x-scenario/config/expectedData.yaml

...o/src/main/java/org/apache/skywalking/apm/testcase/spring/jms/controller/CaseController.java

test/plugin/scenarios/spring-jms-5.x-scenario/config/expectedData.yaml

...in/src/main/java/org/apache/skywalking/apm/plugin/spring/jms/MessageListenerInterceptor.java

wu-sheng · 2024-08-05T14:15:11Z

What happened here?

hyhyf · 2024-08-05T14:30:27Z

What happened here?

I thought you didn't need this plugin.

hyhyf · 2024-08-05T14:34:16Z

What happened here?

I removed the context carrier and replaced it with context snapshot. Is that right?

wu-sheng · 2024-08-05T14:34:31Z

I am waiting for your reply about why do header extra when you only create local span. This is rare from my understanding.

hyhyf · 2024-08-05T14:46:55Z

I am waiting for your reply about why do header extra when you only create local span. This is rare from my understanding.

I hope to trace specific execute of the active MQ consumers.

wu-sheng · 2024-08-05T14:48:55Z

But it is pointless when you have another header extracring from entry span from the actual entry span.

Local span doesn't need reference of the producer.

hyhyf · 2024-08-05T14:56:43Z

But it is pointless when you have another header extracring from entry span from the actual entry span.

Local span doesn't need reference of the producer.

Yes, after your reminder, I realized this mistake.

I did this before because if I use context snapshot to connect to consumers, I also need to enhance the active MQ message.

So, this plugin doesn't need to do anything except create a local span?

wu-sheng · 2024-08-05T16:04:31Z

If that could make the trace continues as your expectations, then yes.

cloudgc · 2024-08-06T03:36:17Z

The jms connectionFactory is very troublesome to obtain the MQ_BROKER Tag

cloudgc · 2024-08-06T03:36:48Z

Can only be used by hard coding for now ， different approaches to JNDI and connectionFactory need to be considered

wu-sheng · 2024-08-06T05:32:52Z

I think active MQ should be updated to put correct tag value in JMS case.

cloudgc · 2024-08-06T09:53:15Z

I have studied the support of several major mqs for jms and how to integrate them. In most scenarios, only ActiveMQ uses JMS, so writing a general plug-in to support JMS seems a bit costly. If you need to use JMS, writing a plug-in with hard coding is the best way.

cloudgc · 2024-08-06T10:45:35Z

should create plugin project
spring-jms-activemq-5.x-plugin
spring-jms-rabbitmq-5.x-plugin .... like this

wu-sheng · 2024-08-06T12:34:13Z

Agree for that. Feel free to update.

hyhyf · 2024-08-06T15:55:11Z

should create plugin project spring-jms-activemq-5.x-plugin spring-jms-rabbitmq-5.x-plugin .... like this

Thank you for your recommend.

But i have no idea about this, do you have any plans?

wu-sheng · 2024-08-06T16:45:56Z

No, Java agent nowadays is totally relying on volunteers.
I don't touch on codes, especially plugins, for years.

hyhyf · 2024-08-06T16:56:06Z

No, Java agent nowadays is totally relying on volunteers. I don't touch on codes, especially plugins, for years.

Yes, i know :)

I asked @cloudgc

cloudgc · 2024-08-07T01:18:59Z

i write demo project

https://github.com/cloudgc/apm-spring-jms

already applied to actual company projects

If you need it urgently, you can fork from it, I will consider adding this plug-in later.

wu-sheng · 2024-08-07T05:52:16Z

Close for now. Discussion could continue.
If either of you wants to add the new one, send a new pull request.

hyhyf · 2024-08-07T10:15:04Z

Close for now. Discussion could continue. If either of you wants to add the new one, send a new pull request.

Agree, thank you for your guidance. @wu-sheng

hyhyf · 2024-08-07T10:22:17Z

i write demo project

https://github.com/cloudgc/apm-spring-jms

already applied to actual company projects

If you need it urgently, you can fork from it, I will consider adding this plug-in later.

I have viewed your demo, and it cloud work together with the activemq-5.x-plugin to create duplicate consumer entry spans. @cloudgc

cloudgc · 2024-08-07T12:14:06Z

There is indeed a problem

This is the call graph. I will think of a friendly way to fix it.

wu-sheng · 2024-08-07T12:16:38Z

i write demo project

https://github.com/cloudgc/apm-spring-jms

already applied to actual company projects

If you need it urgently, you can fork from it, I will consider adding this plug-in later.

I have viewed your demo, and it cloud work together with the activemq-5.x-plugin to create duplicate consumer entry spans. @cloudgc

What do you mean duplicate? Does it leverage the nested entry span mechanism? Which make only the first entry span left.
Is that what you mean?

hyhyf · 2024-08-07T16:21:44Z

I don't think that, the demo projcet will create a new entry span link through ActiveMQ producer span, this repeats with ActiveMQ consumer.

The process of use spring JMS & third-party MQ would be like:

-> create connectionFactory(take ActiveMQ for example)
-> JMS send message
-> MQ client create message(the activemq plugin is working, create producer entry span)
-> JMS listener
-> MQ client consume message(the activemq plugin is working, create consumer entry span)
-> JMS listener execute(the activemq plugin lost trace)

cloudgc · 2024-08-08T02:06:43Z

the final effect is as follows:

activemq-consumer entry was stop when over dequeue method
so there must be a new span at the same level with MQ-consumer Span

and i fix my demo : apm-spring-jms

hyhyf · 2024-08-08T09:40:54Z

the final effect is as follows:

activemq-consumer entry was stop when over dequeue method so there must be a new span at the same level with MQ-consumer Span

and i fix my demo : apm-spring-jms

I have discussed with @wu-sheng, and create two entry spans in the consumer part, which will cause the consumer traffic to be counted twice.

So far, i think the better way is to update the active MQ plugin, and generate a context snapshot when creating messages, the spring JMS plugin create a local span, if message has a context snapshot, it will be linked.

cloudgc · 2024-08-08T09:46:40Z

should be , not only activemq needs to do this, other mq queues need create context snapshot ， looking forward to your code changes

Support tracking the message listener of spring jms

fd8f52c

wu-sheng reviewed Jul 19, 2024

View reviewed changes

test/plugin/scenarios/spring-jms-5.x-scenario/support-version.list Outdated Show resolved Hide resolved

wu-sheng added plugin feature labels Jul 19, 2024

wu-sheng previously approved these changes Jul 19, 2024

View reviewed changes

.../java/org/apache/skywalking/apm/plugin/spring/jms/define/MessageListenerInstrumentation.java Outdated Show resolved Hide resolved

docs/en/setup/service-agent/java-agent/Supported-list.md Outdated Show resolved Hide resolved

CHANGES.md Outdated Show resolved Hide resolved

Update Spring-jms-5.x-plugin & Supported-list.md & CHANGES.md

ccbffda

hyhyf dismissed wu-sheng’s stale review via ccbffda July 20, 2024 04:13

Update Spring-jms-5.x-scenario support-version.list

8beb20a

hyhyf added 2 commits July 28, 2024 15:39

Update Spring-jms-5.x-scenario pom.xml

c2e3c75

Merge branch 'main' into hyhyf

fc175e5

# Conflicts: # CHANGES.md # docs/en/setup/service-agent/java-agent/Plugin-list.md