Add the jdk httpclient plugin #778
Conversation
|
@wu-sheng It seems some Docker images are missing in other plugin test scenarios, Should I rebuild them? |
|
About kafka image, you could refer to main repo fix. |
|
@wu-sheng hi, is there any other problem with it? |
|
Oops, sorry. I left this. Will review it soon. |
...nt-plugin/src/main/java/org/apache/skywalking/apm/plugin/HttpClientSendAsyncInterceptor.java
Outdated
Show resolved
Hide resolved
...nt-plugin/src/main/java/org/apache/skywalking/apm/plugin/HttpClientSendAsyncInterceptor.java
Outdated
Show resolved
Hide resolved
| } else { | ||
| Tags.HTTP_RESPONSE_STATUS_CODE.set(span, 404); | ||
| span.errorOccurred(); | ||
| } |
There was a problem hiding this comment.
Same concern here, if there is no return, should not response code. And if there is a null expected, we should log the reason.
There was a problem hiding this comment.
This seems to be not resolved? No reponse still maps to 404 response code. Is this correct?
There was a problem hiding this comment.
This seems to be not resolved? No reponse still maps to 404 response code. Is this correct?
Sry, had done
I have reviewed the PR, and left some comments. |
hi, all above changes had been done, please take a review. |
|
|
||
| @Override | ||
| protected ClassMatch enhanceClass() { | ||
| return HierarchyMatch.byHierarchyMatch(ENHANCE_PARENT_CLASS); |
There was a problem hiding this comment.
Could you target specific classes in JDK? I have concerned about this, which may instrument 3rd party frameworks' request implementations.
There was a problem hiding this comment.
Could you target specific classes in JDK? I have concerned about this, which may instrument 3rd party frameworks' request implementations.
Replaced by the name matcher
2 participants
CHANGESlog.