-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[Agent] Add plugin for ActiveMQ 5.x #1513
Conversation
Restore configuration
@withlin CI is still broken. @ascrutae Please run this pr with test case pr: SkyAPMTest/agent-auto-integration-testcases#20 |
hi, I have solved the project file, but CI still reported the same error. |
|
||
import net.bytebuddy.description.method.MethodDescription; | ||
import net.bytebuddy.matcher.ElementMatcher; | ||
import org.apache.activemq.ActiveMQSession; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You imported a class, which is not forbidden to exist in Instrument class.
|
||
import net.bytebuddy.description.method.MethodDescription; | ||
import net.bytebuddy.matcher.ElementMatcher; | ||
import org.apache.activemq.ActiveMQSession; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same import issue
Here is the test report and log |
The newest test report looks very good. |
@peng-yongsheng After the merger, it will take time to repair it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, still waiting @ascrutae review.
String url = (String) objInst.getSkyWalkingDynamicField(); | ||
MessageDispatch messageDispatch = (MessageDispatch) allArguments[0]; | ||
AbstractSpan activeSpan = null; | ||
if (messageDispatch.getDestination().getDestinationType() == 1 || messageDispatch.getDestination().getDestinationType() == 5) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number make me confuse. Could you change those number to constant number?
I suggest you send a new pr to change the number to Constant with meaning name. Such as destination type. |
* add activemq-plugin * fix update ActiveMQConsumerInstrumentation Enhance another class * refacor ActiveMQConsumerInstrumentation code * support both topic and queue * clear the code * add UnitTest add fix bug * fix obtain url bug * Refactor the ActiveMQProducerInstrumentation code * fix bug * fix ConstructorInterceptor BUG * add Licensed * Update application.yml Restore configuration * Restore configuration * perfect test * fix * fix License * fix Project Files * fix import class in instrumentation * fix test case * fix java doc * fix Trigger CI * fix ActiveMQConsumerAndProducerConstructorInterceptorTest * delete unit test * fix * fix * fix folder directory problem * Delete ActiveMQConsumerInstrumentation.java
Please answer these questions before submitting pull request
Why submit this pull request?
Bug fix
New feature provided
Improve performance
Related issues
Bug fix
Bug description.
How to fix?
New feature or improvement