[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration URL: https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187968284 ## File path: apm-protocol/apm-network/src/main/java/org/apache/skywalking/apm/network/trace/component/ComponentsDefine.java ## @@ -84,6 +84,8 @@ public static final OfficialComponent HYSTRIX = new OfficialComponent(29, "Hystrix"); +public static final OfficialComponent SOFARPC = new OfficialComponent(30, "SOFARPC"); Review comment: The component id must equals the `ComponentsDefine.java` defined's, or the component in topology will incorrect This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration URL: https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187964503 ## File path: apm-protocol/apm-network/src/main/java/org/apache/skywalking/apm/network/trace/component/ComponentsDefine.java ## @@ -84,6 +84,8 @@ public static final OfficialComponent HYSTRIX = new OfficialComponent(29, "Hystrix"); +public static final OfficialComponent SOFARPC = new OfficialComponent(30, "SOFARPC"); Review comment: You can set the value to `32`, because the value define in `component-libraries.yml` is `32`. and more you also should change length of `components` filed to `33` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration URL: https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187962668 ## File path: apm-collector/apm-collector-configuration/collector-configuration-provider/src/test/resources/component-libraries.yml ## @@ -124,6 +124,9 @@ Hystrix: Jedis: id: 30 languages: Java +SOFARPC: + id: 31 Review comment: Sorry, it's my fault. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration URL: https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187962693 ## File path: apm-protocol/apm-network/src/main/java/org/apache/skywalking/apm/network/trace/component/ComponentsDefine.java ## @@ -84,6 +84,8 @@ public static final OfficialComponent HYSTRIX = new OfficialComponent(29, "Hystrix"); +public static final OfficialComponent SOFARPC = new OfficialComponent(30, "SOFARPC"); Review comment: I mean the value that it define in `component-libraries.yml`file isn't equals the value define in `ComponentsDefine.java` file This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration URL: https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187962693 ## File path: apm-protocol/apm-network/src/main/java/org/apache/skywalking/apm/network/trace/component/ComponentsDefine.java ## @@ -84,6 +84,8 @@ public static final OfficialComponent HYSTRIX = new OfficialComponent(29, "Hystrix"); +public static final OfficialComponent SOFARPC = new OfficialComponent(30, "SOFARPC"); Review comment: the value that it define in `component-libraries.yml`file isn't equals the value define in `ComponentsDefine.java` file This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration URL: https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187962668 ## File path: apm-collector/apm-collector-configuration/collector-configuration-provider/src/test/resources/component-libraries.yml ## @@ -124,6 +124,9 @@ Hystrix: Jedis: id: 30 languages: Java +SOFARPC: + id: 31 Review comment: Sorry，I mean the value that it define in `component-libraries.yml`file isn't equals the value define in `ComponentsDefine.java` file This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration URL: https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187961412 ## File path: apm-collector/apm-collector-configuration/collector-configuration-provider/src/test/resources/component-libraries.yml ## @@ -124,6 +124,9 @@ Hystrix: Jedis: id: 30 languages: Java +SOFARPC: + id: 31 Review comment: No, the id must unique This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration URL: https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187959970 ## File path: apm-collector/apm-collector-configuration/collector-configuration-provider/src/test/resources/component-libraries.yml ## @@ -124,6 +124,9 @@ Hystrix: Jedis: id: 30 languages: Java +SOFARPC: + id: 31 Review comment: the id isn't equals `ComponentsDefine` define's This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration URL: https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187959970 ## File path: apm-collector/apm-collector-configuration/collector-configuration-provider/src/test/resources/component-libraries.yml ## @@ -124,6 +124,9 @@ Hystrix: Jedis: id: 30 languages: Java +SOFARPC: + id: 31 Review comment: the id isn't equals `ComponentsDefine` define's. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin
integration
URL:
https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187761314
##
File path:
apm-sniffer/apm-sdk-plugin/sofarpc-plugin/src/main/java/org/apache/skywalking/apm/plugin/sofarpc/SofaRpcConsumerInterceptor.java
##
@@ -0,0 +1,131 @@
+/*
+ * 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.sofarpc;
+
+import com.alipay.sofa.rpc.client.ProviderInfo;
+import com.alipay.sofa.rpc.context.RpcInternalContext;
+import com.alipay.sofa.rpc.core.request.SofaRequest;
+import com.alipay.sofa.rpc.core.response.SofaResponse;
+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.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;
+
+/**
+ * @author leizhiyuan
+ */
+public class SofaRpcConsumerInterceptor implements
InstanceMethodsAroundInterceptor {
+@Override
+public void beforeMethod(EnhancedInstance objInst, Method method, Object[]
allArguments,
+ Class[] argumentsTypes, MethodInterceptResult
result) throws Throwable {
+SofaRequest sofaRequest = (SofaRequest) allArguments[0];
+RpcInternalContext rpcContext = RpcInternalContext.getContext();
+boolean isConsumer = rpcContext.isConsumerSide();
+ProviderInfo providerInfo = rpcContext.getProviderInfo();
+
+AbstractSpan span = null;
+
+final String host = providerInfo.getHost();
+final int port = providerInfo.getPort();
+if (isConsumer) {
+final ContextCarrier contextCarrier = new ContextCarrier();
+span =
ContextManager.createExitSpan(generateOperationName(providerInfo, sofaRequest),
contextCarrier, host + ":" + port);
+CarrierItem next = contextCarrier.items();
+while (next.hasNext()) {
+next = next.next();
+rpcContext.getAttachments().put(next.getHeadKey(),
next.getHeadValue());
+}
+}
+Tags.URL.set(span, generateRequestURL(providerInfo, sofaRequest));
+span.setComponent(ComponentsDefine.SOFARPC);
Review comment:
This line maybe cause NullPointException when the isConsumer equals false.
Or remove the judgment if you are sure this instrumentation only call by
consumer side
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin
integration
URL:
https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187761246
##
File path:
apm-sniffer/apm-sdk-plugin/sofarpc-plugin/src/main/java/org/apache/skywalking/apm/plugin/sofarpc/SofaRpcProviderInterceptor.java
##
@@ -0,0 +1,117 @@
+/*
+ * 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.sofarpc;
+
+import com.alipay.sofa.rpc.context.RpcInternalContext;
+import com.alipay.sofa.rpc.core.request.SofaRequest;
+import com.alipay.sofa.rpc.core.response.SofaResponse;
+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.trace.AbstractSpan;
+import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
+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;
+
+/**
+ * @author leizhiyuan
+ */
+public class SofaRpcProviderInterceptor implements
InstanceMethodsAroundInterceptor {
+@Override
+public void beforeMethod(EnhancedInstance objInst, Method method, Object[]
allArguments,
+ Class[] argumentsTypes, MethodInterceptResult
result) throws Throwable {
+SofaRequest sofaRequest = (SofaRequest) allArguments[0];
+RpcInternalContext rpcContext = RpcInternalContext.getContext();
+boolean isConsumer = rpcContext.isConsumerSide();
+
+AbstractSpan span = null;
+
+if (!isConsumer) {
+ContextCarrier contextCarrier = new ContextCarrier();
+CarrierItem next = contextCarrier.items();
+while (next.hasNext()) {
+next = next.next();
+final Object attachment =
rpcContext.getAttachment(next.getHeadKey());
+if (attachment != null) {
+next.setHeadValue(attachment.toString());
+} else {
+next.setHeadValue("");
+}
+}
+span =
ContextManager.createEntrySpan(generateViewPoint(sofaRequest), contextCarrier);
+
+}
+
+span.setComponent(ComponentsDefine.SOFARPC);
Review comment:
This line maybe cause NullPointException when the `isConsumer` equals
`true`. Or remove the judgment if you are sure this instrumentation only call
by provide side.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] ascrutae commented on a change in pull request #1210: Add sofa rpc plugin integration
ascrutae commented on a change in pull request #1210: Add sofa rpc plugin
integration
URL:
https://github.com/apache/incubator-skywalking/pull/1210#discussion_r187761246
##
File path:
apm-sniffer/apm-sdk-plugin/sofarpc-plugin/src/main/java/org/apache/skywalking/apm/plugin/sofarpc/SofaRpcProviderInterceptor.java
##
@@ -0,0 +1,117 @@
+/*
+ * 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.sofarpc;
+
+import com.alipay.sofa.rpc.context.RpcInternalContext;
+import com.alipay.sofa.rpc.core.request.SofaRequest;
+import com.alipay.sofa.rpc.core.response.SofaResponse;
+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.trace.AbstractSpan;
+import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
+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;
+
+/**
+ * @author leizhiyuan
+ */
+public class SofaRpcProviderInterceptor implements
InstanceMethodsAroundInterceptor {
+@Override
+public void beforeMethod(EnhancedInstance objInst, Method method, Object[]
allArguments,
+ Class[] argumentsTypes, MethodInterceptResult
result) throws Throwable {
+SofaRequest sofaRequest = (SofaRequest) allArguments[0];
+RpcInternalContext rpcContext = RpcInternalContext.getContext();
+boolean isConsumer = rpcContext.isConsumerSide();
+
+AbstractSpan span = null;
+
+if (!isConsumer) {
+ContextCarrier contextCarrier = new ContextCarrier();
+CarrierItem next = contextCarrier.items();
+while (next.hasNext()) {
+next = next.next();
+final Object attachment =
rpcContext.getAttachment(next.getHeadKey());
+if (attachment != null) {
+next.setHeadValue(attachment.toString());
+} else {
+next.setHeadValue("");
+}
+}
+span =
ContextManager.createEntrySpan(generateViewPoint(sofaRequest), contextCarrier);
+
+}
+
+span.setComponent(ComponentsDefine.SOFARPC);
Review comment:
This line maybe cause NullPointException when the `isConsumer` equals `true`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
