Skip to content

Support RESTEasy 3.x#2665

Merged
wu-sheng merged 17 commits into
apache:masterfrom
yanfch:feature/RESTEasy-plugin
May 24, 2019
Merged

Support RESTEasy 3.x#2665
wu-sheng merged 17 commits into
apache:masterfrom
yanfch:feature/RESTEasy-plugin

Conversation

@yanfch

@yanfch yanfch commented May 14, 2019

Copy link
Copy Markdown
Contributor

Please answer these questions before submitting pull request

  • Why submit this pull request?
  • Bug fix
  • New feature provided
  • Improve performance

New feature or improvement

  • Plugin support RESTEasy 3.x, includes RESTEasy server sync HTTP request and async HTTP request tracking.

@wu-sheng

Copy link
Copy Markdown
Member

Please submit the integration test cases. Do you know where to submit!

@wu-sheng wu-sheng added agent Language agent related. feature New feature plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. labels May 14, 2019
@wu-sheng wu-sheng requested a review from ascrutae May 14, 2019 11:55
@yanfch

yanfch commented May 14, 2019

Copy link
Copy Markdown
Contributor Author

Please submit the integration test cases. Do you know where to submit!

Yes, I'm trying.

@wu-sheng

Copy link
Copy Markdown
Member

CI fails, please do rat:check.

@wu-sheng wu-sheng added the TBD To be decided later, need more discussion or input. label May 14, 2019
@wu-sheng wu-sheng removed the TBD To be decided later, need more discussion or input. label May 14, 2019
@wu-sheng wu-sheng added this to the 6.2.0 milestone May 14, 2019
@wu-sheng

Copy link
Copy Markdown
Member

@ascrutae @IanCao Please review the pull request and test case.

@wu-sheng wu-sheng requested a review from IanCao May 14, 2019 13:42
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.1%) to 16.548% when pulling 2803cd9 on yanfch:feature/RESTEasy-plugin into e07172b on apache:master.

@coveralls

coveralls commented May 15, 2019

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.04%) to 16.863% when pulling ac5eed7 on yanfch:feature/RESTEasy-plugin into 1530a66 on apache:master.

@wu-sheng

Copy link
Copy Markdown
Member

Please resolve the conflict, the shardingsphere plugin just got merged.

@wu-sheng

Copy link
Copy Markdown
Member

@yanfch Is this ready for further tests and review?

@yanfch

yanfch commented May 22, 2019

Copy link
Copy Markdown
Contributor Author

@yanfch Is this ready for further tests and review?

Ready.

@wu-sheng

Copy link
Copy Markdown
Member

@IanCao please review and run tests

@IanCao

IanCao commented May 23, 2019

Copy link
Copy Markdown
Contributor

inline

@SkyWalkingRobot

Copy link
Copy Markdown

Here is the test report and validate logs

@SkyWalkingRobot

Copy link
Copy Markdown

Here is the test report and validate logs

@wu-sheng

Copy link
Copy Markdown
Member

Test passed. @IanCao @ascrutae do code review again before merge, please.

@wu-sheng wu-sheng left a comment

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.

Do we need RESTEasy icon in UI?

@wu-sheng

Copy link
Copy Markdown
Member

RESTEasy doesn't have a clear icon at https://resteasy.github.io/.
@yanfch What is current status of topology when RESTEasy works as server?
@TinyAllen What do you suggest?

@IanCao

IanCao commented May 23, 2019

Copy link
Copy Markdown
Contributor

LGTM

@IanCao IanCao left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@wu-sheng wu-sheng left a comment

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.

Test case change request, please follow this, SkyAPMTest/agent-auto-integration-testcases#68 (review)

@yanfch

yanfch commented May 24, 2019

Copy link
Copy Markdown
Contributor Author

RESTEasy doesn't have a clear icon at https://resteasy.github.io/.

@wu-sheng There should be here icon.

http://design.jboss.org/resteasy/index.htm

@wu-sheng

Copy link
Copy Markdown
Member

@yanfch Then you need a pull request to https://github.com/apache/skywalking-rocketbot-ui/tree/master/src/views/components/topology/assets

There are too many words in the official logo, @TinyAllen provides one,
resteasy, or you could build one and show us the topology screenshot.

@SkyWalkingRobot

Copy link
Copy Markdown

Here is the test report and validate logs

@wu-sheng

Copy link
Copy Markdown
Member

Do we need recheck on 3.5.1? It is acceptable with one version failure. Also, support list document needs to update.

@yanfch

yanfch commented May 24, 2019

Copy link
Copy Markdown
Contributor Author

Try again? Local is the test passed. There is no such exception IllegalDataFileException actualData.yaml

@SkyWalkingRobot

Copy link
Copy Markdown

Here is the test report and validate logs

@wu-sheng wu-sheng left a comment

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.

LGTM. Test passed with all versions. @ascrutae Please do the final confirmation.

@yanfch Please submit pull request to UI and show the image in topology.

@wu-sheng wu-sheng merged commit bc6f06f into apache:master May 24, 2019
@yanfch

yanfch commented May 24, 2019

Copy link
Copy Markdown
Contributor Author

image

@wu-sheng

Copy link
Copy Markdown
Member

@yanfch If this icon makes sense to you, submit the pull request. Otherwise, create a new one if you like :) I am not familiar with RESTEasy, so can't tell it is good or bad.

@yanfch

yanfch commented May 24, 2019

Copy link
Copy Markdown
Contributor Author

I think this is good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Language agent related. feature New feature plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants