HPCC-30301 Code review 1

- Utilizes beforeDispose - Reverts derived destructor pattern - Introduces leaf/branch toString dynamic output Signed-off-by: Rodrigo Pastrana <[email protected]>
hpcc-systems · Sep 22, 2023 · 6a988d6 · 6a988d6
1 parent c003fb0
commit 6a988d6
Showing 1 changed file with 33 additions and 43 deletions.
diff --git a/system/jlib/jtrace.cpp b/system/jlib/jtrace.cpp
@@ -149,33 +149,50 @@ class CSpan : public CInterfaceOf<ISpan>
     CSpan() = delete;
     virtual ~CSpan()
     {
-        //Derived class specific behavior such as toString() should be
-        //called from the destructor of the derived class
         if (span != nullptr)
             span->End();
     }
 
+    virtual void beforeDispose() override
+    {
+        StringBuffer out;
+        toString(out);
+        DBGLOG("Span end: {%s}", out.str());
+    }
+
     ISpan * createClientSpan(const char * name) override;
     ISpan * createInternalSpan(const char * name) override;
 
     void toString(StringBuffer & out) const override
+    {
+        toString(out, true);
+    }
+
+    virtual void toString(StringBuffer & out, bool isLeaf) const
     {
         out.append(",\"Name\":\"").append(name.get()).append("\"");
-        if (!isEmptyString(hpccGlobalId.get()))
-            out.append(",\"HPCCGlobalID\":\"").append(hpccGlobalId.get()).append("\"");
-        if (!isEmptyString(hpccCallerId.get()))
-            out.append(",\"HPCCCallerID\":\"").append(hpccCallerId.get()).append("\"");
+        if (isLeaf)
+        {
+            if (!isEmptyString(hpccGlobalId.get()))
+                out.append(",\"HPCCGlobalID\":\"").append(hpccGlobalId.get()).append("\"");
+            if (!isEmptyString(hpccCallerId.get()))
+                out.append(",\"HPCCCallerID\":\"").append(hpccCallerId.get()).append("\"");
+        }
 
         if (span != nullptr)
         {
-            out.append(",\"SpanID\":\"").append(spanID.get()).append("\"")
-            .append(",\"TraceID\":\"").append(traceID.get()).append("\"")
-            .append(",\"TraceFlags\":\"").append(traceFlags.get()).append("\"");
+            out.append(",\"SpanID\":\"").append(spanID.get()).append("\"");
+
+            if (isLeaf)
+            {
+                out.append(",\"TraceID\":\"").append(traceID.get()).append("\"")
+                 .append(",\"TraceFlags\":\"").append(traceFlags.get()).append("\"");
+            }
 
             if (localParentSpan != nullptr)
             {
                 out.append(",\"LocalParentSpan\":{ ");
-                localParentSpan->toString(out);
+                localParentSpan->toString(out, false);
                 out.append(" }");
             }
         }
@@ -317,18 +334,13 @@ class CSpan : public CInterfaceOf<ISpan>
         localParentSpan = parent;
         if (localParentSpan != nullptr)
             tracerName.set(parent->queryTraceName());
-        //type = spanType;
-
-       // init();
     }
 
     CSpan(const char * spanName, const char * nameOfTracer)
     {
         name.set(spanName);
         localParentSpan = nullptr;
         tracerName.set(nameOfTracer);
-        //type = spanType;
-
     }
 
     void init()
@@ -437,48 +449,34 @@ class CSpan : public CInterfaceOf<ISpan>
 class CInternalSpan : public CSpan
 {
 public:
-    ~CInternalSpan() override
-    {
-        StringBuffer out;
-        toString(out);
-        DBGLOG("Span end: {%s}", out.str());
-    }
-
     CInternalSpan(const char * spanName, CSpan * parent)
     : CSpan(spanName, parent)
     {
         opts.kind = opentelemetry::trace::SpanKind::kInternal;
         init();
     }
 
-    void toString(StringBuffer & out) const override
+    void toString(StringBuffer & out, bool isLeaf) const override
     {
         out.append("\"Type\":\"Internal\"");
-        CSpan::toString(out);
+        CSpan::toString(out, isLeaf);
     }
 };
 
 class CClientSpan : public CSpan
 {
 public:
-    ~CClientSpan() override
-    {
-        StringBuffer out;
-        toString(out);
-        DBGLOG("Span end: {%s}", out.str());
-    }
-
     CClientSpan(const char * spanName, CSpan * parent)
     : CSpan(spanName, parent)
     {
         opts.kind = opentelemetry::trace::SpanKind::kClient;
         init();
     }
 
-    void toString(StringBuffer & out) const override
+    void toString(StringBuffer & out, bool isLeaf) const override
     {
         out.append("\"Type\":\"Client\"");
-        CSpan::toString(out);
+        CSpan::toString(out, isLeaf);
     }
 };
 
@@ -578,14 +576,6 @@ class CServerSpan : public CSpan
     }
 
 public:
-
-    ~CServerSpan() override
-    {
-        StringBuffer out;
-        toString(out);
-        DBGLOG("Span end: {%s}", out.str());
-    }
-
     CServerSpan(const char * spanName, const char * tracerName_, StringArray & httpHeaders)
     : CSpan(spanName, tracerName_)
     {
@@ -602,10 +592,10 @@ class CServerSpan : public CSpan
         init();
     }
 
-    void toString(StringBuffer & out) const override
+    void toString(StringBuffer & out, bool isLeaf) const override
     {
         out.append("\"Type\":\"Server\"");
-        CSpan::toString(out);
+        CSpan::toString(out, isLeaf);
 
         if (remoteParentSpanCtx.IsValid())
         {