You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2021/05/07 05:59:44 UTC

[skywalking] branch master updated: FIX: NPE when thrift field is nested (#6909)

This is an automated email from the ASF dual-hosted git repository.

wusheng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking.git


The following commit(s) were added to refs/heads/master by this push:
     new 0036ea6  FIX: NPE when thrift field is nested (#6909)
0036ea6 is described below

commit 0036ea674f52f880bb2fd5677f3d5a27b2c66e26
Author: Leon Yang <13...@qq.com>
AuthorDate: Fri May 7 13:59:06 2021 +0800

    FIX: NPE when thrift field is nested (#6909)
    
    * FIX: NPE when field is nested
    
    * ADD: change.md
    
    Co-authored-by: Yang Xin <ya...@fenbi.com>
---
 CHANGES.md                                         |  1 +
 .../thrift/wrapper/ServerInProtocolWrapper.java    | 38 ++++++++++++----------
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index a5ad4f5..7da895a 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -23,6 +23,7 @@ Release Notes.
 * Add an optional agent plugin to support mybatis.
 * Add `spring-cloud-gateway-3.x` optional plugin.
 * Add `okhttp-4.x` plugin.
+* Fix NPE when thrift field is nested in plugin `thrift`
 
 #### OAP-Backend
 * BugFix: filter invalid Envoy access logs whose socket address is empty.
diff --git a/apm-sniffer/apm-sdk-plugin/thrift-plugin/src/main/java/org/apache/skywalking/apm/plugin/thrift/wrapper/ServerInProtocolWrapper.java b/apm-sniffer/apm-sdk-plugin/thrift-plugin/src/main/java/org/apache/skywalking/apm/plugin/thrift/wrapper/ServerInProtocolWrapper.java
index 3cde8b4..cf72057 100644
--- a/apm-sniffer/apm-sdk-plugin/thrift-plugin/src/main/java/org/apache/skywalking/apm/plugin/thrift/wrapper/ServerInProtocolWrapper.java
+++ b/apm-sniffer/apm-sdk-plugin/thrift-plugin/src/main/java/org/apache/skywalking/apm/plugin/thrift/wrapper/ServerInProtocolWrapper.java
@@ -85,25 +85,29 @@ public class ServerInProtocolWrapper extends AbstractProtocolWrapper {
             }
             return readFieldBegin();
         }
-        if (field.type == TType.STOP) {
-            Boolean haveCreatedSpan =
-                    (Boolean) ContextManager.getRuntimeContext().get(HAVE_CREATED_SPAN);
-            if (haveCreatedSpan != null && !haveCreatedSpan) {
-                try {
-                    AbstractSpan span = ContextManager.createEntrySpan(
-                            context.getOperatorName(), createContextCarrier(null));
-                    span.start(context.startTime);
-                    span.tag(TAG_ARGS, context.getArguments());
-                    span.setComponent(ComponentsDefine.THRIFT_SERVER);
-                    SpanLayer.asRPCFramework(span);
-                } catch (Throwable throwable) {
-                    LOGGER.error("Failed to create EntrySpan.", throwable);
-                } finally {
-                    context = null;
-                }
+
+        return field;
+    }
+
+    @Override
+    public void readMessageEnd() throws TException {
+        super.readMessageEnd();
+        Boolean haveCreatedSpan =
+                (Boolean) ContextManager.getRuntimeContext().get(HAVE_CREATED_SPAN);
+        if (haveCreatedSpan != null && !haveCreatedSpan) {
+            try {
+                AbstractSpan span = ContextManager.createEntrySpan(
+                        context.getOperatorName(), createContextCarrier(null));
+                span.start(context.startTime);
+                span.tag(TAG_ARGS, context.getArguments());
+                span.setComponent(ComponentsDefine.THRIFT_SERVER);
+                SpanLayer.asRPCFramework(span);
+            } catch (Throwable throwable) {
+                LOGGER.error("Failed to create EntrySpan.", throwable);
+            } finally {
+                context = null;
             }
         }
-        return field;
     }
 
     private ContextCarrier createContextCarrier(Map<String, String> header) {