feature: support apm with skywalking #3652

zhaoyuguang · 2021-04-21T06:32:30Z

Ⅰ. Describe what this PR did

support apm with skywalking

Ⅱ. Does this pull request fix one issue?

#714
#884
apache/skywalking#6579

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

slievrly · 2021-04-27T07:15:54Z

@wu-sheng , @zhaoyuguang What do you think of using ASF or Seata license header in code files? The Seata community supports the both and fully respects intellectual property rights.
cd21703

wu-sheng · 2021-04-27T07:19:45Z

From my understanding, all codes contributed here should be licensed to the Seata community. SkyWalking just provides the tech stack, dependencies. If you have a LICENSE file for the binary distribution, please add Apache SkyWalking in it.

slievrly · 2021-04-27T07:33:49Z

From my understanding, all codes contributed here should be licensed to the Seata community. SkyWalking just provides the tech stack, dependencies. If you have a LICENSE file for the binary distribution, please add Apache SkyWalking in it.

Do you mean that it needs to add Apache Skywalking in https://github.com/seata/seata/blob/1ec5e031de2de113859b74789f2b72ea91c1cf75/distribution/LICENSE-BIN ?

wu-sheng · 2021-04-27T08:29:08Z

Yes, take this as an example of the official Apache Software Foundation's way.

https://github.com/apache/skywalking/blob/master/dist-material/release-docs/LICENSE#L204

slievrly · 2021-04-27T13:13:46Z

cc @zhaoyuguang

codecov-commenter · 2021-04-27T13:14:29Z

Codecov Report

Merging #3652 (9f3448a) into develop (373d538) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             develop    #3652      +/-   ##
=============================================
- Coverage      51.28%   51.25%   -0.03%     
+ Complexity      3573     3570       -3     
=============================================
  Files            645      645              
  Lines          21814    21814              
  Branches        2740     2736       -4     
=============================================
- Hits           11187    11181       -6     
- Misses          9487     9496       +9     
+ Partials        1140     1137       -3

Impacted Files	Coverage Δ	Complexity Δ
...erver/storage/file/session/FileSessionManager.java	`51.82% <0.00%> (-5.11%)`	`26.00% <0.00%> (-1.00%)`
...tasource/sql/struct/cache/MysqlTableMetaCache.java	`80.45% <0.00%> (-1.15%)`	`10.00% <0.00%> (-1.00%)`
...o/seata/server/coordinator/DefaultCoordinator.java	`52.11% <0.00%> (-0.47%)`	`27.00% <0.00%> (-1.00%)`
...rage/redis/store/RedisTransactionStoreManager.java	`63.59% <0.00%> (-0.44%)`	`38.00% <0.00%> (-1.00%)`
...in/java/io/seata/server/session/GlobalSession.java	`83.04% <0.00%> (-0.44%)`	`75.00% <0.00%> (-1.00%)`
...torage/file/store/FileTransactionStoreManager.java	`57.69% <0.00%> (+0.64%)`	`29.00% <0.00%> (+1.00%)`
...n/src/main/java/io/seata/common/util/IdWorker.java	`83.33% <0.00%> (+6.25%)`	`12.00% <0.00%> (+1.00%)`

…apm-sw-0

zhaoyuguang · 2021-04-28T04:08:36Z

@slievrly @wu-sheng LICENSE file update, please recheck

zhaoyuguang · 2021-04-28T04:26:05Z

@wu-sheng From the Seata project package style, I want to change the plugin package to io.seata.skywalking.plugin, can i ?

wu-sheng · 2021-04-28T05:34:52Z

@wu-sheng From the Seata project package style, I want to change the plugin package to io.seata.skywalking.plugin, can i ?

Yes, of course, it is better to use seata package name, as they host codes.

wu-sheng · 2021-04-29T13:11:12Z

BTW, please remember to update Seata website/doc page, about how to use this.
You could take this as a reference, https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/skywalking-agent-deployment.html

zhaoyuguang · 2021-04-30T02:29:05Z

BTW, please remember to update Seata website/doc page, about how to use this.
You could take this as a reference, https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/skywalking-agent-deployment.html

Yes, the PR only focused on plugin monitoring code, Next is the construction of official website documents

wu-sheng · 2021-04-30T02:36:35Z

Is this going to merge now? Is this ready? @slievrly

caohdgege

@slievrly @wu-sheng LICENSE file update, please recheck

@zhaoyuguang Please modify the LICENSE-BIN under the distribution folder at the meantime

slievrly · 2021-05-08T14:11:51Z

It's already in the pipeline process of code review and function test. Assigned to @caohdgege @selfishlover .

wu-sheng · 2021-05-08T14:33:08Z

Hi, I want to give a reminder, DolphinScheduler has a SkyWalking agent installation out of box. https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/skywalking-agent-deployment.html

A user just needs a system env setup, or several new lines in install_config.conf. Then the agent is good to go.

Right now, this PR seems doesn't have related things. Then, the user will need several manual steps to make all things ready to go.

At the same time, only because of using that way, you should update the LICENSE. If you only host plugin source codes and binary, both of them actually belong to the Seata project, rather than SkyWalking. SkyWalking becomes a compiling level dependency.

@zhaoyuguang @slievrly I think from the original discussion and issue description, we should have skywalking agent with all necessary plugins(not all skywalking's plugins) packaged into Seata's distribution. Could you confirm this?

…apm-sw-0

caohdgege

LGTM

slievrly · 2021-05-20T04:09:22Z

a little problem，the component information of Span shows as unknown.

wu-sheng · 2021-05-20T04:53:20Z

a little problem，the component information of Span shows as unknown.

I think this is caused by component metadata is not included in the latest release, but will be in next.

wu-sheng · 2021-05-20T04:53:39Z

If you want, could add one in the component library YAML file.

slievrly · 2021-05-20T08:10:48Z

a little problem，the component information of Span shows as unknown.

I think this is caused by component metadata is not included in the latest release, but will be in next.

It is my fault. I have seen the above issue comment (#3652 (comment).

slievrly · 2021-05-20T08:26:30Z

Hi, I want to give a reminder, DolphinScheduler has a SkyWalking agent installation out of box. https://dolphinscheduler.apache.org/en-us/docs/latest/user_doc/skywalking-agent-deployment.html

A user just needs a system env setup, or several new lines in install_config.conf. Then the agent is good to go.

Right now, this PR seems doesn't have related things. Then, the user will need several manual steps to make all things ready to go.

At the same time, only because of using that way, you should update the LICENSE. If you only host plugin source codes and binary, both of them actually belong to the Seata project, rather than SkyWalking. SkyWalking becomes a compiling level dependency.

@zhaoyuguang @slievrly I think from the original discussion and issue description, we should have skywalking agent with all necessary plugins(not all skywalking's plugins) packaged into Seata's distribution. Could you confirm this?

This was discussed at the Seata monthly meeting. Seata-server is an independently deployed application. As a user-friendly way, skywalking plugin should be included in the distribution package of Seata-server.

When executing mvn clean install -DskipTests=true -P release-seata, copy dependency plugin can add the seata-server’s dependencies to the distribution folder. apm-Seata-Skywalking-plugin can also use the plugin add all the necessary plugins to distribution /plugin, and set 'javaagent:$path/plugin/skywalking-agent.jar -Dskywalking.agent.service_name=seata_tc' into startup.sh .

I think this function can be optimized in the next pr.

...-skywalking-plugin/src/main/java/io/seata/apm/skywalking/plugin/common/SWSeataConstants.java

…apm-sw-0 # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.

zhaoyuguang · 2021-05-20T10:39:40Z

a little problem，the component information of Span shows as unknown.

I think this is caused by component metadata is not included in the latest release, but will be in next.

a little problem，the component information of Span shows as unknown.

I think this is caused by component metadata is not included in the latest release, but will be in next.

It is my fault. I have seen the above issue comment (#3652 (comment).

I think layer is DistributedTransaction - Seata is better

slievrly · 2021-05-20T12:14:09Z

It looks very good.

Spring Cloud & Seata GlobalCommit case:

The following PR needs to be optimized here. 127.0.0.1:xxx is not an independent node, it always belongs to the biz / order / inventory / account remote service.

slievrly · 2021-05-20T12:15:49Z

Spring Cloud & Seata GlobalRollback case:

slievrly · 2021-05-20T12:21:46Z

In addition, too many SEATA/TC/HeartbeatMessage, it can be ignored, it just keep channel alive and connected.

slievrly

LGTM

wu-sheng · 2021-05-20T12:23:18Z

In addition, too many SEATA/TC/HeartbeatMessage, it can be ignored, it just keep channel alive and connected.

In my mind, a health check should be a meter plugin, rather than tracing.

slievrly · 2021-05-20T12:35:51Z

thanks @wu-sheng @zhaoyuguang @kezhenxu94 @skywalking

A great thing has been accomplished.

wu-sheng · 2021-05-20T12:37:43Z

Look forward to your blog sharing how to use this to diagnose Seata.

zhaoyuguang added 4 commits April 21, 2021 14:31

feature: support apm with skywalking

17932c9

fix CI

ab07b46

Fix CI 2

cd21703

Fix CI 3

c57f20f

slievrly added this to the 1.5.0 milestone Apr 22, 2021

Merge branch 'develop' into apm-sw-0

5b6fb35

zhaoyuguang added 2 commits April 28, 2021 11:28

fix LICENSE

5850015

Merge branch 'apm-sw-0' of https://github.com/zhaoyuguang/seata into …

c08cf2a

…apm-sw-0

fix package path

0cfc00e

Merge branch 'develop' into apm-sw-0

1844b67

funky-eyes requested a review from slievrly May 6, 2021 06:01

caohdgege reviewed May 8, 2021

View reviewed changes

zhaoyuguang added 4 commits May 8, 2021 22:49

fix

a42a91f

fix license

495bdcf

Merge branch 'apm-sw-0' of https://github.com/zhaoyuguang/seata into …

102724d

…apm-sw-0

Merge branch 'develop' into apm-sw-0

5cf718f

zhaoyuguang and others added 9 commits May 9, 2021 15:37

Update README.md

e200cd8

Update SWSeataConstants.java

6dfdd65

fix

90721d2

Merge branch 'apm-sw-0' of https://github.com/zhaoyuguang/seata into …

32d4b2c

…apm-sw-0

Merge branch 'develop' into apm-sw-0

44501b6

fix opt

71d6f10

Merge branch 'apm-sw-0' of https://github.com/zhaoyuguang/seata into …

a8eef54

…apm-sw-0

Merge branch 'develop' into apm-sw-0

86da82a

add md

74e56b0

zhaoyuguang mentioned this pull request May 18, 2021

support apm with skywalking apache/incubator-seata-website#321

Merged

zhaoyuguang and others added 2 commits May 18, 2021 21:14

Merge branch 'develop' into apm-sw-0

76afd24

Merge branch 'develop' into apm-sw-0

112bf45

caohdgege approved these changes May 19, 2021

View reviewed changes

slievrly reviewed May 20, 2021

View reviewed changes

...-skywalking-plugin/src/main/java/io/seata/apm/skywalking/plugin/common/SWSeataConstants.java Show resolved Hide resolved

zhaoyuguang added 2 commits May 20, 2021 17:02

fix at start

e189172

slievrly approved these changes May 20, 2021

View reviewed changes

slievrly merged commit 55dd55d into apache:develop May 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: support apm with skywalking #3652

feature: support apm with skywalking #3652

zhaoyuguang commented Apr 21, 2021

slievrly commented Apr 27, 2021

wu-sheng commented Apr 27, 2021

slievrly commented Apr 27, 2021

wu-sheng commented Apr 27, 2021

slievrly commented Apr 27, 2021

codecov-commenter commented Apr 27, 2021 •

edited

zhaoyuguang commented Apr 28, 2021

zhaoyuguang commented Apr 28, 2021

wu-sheng commented Apr 28, 2021

wu-sheng commented Apr 29, 2021

zhaoyuguang commented Apr 30, 2021

wu-sheng commented Apr 30, 2021

caohdgege left a comment •

edited

slievrly commented May 8, 2021

wu-sheng commented May 8, 2021

caohdgege left a comment

slievrly commented May 20, 2021

wu-sheng commented May 20, 2021

wu-sheng commented May 20, 2021

slievrly commented May 20, 2021

slievrly commented May 20, 2021

zhaoyuguang commented May 20, 2021

slievrly commented May 20, 2021

slievrly commented May 20, 2021

slievrly commented May 20, 2021

slievrly left a comment

wu-sheng commented May 20, 2021

slievrly commented May 20, 2021

wu-sheng commented May 20, 2021

feature: support apm with skywalking #3652

feature: support apm with skywalking #3652

Conversation

zhaoyuguang commented Apr 21, 2021

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

slievrly commented Apr 27, 2021

wu-sheng commented Apr 27, 2021

slievrly commented Apr 27, 2021

wu-sheng commented Apr 27, 2021

slievrly commented Apr 27, 2021

codecov-commenter commented Apr 27, 2021 • edited

Codecov Report

zhaoyuguang commented Apr 28, 2021

zhaoyuguang commented Apr 28, 2021

wu-sheng commented Apr 28, 2021

wu-sheng commented Apr 29, 2021

zhaoyuguang commented Apr 30, 2021

wu-sheng commented Apr 30, 2021

caohdgege left a comment • edited

Choose a reason for hiding this comment

slievrly commented May 8, 2021

wu-sheng commented May 8, 2021

caohdgege left a comment

Choose a reason for hiding this comment

slievrly commented May 20, 2021

wu-sheng commented May 20, 2021

wu-sheng commented May 20, 2021

slievrly commented May 20, 2021

slievrly commented May 20, 2021

zhaoyuguang commented May 20, 2021

slievrly commented May 20, 2021

slievrly commented May 20, 2021

slievrly commented May 20, 2021

slievrly left a comment

Choose a reason for hiding this comment

wu-sheng commented May 20, 2021

slievrly commented May 20, 2021

wu-sheng commented May 20, 2021

codecov-commenter commented Apr 27, 2021 •

edited

caohdgege left a comment •

edited