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 2021/09/22 09:01:40 UTC
[GitHub] [skywalking-java] heihaozi opened a new pull request #31: Support for Apache HttpClient 5 plug
heihaozi opened a new pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31
### Support for Apache HttpClient 5 plug
- [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
- [ ] Update the documentation to include this new feature.
- [x] Tests(including UT, IT, E2E) are added to verify the new feature.
- [ ] If it's UI related, attach the screenshots below.
---
- [x] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes [apache/skywalking#7763](https://github.com/apache/skywalking/issues/7763).
- [x] Update the [`CHANGES` log](https://github.com/apache/skywalking-java/blob/main/CHANGES.md).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715284077
##########
File path: test/plugin/scenarios/httpclient-5.x-scenario/config/expectedData.yaml
##########
@@ -110,7 +110,7 @@ segmentItems:
spanLayer: Http
startTime: nq 0
endTime: nq 0
- componentId: not null
+ componentId: SpringMVC
Review comment:
This is ID, not name.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715274483
##########
File path: test/plugin/scenarios/httpclient-5.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,120 @@
+# 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.
+segmentItems:
+ - serviceName: httpclient-5.x-scenario
+ segmentSize: ge 4
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/back
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
+ isError: false
+ spanType: Entry
+ peer: ''
+ tags:
+ - {key: url, value: 'http://127.0.0.1:8080/httpclient-5.x/back'}
+ - {key: http.method, value: GET}
+ refs:
+ - {parentEndpoint: httpasyncclient/local, networkAddress: '127.0.0.1:8080',
+ refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not
+ null, parentService: httpclient-5.x-scenario, traceId: not null}
+ skipAnalysis: 'false'
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/back
+ parentSpanId: 0
+ spanId: 1
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
+ isError: false
+ spanType: Exit
+ peer: 127.0.0.1:8080
+ tags:
+ - {key: url, value: 'http://127.0.0.1:8080/httpclient-5.x/back'}
+ - {key: http.method, value: GET}
+ skipAnalysis: 'false'
+ - operationName: httpasyncclient/local
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
+ isError: false
+ spanType: Local
+ peer: ''
+ refs:
+ - {parentEndpoint: /httpclient-5.x/case/asyncGet, networkAddress: '',
+ refType: CrossThread, parentSpanId: 0, parentTraceSegmentId: not null, parentServiceInstance: not
+ null, parentService: httpclient-5.x-scenario, traceId: not null}
+ skipAnalysis: 'false'
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/case/asyncGet
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
+ isError: false
+ spanType: Entry
+ peer: ''
+ tags:
+ - {key: url, value: 'http://127.0.0.1:8080/httpclient-5.x/case/asyncGet'}
+ - {key: http.method, value: GET}
+ refs:
+ - {parentEndpoint: /httpclient-5.x/case/get, networkAddress: '127.0.0.1:8080',
+ refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not
+ null, parentService: httpclient-5.x-scenario, traceId: not null}
+ skipAnalysis: 'false'
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/case/asyncGet
+ parentSpanId: 0
+ spanId: 1
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
+ isError: false
+ spanType: Exit
+ peer: 127.0.0.1:8080
+ tags:
+ - {key: url, value: 'http://127.0.0.1:8080/httpclient-5.x/case/asyncGet'}
+ - {key: http.method, value: GET}
+ skipAnalysis: 'false'
+ - operationName: /httpclient-5.x/case/get
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
Review comment:
Please use real componentId in the expectedData file.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715292573
##########
File path: test/plugin/scenarios/httpclient-5.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,120 @@
+# 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.
+segmentItems:
+ - serviceName: httpclient-5.x-scenario
+ segmentSize: ge 4
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/back
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 14
Review comment:
I expect the tests will fail.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] heihaozi commented on pull request #31: Support for Apache HttpClient 5 plug
Posted by GitBox <gi...@apache.org>.
heihaozi commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-925564023
I am using a windows system and cannot execute the `bash ./test/plugin/run.sh -f ${scenario_name}` command.
Is there an alternative?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #31: Support for Apache HttpClient 5 plug
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-924745941
> OK, Does the integration test create the corresponding project in `\test\plugin\scenarios`?
Yes, do you know where the document is?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #31: Support for Apache HttpClient 5 plug
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-925579219
`/docs/en/setup/service-agent/java-agent/Plugin-list.md` should be updated in every time you add a new plugin.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #31: Support for Apache HttpClient 5 plug
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-924760754
1. https://skywalking.apache.org/docs/skywalking-java/latest/en/setup/service-agent/java-agent/java-plugin-development-guide/
2. https://skywalking.apache.org/docs/skywalking-java/latest/en/setup/service-agent/java-agent/plugin-test/ linked from (1).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #31: Support for Apache HttpClient 5 plug
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-924736057
Please fix CI, and you need to follow plugin dev doc to add integration tests for new plugins, which makes sure this works in both old and new(5.x) releases.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715276778
##########
File path: apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpAsyncClientDoExecuteInterceptor.java
##########
@@ -0,0 +1,59 @@
+/*
+ * 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.httpclient.v5;
+
+import org.apache.hc.core5.concurrent.FutureCallback;
+import org.apache.hc.core5.http.nio.AsyncResponseConsumer;
+import org.apache.hc.core5.http.protocol.HttpContext;
+import org.apache.skywalking.apm.agent.core.context.ContextManager;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.plugin.httpclient.v5.wrapper.AsyncResponseConsumerWrapper;
+import org.apache.skywalking.apm.plugin.httpclient.v5.wrapper.FutureCallbackWrapper;
+
+import java.lang.reflect.Method;
+
+public class HttpAsyncClientDoExecuteInterceptor implements InstanceMethodsAroundInterceptor {
+
+ @Override
+ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ MethodInterceptResult result) throws Throwable {
+ AsyncResponseConsumer consumer = (AsyncResponseConsumer) allArguments[2];
+ HttpContext context = (HttpContext) allArguments[4];
+ FutureCallback callback = (FutureCallback) allArguments[5];
+ allArguments[2] = new AsyncResponseConsumerWrapper(consumer);
+ allArguments[5] = new FutureCallbackWrapper(callback);
+ if (ContextManager.isActive() && !ContextManager.activeSpan().isExit()) {
Review comment:
What is the case there is no `exit span` available? We usually use this kind of check when these are expected.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715292504
##########
File path: test/plugin/scenarios/httpclient-5.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,120 @@
+# 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.
+segmentItems:
+ - serviceName: httpclient-5.x-scenario
+ segmentSize: ge 4
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/back
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 14
+ isError: false
+ spanType: Entry
+ peer: ''
+ tags:
+ - {key: url, value: 'http://127.0.0.1:8080/httpclient-5.x/back'}
+ - {key: http.method, value: GET}
+ refs:
+ - {parentEndpoint: httpasyncclient/local, networkAddress: '127.0.0.1:8080',
+ refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not
+ null, parentService: httpclient-5.x-scenario, traceId: not null}
+ skipAnalysis: 'false'
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/back
+ parentSpanId: 0
+ spanId: 1
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 26
+ isError: false
+ spanType: Exit
+ peer: 127.0.0.1:8080
+ tags:
+ - {key: url, value: 'http://127.0.0.1:8080/httpclient-5.x/back'}
+ - {key: http.method, value: GET}
+ skipAnalysis: 'false'
+ - operationName: httpasyncclient/local
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 26
+ isError: false
+ spanType: Local
+ peer: ''
+ refs:
+ - {parentEndpoint: /httpclient-5.x/case/asyncGet, networkAddress: '',
+ refType: CrossThread, parentSpanId: 0, parentTraceSegmentId: not null, parentServiceInstance: not
+ null, parentService: httpclient-5.x-scenario, traceId: not null}
+ skipAnalysis: 'false'
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/case/asyncGet
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 14
Review comment:
```suggestion
componentId: 1
```
Same here.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #31: Support for Apache HttpClient 5 plug
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-925607030
Your new case has not been added into GitHub action control file, so, no verification will happen in the CI process.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715288255
##########
File path: CHANGES.md
##########
@@ -26,6 +26,7 @@ Release Notes.
rename `plugin.toolkit.log.grpc.reporter.max_message_size` to `log.max_message_size`.
* Implement Kafka Log Reporter. Add config item of `agnt.conf`, `plugin.kafka.topic_logging`.
* Upgrade byte-buddy to 1.11.18
+* Support for Apache HttpClient 5 plug.
Review comment:
```suggestion
* Add plugin to support Apache HttpClient 5.
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #31: Support for Apache HttpClient 5 plug
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-925564674
> I am using a windows system and cannot execute the `bash ./test/plugin/run.sh -f ${scenario_name}` command.
> Is there an alternative?
It only works on Linux or MacOS. Could you find one?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-926301038
> Update suggested change
You should not do this, I didn't change all related things. Please apply changes locally.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] heihaozi commented on pull request #31: Support for Apache HttpClient 5 plug
Posted by GitBox <gi...@apache.org>.
heihaozi commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-924745260
OK, Does the integration test create the corresponding project in `\test\plugin\scenarios`?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] heihaozi commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
heihaozi commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715285406
##########
File path: test/plugin/scenarios/httpclient-5.x-scenario/config/expectedData.yaml
##########
@@ -110,7 +110,7 @@ segmentItems:
spanLayer: Http
startTime: nq 0
endTime: nq 0
- componentId: not null
+ componentId: SpringMVC
Review comment:
OK, I fix it.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] heihaozi commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
heihaozi commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715280674
##########
File path: apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpAsyncClientDoExecuteInterceptor.java
##########
@@ -0,0 +1,59 @@
+/*
+ * 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.httpclient.v5;
+
+import org.apache.hc.core5.concurrent.FutureCallback;
+import org.apache.hc.core5.http.nio.AsyncResponseConsumer;
+import org.apache.hc.core5.http.protocol.HttpContext;
+import org.apache.skywalking.apm.agent.core.context.ContextManager;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.plugin.httpclient.v5.wrapper.AsyncResponseConsumerWrapper;
+import org.apache.skywalking.apm.plugin.httpclient.v5.wrapper.FutureCallbackWrapper;
+
+import java.lang.reflect.Method;
+
+public class HttpAsyncClientDoExecuteInterceptor implements InstanceMethodsAroundInterceptor {
+
+ @Override
+ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ MethodInterceptResult result) throws Throwable {
+ AsyncResponseConsumer consumer = (AsyncResponseConsumer) allArguments[2];
+ HttpContext context = (HttpContext) allArguments[4];
+ FutureCallback callback = (FutureCallback) allArguments[5];
+ allArguments[2] = new AsyncResponseConsumerWrapper(consumer);
+ allArguments[5] = new FutureCallbackWrapper(callback);
+ if (ContextManager.isActive() && !ContextManager.activeSpan().isExit()) {
Review comment:
This is invalid code, I will remove it.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] heihaozi commented on pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
heihaozi commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-925762148
Is the `segments` in `expectedData.yaml` in order?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] heihaozi commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
heihaozi commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715280460
##########
File path: test/plugin/scenarios/httpclient-5.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,120 @@
+# 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.
+segmentItems:
+ - serviceName: httpclient-5.x-scenario
+ segmentSize: ge 4
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/back
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
+ isError: false
+ spanType: Entry
+ peer: ''
+ tags:
+ - {key: url, value: 'http://127.0.0.1:8080/httpclient-5.x/back'}
+ - {key: http.method, value: GET}
+ refs:
+ - {parentEndpoint: httpasyncclient/local, networkAddress: '127.0.0.1:8080',
+ refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not
+ null, parentService: httpclient-5.x-scenario, traceId: not null}
+ skipAnalysis: 'false'
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/back
+ parentSpanId: 0
+ spanId: 1
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
+ isError: false
+ spanType: Exit
+ peer: 127.0.0.1:8080
+ tags:
+ - {key: url, value: 'http://127.0.0.1:8080/httpclient-5.x/back'}
+ - {key: http.method, value: GET}
+ skipAnalysis: 'false'
+ - operationName: httpasyncclient/local
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
+ isError: false
+ spanType: Local
+ peer: ''
+ refs:
+ - {parentEndpoint: /httpclient-5.x/case/asyncGet, networkAddress: '',
+ refType: CrossThread, parentSpanId: 0, parentTraceSegmentId: not null, parentServiceInstance: not
+ null, parentService: httpclient-5.x-scenario, traceId: not null}
+ skipAnalysis: 'false'
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/case/asyncGet
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
+ isError: false
+ spanType: Entry
+ peer: ''
+ tags:
+ - {key: url, value: 'http://127.0.0.1:8080/httpclient-5.x/case/asyncGet'}
+ - {key: http.method, value: GET}
+ refs:
+ - {parentEndpoint: /httpclient-5.x/case/get, networkAddress: '127.0.0.1:8080',
+ refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not
+ null, parentService: httpclient-5.x-scenario, traceId: not null}
+ skipAnalysis: 'false'
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/case/asyncGet
+ parentSpanId: 0
+ spanId: 1
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
+ isError: false
+ spanType: Exit
+ peer: 127.0.0.1:8080
+ tags:
+ - {key: url, value: 'http://127.0.0.1:8080/httpclient-5.x/case/asyncGet'}
+ - {key: http.method, value: GET}
+ skipAnalysis: 'false'
+ - operationName: /httpclient-5.x/case/get
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: not null
Review comment:
OK, I add it.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715292421
##########
File path: test/plugin/scenarios/httpclient-5.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,120 @@
+# 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.
+segmentItems:
+ - serviceName: httpclient-5.x-scenario
+ segmentSize: ge 4
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: /httpclient-5.x/back
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: nq 0
+ endTime: nq 0
+ componentId: 14
Review comment:
As your controller in `org.apache.skywalking`, this wouldn't be traced by SpringMVC plugin, but Tomcat plugin should work.
```suggestion
componentId: 1
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] heihaozi commented on pull request #31: Support for Apache HttpClient 5 plug
Posted by GitBox <gi...@apache.org>.
heihaozi commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-925569401
I will try.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715274783
##########
File path: apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpAsyncClientDoExecuteInterceptor.java
##########
@@ -0,0 +1,59 @@
+/*
+ * 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.httpclient.v5;
+
+import org.apache.hc.core5.concurrent.FutureCallback;
+import org.apache.hc.core5.http.nio.AsyncResponseConsumer;
+import org.apache.hc.core5.http.protocol.HttpContext;
+import org.apache.skywalking.apm.agent.core.context.ContextManager;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.plugin.httpclient.v5.wrapper.AsyncResponseConsumerWrapper;
+import org.apache.skywalking.apm.plugin.httpclient.v5.wrapper.FutureCallbackWrapper;
+
+import java.lang.reflect.Method;
+
+public class HttpAsyncClientDoExecuteInterceptor implements InstanceMethodsAroundInterceptor {
+
+ @Override
+ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ MethodInterceptResult result) throws Throwable {
+ AsyncResponseConsumer consumer = (AsyncResponseConsumer) allArguments[2];
+ HttpContext context = (HttpContext) allArguments[4];
+ FutureCallback callback = (FutureCallback) allArguments[5];
+ allArguments[2] = new AsyncResponseConsumerWrapper(consumer);
+ allArguments[5] = new FutureCallbackWrapper(callback);
+ if (ContextManager.isActive() && !ContextManager.activeSpan().isExit()) {
Review comment:
Why do we need to check `isExit`?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng merged pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#discussion_r715275452
##########
File path: apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpClientDoExecuteInterceptor.java
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.httpclient.v5;
+
+import org.apache.hc.core5.http.ClassicHttpRequest;
+import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.HttpHost;
+import org.apache.skywalking.apm.agent.core.context.CarrierItem;
+import org.apache.skywalking.apm.agent.core.context.ContextCarrier;
+import org.apache.skywalking.apm.agent.core.context.ContextManager;
+import org.apache.skywalking.apm.agent.core.context.tag.Tags;
+import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
+import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
+import org.apache.skywalking.apm.agent.core.logging.api.ILog;
+import org.apache.skywalking.apm.agent.core.logging.api.LogManager;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
+
+import java.lang.reflect.Method;
+import java.net.MalformedURLException;
+import java.net.URL;
+
+public class HttpClientDoExecuteInterceptor implements InstanceMethodsAroundInterceptor {
+
+ private static final ILog LOGGER = LogManager.getLogger(HttpClientDoExecuteInterceptor.class);
+
+ @Override
+ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ MethodInterceptResult result) throws Throwable {
+ if (allArguments[0] == null || allArguments[1] == null) {
+ // illegal args, can't trace. ignore.
+ return;
+ }
+ final HttpHost httpHost = (HttpHost) allArguments[0];
+ ClassicHttpRequest httpRequest = (ClassicHttpRequest) allArguments[1];
+ final ContextCarrier contextCarrier = new ContextCarrier();
+
+ String remotePeer = httpHost.getHostName() + ":" + port(httpHost);
+
+ String uri = httpRequest.getUri().toString();
+ String requestURI = getRequestURI(uri);
+ String operationName = requestURI;
+ AbstractSpan span = ContextManager.createExitSpan(operationName, contextCarrier, remotePeer);
+
+ span.setComponent(ComponentsDefine.HTTPCLIENT);
+ Tags.URL.set(span, buildSpanValue(httpHost, uri));
+ Tags.HTTP.METHOD.set(span, httpRequest.getMethod());
+ SpanLayer.asHttp(span);
+
+ CarrierItem next = contextCarrier.items();
+ while (next.hasNext()) {
+ next = next.next();
+ httpRequest.setHeader(next.getHeadKey(), next.getHeadValue());
+ }
+ }
+
+ @Override
+ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ Object ret) throws Throwable {
+ if (allArguments[0] == null || allArguments[1] == null) {
+ return ret;
+ }
+
+ if (ret != null) {
+ ClassicHttpResponse response = (ClassicHttpResponse) ret;
+
+ int statusCode = response.getCode();
+ AbstractSpan span = ContextManager.activeSpan();
+ if (statusCode >= 400) {
+ span.errorOccurred();
+ Tags.HTTP_RESPONSE_STATUS_CODE.set(span, statusCode);
+ }
+ }
+
+ ContextManager.stopSpan();
+ return ret;
+ }
+
+ @Override
+ public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
+ Class<?>[] argumentsTypes, Throwable t) {
+ AbstractSpan activeSpan = ContextManager.activeSpan();
+ activeSpan.log(t);
+ }
+
+ private String getRequestURI(String uri) throws MalformedURLException {
+ if (isUrl(uri)) {
+ String requestPath = new URL(uri).getPath();
+ return requestPath != null && requestPath.length() > 0 ? requestPath : "/";
+ } else {
+ return uri;
+ }
+ }
+
+ private boolean isUrl(String uri) {
+ String lowerUrl = uri.toLowerCase();
+ return lowerUrl.startsWith("http") || lowerUrl.startsWith("https");
+ }
+
+ private String buildSpanValue(HttpHost httpHost, String uri) {
Review comment:
```suggestion
private String buildURL(HttpHost httpHost, String uri) {
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #31: Support for Apache HttpClient 5 plug
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-925579455
Also `Supported-list.md`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] heihaozi commented on pull request #31: Support for Apache HttpClient 5 plug
Posted by GitBox <gi...@apache.org>.
heihaozi commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-924748906
I don’t know, where is the document?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #31: Support for Apache HttpClient 5 plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #31:
URL: https://github.com/apache/skywalking-java/pull/31#issuecomment-925771847
> Is the `segments` in `expectedData.yaml` in order?
For segments, NO. Because those are not expected, mock traffic many trace segments, we just check whether there is at least one matched.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org