HPCC-30405 CNullSpan Implementation

- Defines CNullSpan - Adds cppunits - Defaults activespans to CnullSpan - Removes activespan nullptr checks - Adds default global.tracing.disable - Exposes getNullSpan jlib function - Utilizes getNullSpan jlib function Signed-off-by: Rodrigo Pastrana <[email protected]>
hpcc-systems · Oct 27, 2023 · 2a5853f · 2a5853f
1 parent 7d8b69a
commit 2a5853f
Show file tree

Hide file tree

Showing 9 changed files with 71 additions and 39 deletions.
diff --git a/common/thorhelper/thorcommon.hpp b/common/thorhelper/thorcommon.hpp
@@ -678,7 +678,7 @@ class CStatsContextLogger : public CSimpleInterfaceOf<IContextLogger>
 protected:
     const LogMsgJobInfo job;
     unsigned traceLevel = 1;
-    Owned<ISpan> activeSpan;
+    Owned<ISpan> activeSpan = getNullSpan();
     mutable CRuntimeStatisticCollection stats;
 public:
     CStatsContextLogger(const CRuntimeStatisticCollection  &_mapping, const LogMsgJobInfo & _job=unknownJob) : job(_job), stats(_mapping) {}
@@ -723,26 +723,18 @@ class CStatsContextLogger : public CSimpleInterfaceOf<IContextLogger>
     }
     virtual IProperties * getClientHeaders() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return ::getClientHeaders(activeSpan);
     }
     virtual const char *queryGlobalId() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return activeSpan->queryGlobalId();
     }
     virtual const char *queryLocalId() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return activeSpan->queryLocalId();
     }
     virtual const char *queryCallerId() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return activeSpan->queryCallerId();
     }
     virtual const CRuntimeStatisticCollection &queryStats() const override

diff --git a/esp/services/esdl_svc_engine/esdl_binding.cpp b/esp/services/esdl_svc_engine/esdl_binding.cpp
@@ -1573,8 +1573,7 @@ void EsdlServiceImpl::sendTargetSOAP(IEspContext & context,
 
     Owned<ISpan> clientSpan;
     ISpan * activeSpan = context.queryActiveSpan();
-    if (activeSpan)
-        clientSpan.setown(activeSpan->createClientSpan("soapcall"));
+    clientSpan.setown(activeSpan->createClientSpan("soapcall"));
 
     Owned<IProperties> headers = ::getClientHeaders(clientSpan);
     StringBuffer status;

diff --git a/esp/services/ws_ecl/ws_ecl_service.cpp b/esp/services/ws_ecl/ws_ecl_service.cpp
@@ -1996,12 +1996,9 @@ int CWsEclBinding::submitWsEclWorkunit(IEspContext & context, WsEclWuInfo &wsinf
 
     Owned<ISpan> clientSpan;
     ISpan * activeSpan = context.queryActiveSpan();
-    if (activeSpan)
-    {
-        clientSpan.setown(activeSpan->createClientSpan("wsecl/SubmitWorkunit"));
-        Owned<IProperties> httpHeaders = ::getClientHeaders(clientSpan);
-        recordTraceDebugOptions(workunit, httpHeaders);
-    }
+    clientSpan.setown(activeSpan->createClientSpan("wsecl/SubmitWorkunit"));
+    Owned<IProperties> httpHeaders = ::getClientHeaders(clientSpan);
+    recordTraceDebugOptions(workunit, httpHeaders);
 
     if (httpreq)
     {

diff --git a/helm/hpcc/values.yaml b/helm/hpcc/values.yaml
@@ -25,6 +25,10 @@ global:
   logging:
     detail: 80
 
+  # tracing sets the default tracing information for all components. Can be overridden locally
+  tracing:
+    disable: false
+
   ## resource settings for stub components
   #stubInstanceResources:
   #  memory: "200Mi"

diff --git a/roxie/ccd/ccd.hpp b/roxie/ccd/ccd.hpp
@@ -589,7 +589,7 @@ class ContextLogger : implements IRoxieContextLogger, public CInterface
     mutable bool aborted;
     mutable CIArrayOf<LogItem> log;
 private:
-    Owned<ISpan> activeSpan;
+    Owned<ISpan> activeSpan = getNullSpan();
     ContextLogger(const ContextLogger &);  // Disable copy constructor
 public:
     IMPLEMENT_IINTERFACE;
@@ -748,26 +748,18 @@ class ContextLogger : implements IRoxieContextLogger, public CInterface
     }
     virtual IProperties * getClientHeaders() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return ::getClientHeaders(activeSpan);
     }
     virtual const char *queryGlobalId() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return activeSpan->queryGlobalId();
     }
     virtual const char *queryCallerId() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return activeSpan->queryCallerId();
     }
     virtual const char *queryLocalId() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return activeSpan->queryLocalId();
     }
     virtual const CRuntimeStatisticCollection & queryStats() const override

diff --git a/system/jlib/jlog.cpp b/system/jlib/jlog.cpp
@@ -2829,7 +2829,7 @@ class CRuntimeStatisticCollection;
 class DummyLogCtx : implements IContextLogger
 {
 private:
-    Owned<ISpan> activeSpan;
+    Owned<ISpan> activeSpan = getNullSpan();
 
 public:
     DummyLogCtx() {}
@@ -2874,26 +2874,18 @@ class DummyLogCtx : implements IContextLogger
     }
     virtual IProperties * getClientHeaders() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return ::getClientHeaders(activeSpan);
     }
     virtual const char *queryGlobalId() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return activeSpan->queryGlobalId();
     }
     virtual const char *queryCallerId() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return activeSpan->queryCallerId();
     }
     virtual const char *queryLocalId() const override
     {
-        if (!activeSpan)
-            return nullptr;
         return activeSpan->queryLocalId();
     }
     virtual const CRuntimeStatisticCollection &queryStats() const override

diff --git a/system/jlib/jtrace.cpp b/system/jlib/jtrace.cpp
@@ -536,6 +536,33 @@ class CSpan : public CInterfaceOf<ISpan>
     CSpan * localParentSpan = nullptr;
 };
 
+class CNullSpan : public CInterfaceOf<ISpan>
+{
+public:
+    CNullSpan() = default;
+
+    virtual void setSpanAttribute(const char * key, const char * val) override {}
+    virtual void setSpanAttributes(const IProperties * attributes) override {}
+    virtual void addSpanEvent(const char * eventName) override {}
+    virtual bool getSpanContext(IProperties * ctxProps, bool otelFormatted) const override { return false; }
+
+    virtual void toString(StringBuffer & out) const override {}
+    virtual void toLog(StringBuffer & out) const override {}
+    virtual void getLogPrefix(StringBuffer & out) const override {}
+
+    virtual const char* queryGlobalId() const override { return nullptr; }
+    virtual const char* queryCallerId() const override { return nullptr; }
+    virtual const char* queryLocalId() const override { return nullptr; }
+
+    virtual ISpan * createClientSpan(const char * name) override { return getNullSpan(); }
+    virtual ISpan * createInternalSpan(const char * name) override { return getNullSpan(); }
+
+private:
+    CNullSpan(const CNullSpan&) = delete;
+    CNullSpan& operator=(const CNullSpan&) = delete;
+};
+
+
 class CInternalSpan : public CSpan
 {
 public:
@@ -917,13 +944,13 @@ class CTraceManager : implements ITraceManager, public CInterface
         throw makeStringExceptionV(-1, "TraceManager must be intialized!");
     }
 
-    ISpan * createServerSpan(const char * name, StringArray & httpHeaders, SpanFlags flags) override
+    ISpan * createServerSpan(const char * name, StringArray & httpHeaders, SpanFlags flags) const override
     {
         Owned<IProperties> headerProperties = getHeadersAsProperties(httpHeaders);
         return new CServerSpan(name, moduleName.get(), headerProperties, flags);
     }
 
-    ISpan * createServerSpan(const char * name, const IProperties * httpHeaders, SpanFlags flags) override
+    ISpan * createServerSpan(const char * name, const IProperties * httpHeaders, SpanFlags flags) const override
     {
         return new CServerSpan(name, moduleName.get(), httpHeaders, flags);
     }
@@ -956,6 +983,13 @@ MODULE_EXIT()
     theTraceManager.destroy();
 }
 
+static Owned<ISpan> nullSpan = new CNullSpan();
+
+ISpan * getNullSpan()
+{
+    return nullSpan.getLink();
+}
+
 void initTraceManager(const char * componentName, const IPropertyTree * componentConfig, const IPropertyTree * globalConfig)
 {
     theTraceManager.query([=] () { return new CTraceManager(componentName, componentConfig, globalConfig); });

diff --git a/system/jlib/jtrace.hpp b/system/jlib/jtrace.hpp
@@ -58,13 +58,14 @@ extern jlib_decl IProperties * getClientHeaders(const ISpan * span);
 
 interface ITraceManager : extends IInterface
 {
-    virtual ISpan * createServerSpan(const char * name, StringArray & httpHeaders, SpanFlags flags = SpanFlags::None) = 0;
-    virtual ISpan * createServerSpan(const char * name, const IProperties * httpHeaders, SpanFlags flags = SpanFlags::None) = 0;
+    virtual ISpan * createServerSpan(const char * name, StringArray & httpHeaders, SpanFlags flags = SpanFlags::None) const = 0;
+    virtual ISpan * createServerSpan(const char * name, const IProperties * httpHeaders, SpanFlags flags = SpanFlags::None) const = 0;
     virtual bool isTracingEnabled() const = 0;
     virtual bool alwaysCreateGlobalIds() const = 0;
     virtual const char * getTracedComponentName() const = 0;
  };
 
+extern jlib_decl ISpan * getNullSpan();
 extern jlib_decl void initTraceManager(const char * componentName, const IPropertyTree * componentConfig, const IPropertyTree * globalConfig);
 extern jlib_decl ITraceManager & queryTraceManager();
 

diff --git a/testing/unittests/jlibtests.cpp b/testing/unittests/jlibtests.cpp
@@ -54,6 +54,7 @@ class JlibTraceTest : public CppUnit::TestFixture
         CPPUNIT_TEST(testInvalidPropegatedServerSpan);
         CPPUNIT_TEST(testInternalSpan);
         CPPUNIT_TEST(testMultiNestedSpanTraceOutput);
+        CPPUNIT_TEST(testNullSpan);
     CPPUNIT_TEST_SUITE_END();
 
     const char * simulatedGlobalYaml = R"!!(global:
@@ -153,6 +154,26 @@ class JlibTraceTest : public CppUnit::TestFixture
          initTraceManager("somecomponent", traceConfig, nullptr);
     }
 
+    void testNullSpan()
+    {
+        if (!queryTraceManager().isTracingEnabled())
+        {
+            DBGLOG("Skipping testNullSpan, tracing is not enabled");
+            return;
+        }
+
+        Owned<ISpan> nullSpan = getNullSpan();
+        CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected nullptr nullspan detected", true, nullSpan != nullptr);
+
+        {
+            Owned<IProperties> headers = createProperties(true);
+            nullSpan->getSpanContext(headers, true);
+        }
+
+        Owned<ISpan> nullSpanChild = nullSpan->createClientSpan("nullSpanChild");
+        CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected nullptr nullSpanChild detected", true, nullSpanChild != nullptr);
+    }
+
     void testClientSpan()
     {
         Owned<IProperties> emptyMockHTTPHeaders = createProperties();