FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add the jdk httpclient plugin by peachisai · Pull Request #778 · apache/skywalking-java · GitHub

/ skywalking-java Public

Add the jdk httpclient plugin - #778

Merged
wu-sheng merged 43 commits into
apache:mainfrom
peachisai:master
Nov 2, 2025
ConversationCommits (43)ChecksFiles changed
Merged

Add the jdk httpclient plugin#778
wu-sheng merged 43 commits into
apache:mainfrom
peachisai:master

Conversation

peachisai commented Oct 26, 2025 •
edited by wu-sheng
Loading

Copy link
Copy Markdown
Member
  • If this pull request closes/resolves/fixes an existing issue, replace the issue number. #13210
  • Update the CHANGES log.

wu-sheng added enhancement New feature or request plugin labels Oct 26, 2025
wu-sheng added this to the 9.6.0 milestone Oct 26, 2025

peachisai commented Oct 26, 2025

Copy link
Copy Markdown
Member Author

@wu-sheng It seems some Docker images are missing in other plugin test scenarios, Should I rebuild them?

wu-sheng commented Oct 26, 2025

Copy link
Copy Markdown
Member

About kafka image, you could refer to main repo fix.

peachisai commented Nov 1, 2025

Copy link
Copy Markdown
Member Author

@wu-sheng hi, is there any other problem with it?

wu-sheng commented Nov 1, 2025

Copy link
Copy Markdown
Member

Oops, sorry. I left this. Will review it soon.

wu-sheng reviewed Nov 2, 2025
View reviewed changes
Comment thread ...pclient-plugin/src/main/java/org/apache/skywalking/apm/plugin/HttpClientSendInterceptor.java
Comment on lines +69 to +72
} else {
Tags.HTTP_RESPONSE_STATUS_CODE.set(span, 404);
span.errorOccurred();
}

wu-sheng Nov 2, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Same concern here, if there is no return, should not response code. And if there is a null expected, we should log the reason.

wu-sheng Nov 2, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This seems to be not resolved? No reponse still maps to 404 response code. Is this correct?

peachisai Nov 2, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This seems to be not resolved? No reponse still maps to 404 response code. Is this correct?

Sry, had done

wu-sheng commented Nov 2, 2025

Copy link
Copy Markdown
Member

@wu-sheng hi, is there any other problem with it?

I have reviewed the PR, and left some comments.

peachisai commented Nov 2, 2025

Copy link
Copy Markdown
Member Author

Same concern here, if there is no return, should not response code. And if there is a null expected, we should log the reason.

hi, all above changes had been done, please take a review.

wu-sheng reviewed Nov 2, 2025
View reviewed changes
Comment thread ...plugin/src/main/java/org/apache/skywalking/apm/plugin/define/HttpRequestInstrumentation.java Outdated

@Override
protected ClassMatch enhanceClass() {
return HierarchyMatch.byHierarchyMatch(ENHANCE_PARENT_CLASS);

wu-sheng Nov 2, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Could you target specific classes in JDK? I have concerned about this, which may instrument 3rd party frameworks' request implementations.

peachisai Nov 2, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

wu-sheng merged commit e43a802 into apache:main Nov 2, 2025
205 of 230 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

wu-sheng wu-sheng approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request plugin

Projects

None yet

Milestone

9.6.0

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL