You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/02/19 14:30:29 UTC
[GitHub] [skywalking] aderm opened a new pull request #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
aderm opened a new pull request #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
URL: https://github.com/apache/skywalking/pull/4386
…fix agent test removeOnExit parse error.
Please answer these questions before submitting pull request
- Why submit this pull request?
- [ ] Bug fix
- [ ] New feature provided
- [x] Improve performance
- Related issues
#4381
___
### Bug fix
- Bug description.
- How to fix?
___
### New feature or improvement
- Describe the details and related test reports.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io commented on issue #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
URL: https://github.com/apache/skywalking/pull/4386#issuecomment-588586916
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4386?src=pr&el=h1) Report
> Merging [#4386](https://codecov.io/gh/apache/skywalking/pull/4386?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/29da5738bcea2a55e5d88f4eda9ea9056be38867?src=pr&el=desc) will **increase** coverage by `0.12%`.
> The diff coverage is `73.97%`.
[](https://codecov.io/gh/apache/skywalking/pull/4386?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4386 +/- ##
==========================================
+ Coverage 25.43% 25.55% +0.12%
==========================================
Files 1203 1209 +6
Lines 27841 27914 +73
Branches 3837 3851 +14
==========================================
+ Hits 7081 7134 +53
- Misses 20122 20129 +7
- Partials 638 651 +13
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4386?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../RestHighLevelClientClusterMethodsInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4386/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vZWxhc3RpY3NlYXJjaC02LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vZWxhc3RpY3NlYXJjaC92Ni9pbnRlcmNlcHRvci9SZXN0SGlnaExldmVsQ2xpZW50Q2x1c3Rlck1ldGhvZHNJbnRlcmNlcHRvci5qYXZh) | `0% <0%> (ø)` | |
| [...plugin/ehcache/v2/EhcacheCacheNameInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4386/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vZWhjYWNoZS0yLngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vZWhjYWNoZS92Mi9FaGNhY2hlQ2FjaGVOYW1lSW50ZXJjZXB0b3IuamF2YQ==) | `100% <100%> (ø)` | |
| [...cache/v2/EhcachePrivateConstructorInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4386/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vZWhjYWNoZS0yLngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vZWhjYWNoZS92Mi9FaGNhY2hlUHJpdmF0ZUNvbnN0cnVjdG9ySW50ZXJjZXB0b3IuamF2YQ==) | `75% <75%> (ø)` | |
| [...or/ClusterClientPutSettingsMethodsInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4386/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vZWxhc3RpY3NlYXJjaC02LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vZWxhc3RpY3NlYXJjaC92Ni9pbnRlcmNlcHRvci9DbHVzdGVyQ2xpZW50UHV0U2V0dGluZ3NNZXRob2RzSW50ZXJjZXB0b3IuamF2YQ==) | `75% <75%> (ø)` | |
| [...rceptor/ClusterClientHealthMethodsInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4386/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vZWxhc3RpY3NlYXJjaC02LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vZWxhc3RpY3NlYXJjaC92Ni9pbnRlcmNlcHRvci9DbHVzdGVyQ2xpZW50SGVhbHRoTWV0aG9kc0ludGVyY2VwdG9yLmphdmE=) | `80% <80%> (ø)` | |
| [...or/ClusterClientGetSettingsMethodsInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4386/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vZWxhc3RpY3NlYXJjaC02LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vZWxhc3RpY3NlYXJjaC92Ni9pbnRlcmNlcHRvci9DbHVzdGVyQ2xpZW50R2V0U2V0dGluZ3NNZXRob2RzSW50ZXJjZXB0b3IuamF2YQ==) | `81.25% <81.25%> (ø)` | |
| [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4386/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `67.94% <0%> (-1.29%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/skywalking/pull/4386/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4386?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4386?src=pr&el=footer). Last update [29da573...c9ef44a](https://codecov.io/gh/apache/skywalking/pull/4386?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
URL: https://github.com/apache/skywalking/pull/4386#issuecomment-588577687
> checked CI fail is configuration-nacos fails
> <img alt="WX20200220-092041@2x" width="1088" src="https://user-images.githubusercontent.com/2892433/74891835-f5c04200-53c2-11ea-8654-1c66cd4a10a5.png">
>
> ElasticSearch6_7 is es6 test： docker startup failure! but my local is passed.
> <img alt="WX20200220-092231@2x" width="1428" src="https://user-images.githubusercontent.com/2892433/74891841-f953c900-53c2-11ea-8b41-ef4c9f049e14.png">
>
> <img alt="WX20200220-092722@2x" width="871" src="https://user-images.githubusercontent.com/2892433/74891934-3b7d0a80-53c3-11ea-8303-990e1abc8861.png">
>
> FYI @dmsolr
Found the reason, `removeOnExit` caused, my local plugin test dependency package is not updated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
URL: https://github.com/apache/skywalking/pull/4386#discussion_r381694751
##########
File path: apm-sniffer/apm-sdk-plugin/elasticsearch-6.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/elasticsearch/v6/define/ClusterClientInstrumentation.java
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.plugin.elasticsearch.v6.define;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import org.apache.skywalking.apm.plugin.elasticsearch.v6.interceptor.Constants;
+
+public class ClusterClientInstrumentation extends ClassEnhancePluginDefine {
+
+ public static final String ENHANCE_CLASS = "org.elasticsearch.client.ClusterClient";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return new ConstructorInterceptPoint[0];
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("health").or(named("healthAsync"));
Review comment:
Two points:
1 when the local test found that the ES server did not display correctly when calling `health` #4370
2 enhance class `ClusterClient` integrity.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] dmsolr commented on a change in pull request #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
URL: https://github.com/apache/skywalking/pull/4386#discussion_r381686482
##########
File path: apm-sniffer/apm-sdk-plugin/elasticsearch-6.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/elasticsearch/v6/define/ClusterClientInstrumentation.java
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.plugin.elasticsearch.v6.define;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import org.apache.skywalking.apm.plugin.elasticsearch.v6.interceptor.Constants;
+
+public class ClusterClientInstrumentation extends ClassEnhancePluginDefine {
+
+ public static final String ENHANCE_CLASS = "org.elasticsearch.client.ClusterClient";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return new ConstructorInterceptPoint[0];
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("health").or(named("healthAsync"));
Review comment:
Why do we need to enhance this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
URL: https://github.com/apache/skywalking/pull/4386#issuecomment-588562896
checked CI fail is configuration-nacos fails
<img width="1088" alt="WX20200220-092041@2x" src="https://user-images.githubusercontent.com/2892433/74891835-f5c04200-53c2-11ea-8654-1c66cd4a10a5.png">
ElasticSearch6_7 is es6 test： docker startup failure! but my local is passed.
<img width="1428" alt="WX20200220-092231@2x" src="https://user-images.githubusercontent.com/2892433/74891841-f953c900-53c2-11ea-8b41-ef4c9f049e14.png">
<img width="871" alt="WX20200220-092722@2x" src="https://user-images.githubusercontent.com/2892433/74891934-3b7d0a80-53c3-11ea-8303-990e1abc8861.png">
FYI @dmsolr
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng merged pull request #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
URL: https://github.com/apache/skywalking/pull/4386
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
URL: https://github.com/apache/skywalking/pull/4386#discussion_r381996407
##########
File path: apm-sniffer/apm-sdk-plugin/elasticsearch-6.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/elasticsearch/v6/define/ClusterClientInstrumentation.java
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.plugin.elasticsearch.v6.define;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import org.apache.skywalking.apm.plugin.elasticsearch.v6.interceptor.Constants;
+
+public class ClusterClientInstrumentation extends ClassEnhancePluginDefine {
+
+ public static final String ENHANCE_CLASS = "org.elasticsearch.client.ClusterClient";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return new ConstructorInterceptPoint[0];
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("health").or(named("healthAsync"));
Review comment:
@dmsolr Why concern about this? I plan to accept this rather than #4386
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] dmsolr commented on issue #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
Posted by GitBox <gi...@apache.org>.
dmsolr commented on issue #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
URL: https://github.com/apache/skywalking/pull/4386#issuecomment-588578646
> Found the reason, removeOnExit caused, my local plugin test dependency package is not updated.
Great.
For stable, I think to leave it here better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4386: 1.add es agent resthighlevelclient of ClusterClientInstrumentation.2.…
URL: https://github.com/apache/skywalking/pull/4386#issuecomment-588577687
> checked CI fail is configuration-nacos fails
> <img alt="WX20200220-092041@2x" width="1088" src="https://user-images.githubusercontent.com/2892433/74891835-f5c04200-53c2-11ea-8654-1c66cd4a10a5.png">
>
> ElasticSearch6_7 is es6 test： docker startup failure! but my local is passed.
> <img alt="WX20200220-092231@2x" width="1428" src="https://user-images.githubusercontent.com/2892433/74891841-f953c900-53c2-11ea-8b41-ef4c9f049e14.png">
>
> <img alt="WX20200220-092722@2x" width="871" src="https://user-images.githubusercontent.com/2892433/74891934-3b7d0a80-53c3-11ea-8303-990e1abc8861.png">
>
> FYI @dmsolr
Found the reason, removeOnExit caused, my local plugin test dependency package is not updated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services