provide cassandra java driver 3.x plugin by stone-wlg · Pull Request #3410 · apache/skywalking

stone-wlg · 2019-09-04T05:57:49Z

Please answer these questions before submitting pull request

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

New feature or improvement

http://cassandra.apache.org/

dmsolr · 2019-09-04T07:28:46Z

...g/apm/plugin/cassandra/java/driver/v3/ClusterConstructorWithStateListenerArgInterceptor.java

+        List<InetSocketAddress> inetSocketAddresses = (List<InetSocketAddress>) allArguments[1];
+        StringBuilder hosts = new StringBuilder();
+        for (InetSocketAddress inetSocketAddress : inetSocketAddresses) {
+            hosts.append(inetSocketAddress.getHostName() + ":" + inetSocketAddress.getPort() + ",");


It does not matter, but I suggest to do like that.
hosts.append(inetSocketAddress.getHostName()).append(":").append(inetSocketAddress.getPort()).append(",");

ok, thanks.

...assandra/java/driver/v3/SessionManagerExecuteAndExecuteAsyncWithStatementArgInterceptor.java

wu-sheng · 2019-09-07T17:20:41Z

@dmsolr Any update?

@zhaoyuguang Please run this codes test case again. I want to see any good luck this time.

dmsolr · 2019-09-07T18:00:53Z

@wu-sheng, I think you want to ping @stone-wlg , right?
I had a look deeply, I think it is okay.

wu-sheng · 2019-09-07T18:02:14Z

@dmsolr I was really pinging you, to make sure you don't have a further review to do. Why are you still awake at this moment?

wu-sheng · 2019-09-07T18:04:28Z

@stone-wlg Do you update this PR to support async execution? Do we have the accurate execution time now?

dmsolr · 2019-09-07T18:10:14Z

haha, a glace before sleep. @wu-sheng , have a good dream.

stone-wlg · 2019-09-08T01:38:53Z

@stone-wlg Do you update this PR to support async execution? Do we have the accurate execution time now?

yes, we have.

wu-sheng · 2019-09-08T01:40:57Z

yes, we have.

But I could not find span#prepareForAsync. Is there unnecessary now?

stone-wlg · 2019-09-08T01:44:22Z

yes, we have.

But I could not find span#prepareForAsync. Is there unnecessary now?

No, the async key is the getUninterruptibly method. it likes lazy mode or cursor of database.

wu-sheng · 2019-09-08T01:45:37Z

No, the key is the getUninterruptibly method. it likes lazy mode or cursor of database.

OK. That is another story. I am thinking about this too. But not related to this PR, could be done in a different way.

stone-wlg · 2019-09-08T01:47:45Z

No, the key is the getUninterruptibly method. it likes lazy mode or cursor of database.

OK. That is another story. I am thinking about this too. But not related to this PR, could be done in a different way.

Currently, i intercept getUninterruptibly method by LOCAL METHOD in skywalking. So we can get accurate execution time whatever sync or async way.

wu-sheng · 2019-09-08T01:47:56Z

Let's wait for test case and 6.4.0 shipped. We are only fixing bugs before 6.4.0 release.

wu-sheng · 2019-09-08T01:49:08Z

Currently, i intercept getUninterruptibly method by LOCAL METHOD in skywalking. So we can get accurate execution time whatever sync or async way.

Make sense for now. I am thinking about a special tag for this kind of OP(cursor reading)

stone-wlg · 2019-09-08T01:49:32Z

Let's wait for test case and 6.4.0 shipped. We are only fixing bugs before 6.4.0 release.

ok

stone-wlg · 2019-09-08T01:49:51Z

Currently, i intercept getUninterruptibly method by LOCAL METHOD in skywalking. So we can get accurate execution time whatever sync or async way.

Make sense for now. I am thinking about a special tag for this kind of OP(cursor reading)

Got it.

stone-wlg · 2019-09-08T01:57:08Z

@wu-sheng Currently, i am working monitor whatever log, metric and trace. i am thinking that is it necessary to add MQ(kafka) between skywalking agent and OAP? this is maybe another topic, i just want to know in a short word, thanks

wu-sheng · 2019-09-08T01:58:58Z

Currently, i am working monitor whatever log, metric and trace. i am thinking that is it necessary to add MQ(kafka) between skywalking agent and OAP? this is maybe another topic, i just want to know in a short word, thanks

Most APM didn't do this, because it doesn't necessarily. If you want to discuss detail, feel free to open an issue, we could discuss there and move it to FAQ in case others want to know too.

kezhenxu94

Just for reminder, it seems that there are pairs of async and sync methods in Cassandra, so please consider both of them when you intercept methods (consider whether they have async counterparts or not).

To name a few:

com.datastax.driver.core.Cluster#connect() vs com.datastax.driver.core.Cluster#connectAsync()
com.datastax.driver.core.SessionManager#execute vs com.datastax.driver.core.SessionManager#executeAsync

P.S. seems that the sync version simply calls the async version and wait for the result, so it should be able to just intercept the async version, maybe I'm wrong? Just a hint :)

kezhenxu94 · 2019-09-11T15:08:05Z

...che/skywalking/apm/plugin/cassandra/java/driver/v3/define/SessionManagerInstrumentation.java

+                @Override
+                public ElementMatcher<MethodDescription> getMethodsMatcher() {
+                    return named("execute").and(takesArgumentWithType(0, "com.datastax.driver.core.Statement"))
+                        .or(named("executeAsync").and(takesArgumentWithType(0, "com.datastax.driver.core.Statement")));


executeAsync calls execute at the bottom, so you should be able to only intercept execute (minor issue)

i think the key is the or method. Does it intercept both execute and executeAsync methods, or only intercept execute methods. right ?
but my test result is both.

...che/skywalking/apm/plugin/cassandra/java/driver/v3/define/SessionManagerInstrumentation.java

...org/apache/skywalking/apm/plugin/cassandra/java/driver/v3/define/ClusterInstrumentation.java

stone-wlg · 2019-09-12T02:49:07Z

Just for reminder, it seems that there are pairs of async and sync methods in Cassandra, so please consider both of them when you intercept methods (consider whether they have async counterparts or not).

To name a few:

com.datastax.driver.core.Cluster#connect() vs com.datastax.driver.core.Cluster#connectAsync()

com.datastax.driver.core.SessionManager#execute vs com.datastax.driver.core.SessionManager#executeAsync

P.S. seems that the sync version simply calls the async version and wait for the result, so it should be able to just intercept the async version, maybe I'm wrong? Just a hint :)

exactly. sync version call async version and wait for results.
So you mean that only intercept async version is enough, right?
there are 2 questions, as below

if i intercept both sync and async as EXIT SPAN what is going on in skywalking?
if i only intercept async version, there is a little bit confuse on UI. when user call sync version, but tracing on UI will display async version, is there a standard way to solve it?
thanks.

wu-sheng · 2019-09-12T04:12:56Z

if i intercept both sync and async as EXIT SPAN what is going on in skywalking?

Depend on what kind of codes you are written, basically, override happens. If sync calls async then, span set as sync name. Otherwise, async. Does async interceptor need #asyncFinish?

if i only intercept async version, there is a little bit confuse on UI. when user call sync version, but tracing on UI will display async version, is there a standard way to solve it?

If you only did this, you could just don't separate the sync and async in UI, then should be fine always. You could choose to do it separately or only async.

stone-wlg · 2019-09-12T05:20:38Z

if i intercept both sync and async as EXIT SPAN what is going on in skywalking?

Depend on what kind of codes you are written, basically, override happens. If sync calls async then, span set as sync name. Otherwise, async. Does async interceptor need #asyncFinish?

if i only intercept async version, there is a little bit confuse on UI. when user call sync version, but tracing on UI will display async version, is there a standard way to solve it?

If you only did this, you could just don't separate the sync and async in UI, then should be fine always. You could choose to do it separately or only async.

Ok, override is what i want, and test result is LGTM.

async interceptor do not need #asyncFinish

wu-sheng · 2019-09-12T05:25:51Z

@stone-wlg Your comment format is not clear. Could you reformat it?

stone-wlg · 2019-09-12T05:27:44Z

@stone-wlg Your comment format is not clear. Could you reformat it?

OK

wu-sheng · 2019-09-12T06:12:37Z

You need to update this PR and test cases.

stone-wlg · 2019-09-12T06:18:05Z

You need to update this PR and test cases.

Ok, Done
currently only support 3.7.2 version

wu-sheng · 2019-09-12T06:20:30Z

I think the plugins supporting more versions, right? Just in test case, we plan to merge one version.

@zhaoyuguang run tests again.

@kezhenxu94 recheck please.

stone-wlg · 2019-09-12T06:24:11Z

versions

yes, i have tested 4 versions, all of them are passed on my pc.

zhaoyuguang · 2019-09-12T10:01:04Z

I had triggered the auto tests, let's wait for the result

SkyWalkingRobot · 2019-09-12T10:12:35Z

Here is the test report and validate logs

wu-sheng · 2019-09-12T14:21:17Z

@stone-wlg Welcome to be 156th contributor. Look forward to more contributions from you.

wu-sheng · 2019-09-12T14:22:09Z

docs/en/setup/service-agent/java-agent/Supported-list.md

  * [Solr](https://github.com/apache/lucene-solr/)
    * [SolrJ](https://github.com/apache/lucene-solr/tree/master/solr/solrj) 7.x
+  * [Cassandra](https://github.com/apache/cassandra) 3.x
+    * [cassandra-java-driver](https://github.com/datastax/java-driver) 3.7.2


One nit, we should list all you tested version, don't need to leave 3.7.2 due to test issue.

stone-wlg · 2019-09-12T19:41:34Z

@stone-wlg Welcome to be 156th contributor. Look forward to more contributions from you.

Thanks, 156 is a good number for me

wu-sheng · 2019-09-12T20:10:26Z

/run e2e

wu-sheng · 2019-09-12T20:10:43Z

/run ci

wu-sheng · 2019-09-12T22:51:05Z

/run e2e

wu-sheng · 2019-09-12T23:18:37Z

@stone-wlg The twitter belongs to you, https://twitter.com/ASFSkyWalking/status/1172288313559928832

stone-wlg · 2019-09-13T00:11:01Z

@stone-wlg The twitter belongs to you, https://twitter.com/ASFSkyWalking/status/1172288313559928832

Cool, thanks. Happy Mid-Autumn Festival

add cassandra java driver 3.x plugin

1681725

stone-wlg changed the title ~~add cassandra java driver 3.x plugin~~ provide cassandra java driver 3.x plugin Sep 4, 2019

stone-wlg mentioned this pull request Sep 4, 2019

Cassandra 3.x Plugin #3330

Closed

3 tasks

kezhenxu94 added agent Language agent related. feature New feature plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. labels Sep 4, 2019

kezhenxu94 requested review from IanCao, ascrutae, kezhenxu94, wu-sheng and zhaoyuguang September 4, 2019 06:03

add cassandra java driver 3.x plugin

06b9646

dmsolr reviewed Sep 4, 2019

View reviewed changes

add ContextManager.isActive() condition

0977457

kezhenxu94 requested changes Sep 11, 2019

View reviewed changes

Merge branch 'master' into features/cassandra-java-driver-3.x-plugin

667b742

kezhenxu94 previously approved these changes Sep 12, 2019

View reviewed changes

wu-sheng reviewed Sep 12, 2019

View reviewed changes

update supported list

90a02da

stone-wlg dismissed stale reviews from kezhenxu94 and wu-sheng via 90a02da September 12, 2019 19:47

Merge branch 'master' into features/cassandra-java-driver-3.x-plugin

0b42374

wu-sheng approved these changes Sep 12, 2019

View reviewed changes

wu-sheng merged commit bb0b3d3 into apache:master Sep 12, 2019

kezhenxu94 mentioned this pull request Sep 19, 2019

Provide plugin for Apache Cassandra #3305

Closed

4 tasks

wu-sheng added this to the 6.5.0 milestone Sep 19, 2019

Conversation

stone-wlg commented Sep 4, 2019

New feature or improvement

Uh oh!

dmsolr Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

stone-wlg Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wu-sheng commented Sep 7, 2019

Uh oh!

dmsolr commented Sep 7, 2019

Uh oh!

wu-sheng commented Sep 7, 2019

Uh oh!

wu-sheng commented Sep 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmsolr commented Sep 7, 2019

Uh oh!

stone-wlg commented Sep 8, 2019

Uh oh!

wu-sheng commented Sep 8, 2019

Uh oh!

stone-wlg commented Sep 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wu-sheng commented Sep 8, 2019

Uh oh!

stone-wlg commented Sep 8, 2019

Uh oh!

wu-sheng commented Sep 8, 2019

Uh oh!

wu-sheng commented Sep 8, 2019

Uh oh!

stone-wlg commented Sep 8, 2019

Uh oh!

stone-wlg commented Sep 8, 2019

Uh oh!

stone-wlg commented Sep 8, 2019

Uh oh!

wu-sheng commented Sep 8, 2019

Uh oh!

kezhenxu94 left a comment

Choose a reason for hiding this comment

Uh oh!

kezhenxu94 Sep 11, 2019

Choose a reason for hiding this comment

Uh oh!

stone-wlg Sep 12, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

stone-wlg commented Sep 12, 2019

Uh oh!

wu-sheng commented Sep 12, 2019

Uh oh!

stone-wlg commented Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wu-sheng commented Sep 12, 2019

Uh oh!

stone-wlg commented Sep 12, 2019

Uh oh!

wu-sheng commented Sep 12, 2019

Uh oh!

stone-wlg commented Sep 12, 2019

Uh oh!

wu-sheng commented Sep 12, 2019

Uh oh!

stone-wlg commented Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhaoyuguang commented Sep 12, 2019

Uh oh!

SkyWalkingRobot commented Sep 12, 2019

Uh oh!

wu-sheng commented Sep 7, 2019 •

edited

Loading

stone-wlg commented Sep 8, 2019 •

edited

Loading

stone-wlg commented Sep 12, 2019 •

edited

Loading

stone-wlg commented Sep 12, 2019 •

edited

Loading