Code review changes

system/jlib/jtrace.cpp

@@ -915,12 +915,13 @@ class CNullSpan final : public CInterfaceOf<ISpan>
     virtual void recordError(const SpanError & error) override {};
     virtual void setSpanStatusSuccess(bool spanSucceeded, const char * statusMessage) override {}
 
-    virtual const char * queryTraceId() const override { return nullptr; }
-    virtual const char * querySpanId() const override { return nullptr; }
+    virtual const char * queryTraceId() const override { return "00000000000000000000000000000000"; }
+    virtual const char * querySpanId() const override { return "0000000000000000"; }
 
-    virtual const char* queryGlobalId() const override { return nullptr; }
-    virtual const char* queryCallerId() const override { return nullptr; }
-    virtual const char* queryLocalId() const override { return nullptr; }
+    // Note: GlobalID & LocalID are created from lnuid, which creates 23 char UIDs (16 rand bytes in base58), and uses "1" for zeroes
+    virtual const char* queryGlobalId() const override { return "11111111111111111111111"; }
+    virtual const char* queryCallerId() const override { return ""; }
+    virtual const char* queryLocalId() const override { return "11111111111111111111111"; }
 
     virtual ISpan * createClientSpan(const char * name, const SpanTimeStamp * spanStartTimeStamp = nullptr) override { return getNullSpan(); }
     virtual ISpan * createInternalSpan(const char * name, const SpanTimeStamp * spanStartTimeStamp = nullptr) override { return getNullSpan(); }
@@ -1515,8 +1516,8 @@ ActiveSpanScope::~ActiveSpanScope()
     ISpan* current = queryThreadedActiveSpan();
     if (current != span)
     {
-        const char* currSpanID = current != nullptr ? current->querySpanId() : "null";
-        const char* expectedSpanID = span != nullptr ? span->querySpanId() : "null";
+        const char* currSpanID = current->querySpanId();
+        const char* expectedSpanID = span != nullptr ? span->querySpanId() : "0000000000000000";
 
         IERRLOG("~ActiveSpanScope: threadActiveSpan has changed unexpectedly, expected: %s actual: %s", expectedSpanID, currSpanID);
         return;
@@ -1564,19 +1565,19 @@ OwnedActiveSpanScope::~OwnedActiveSpanScope()
 
 //---------------------------------------------------------------------------------------------------------------------
 
-void OwnedSpanScope::setown(ISpan * _span)
+void OwnedSpanLifetime::setown(ISpan * _span)
 {
     assertex(_span);
     clear();
     span.setown(_span);
 }
 
-void OwnedSpanScope::set(ISpan * _span)
+void OwnedSpanLifetime::set(ISpan * _span)
 {
     setown(LINK(_span));
 }
 
-void OwnedSpanScope::clear()
+void OwnedSpanLifetime::clear()
 {
     if (span)
     {
@@ -1585,7 +1586,7 @@ void OwnedSpanScope::clear()
     }
 }
 
-OwnedSpanScope::~OwnedSpanScope()
+OwnedSpanLifetime::~OwnedSpanLifetime()
 {
     clear();
 }

system/jlib/jtrace.hpp

@@ -166,10 +166,16 @@ interface ISpan : extends IInterface
 // OwnedActiveSpanScope controls the lifetime of its ISpan while ActiveSpanScope
 // does not. In cases where the ISpan will be used from a single thread and within
 // a single scope OwnedActiveSpanScope should be used. For more complicated scenarios,
-// involving multiple threads, time sliced work, etc ActiveSpanScope should be used.
+// involving multiple threads, time sliced work, etc ActiveSpanScope should be used
+// to associate that ISpan with each processing thread / unit of work, while an 
+// OwnedSpanLifetime, likely a class member, should control the ISpan lifetime.
+//
+// When using ActiveSpanScope another class such as OwnedSpanLifetime should be 
+// used to control the lifetime of the ISpan and the referenced ISpans lifetime
+// should be guaranteed to be longer than the ActiveSpanScopes lifetime.
 //------------------------------------------------------------------------------
 
-class ActiveSpanScope
+class jlib_decl ActiveSpanScope
 {
 public:
     // Captures current threadActiveSpan for prevSpan
@@ -192,57 +198,58 @@ class ActiveSpanScope
     ISpan * prevSpan = nullptr;
 };
 
-class jlib_decl OwnedActiveSpanScope
+class jlib_decl OwnedSpanLifetime
 {
 public:
-    OwnedActiveSpanScope() = default;
-    OwnedActiveSpanScope(ISpan * _ptr);
-    OwnedActiveSpanScope(const OwnedActiveSpanScope& rhs) = delete;
-    OwnedActiveSpanScope(OwnedActiveSpanScope&& rhs) = default;
-    ~OwnedActiveSpanScope();
-
-    inline OwnedActiveSpanScope& operator=(ISpan * ptr) = delete;
-    inline OwnedActiveSpanScope& operator=(const OwnedActiveSpanScope& rhs) = delete;
-    inline OwnedActiveSpanScope& operator=(OwnedActiveSpanScope&& rhs) = delete;
+    OwnedSpanLifetime() = default;
+    OwnedSpanLifetime(ISpan * _ptr) : span(_ptr) {}
+    OwnedSpanLifetime(const OwnedSpanLifetime& rhs) = delete;
+    OwnedSpanLifetime(OwnedSpanLifetime&& rhs) = default;
+    ~OwnedSpanLifetime();
 
     inline ISpan * operator -> () const         { return span; }
     inline operator ISpan *() const             { return span; }
 
+    inline OwnedSpanLifetime& operator=(ISpan * ptr) = delete;
+    inline OwnedSpanLifetime& operator=(const OwnedSpanLifetime& rhs) = delete;
+    inline OwnedSpanLifetime& operator=(OwnedSpanLifetime&& rhs) = delete;
+
     void clear();
     ISpan * query() const { return span; }
     void set(ISpan * _span);
     void setown(ISpan * _span);
 
 private:
     Owned<ISpan> span;
-    ISpan * prevSpan = nullptr;
 };
 
-class jlib_decl OwnedSpanScope
+class jlib_decl OwnedActiveSpanScope
 {
 public:
-    OwnedSpanScope() = default;
-    OwnedSpanScope(ISpan * _ptr) : span(_ptr) {}
-    OwnedSpanScope(const OwnedSpanScope& rhs) = delete;
-    OwnedSpanScope(OwnedSpanScope&& rhs) = default;
-    ~OwnedSpanScope();
+    OwnedActiveSpanScope() = default;
+    OwnedActiveSpanScope(ISpan * _ptr);
+    OwnedActiveSpanScope(const OwnedActiveSpanScope& rhs) = delete;
+    OwnedActiveSpanScope(OwnedActiveSpanScope&& rhs) = default;
+    ~OwnedActiveSpanScope();
+
+    inline OwnedActiveSpanScope& operator=(ISpan * ptr) = delete;
+    inline OwnedActiveSpanScope& operator=(const OwnedActiveSpanScope& rhs) = delete;
+    inline OwnedActiveSpanScope& operator=(OwnedActiveSpanScope&& rhs) = delete;
 
     inline ISpan * operator -> () const         { return span; }
     inline operator ISpan *() const             { return span; }
 
-    inline OwnedSpanScope& operator=(ISpan * ptr) = delete;
-    inline OwnedSpanScope& operator=(const OwnedSpanScope& rhs) = delete;
-    inline OwnedSpanScope& operator=(OwnedSpanScope&& rhs) = delete;
-
     void clear();
     ISpan * query() const { return span; }
     void set(ISpan * _span);
     void setown(ISpan * _span);
 
 private:
     Owned<ISpan> span;
+    ISpan * prevSpan = nullptr;
 };
 
+
 extern jlib_decl IProperties * getClientHeaders(const ISpan * span);
 extern jlib_decl IProperties * getSpanContext(const ISpan * span);
 

testing/unittests/jlibtests.cpp

@@ -363,6 +363,16 @@ class JlibTraceTest : public CppUnit::TestFixture
         savedSpan.clear();
     }
 
+    void nullSpanTest()
+    {
+        Owned<ISpan> nullSpan = getNullSpan();
+        CPPUNIT_ASSERT(nullSpan->queryTraceId() != nullptr);
+        CPPUNIT_ASSERT(nullSpan->querySpanId() != nullptr);
+        CPPUNIT_ASSERT(nullSpan->queryGlobalId() != nullptr);
+        CPPUNIT_ASSERT(nullSpan->queryCallerId() != nullptr);
+        CPPUNIT_ASSERT(nullSpan->queryLocalId() != nullptr);
+    }
+
     void testTraceDisableConfig()
     {
         Owned<IPropertyTree> testTree = createPTreeFromYAMLString(disableTracingYaml, ipt_none, ptr_ignoreWhiteSpace, nullptr);
@@ -4190,6 +4200,7 @@ class JLibOpensslAESTest : public CppUnit::TestFixture
                 E->Release();
             }
         }        
+    // Note: Using OpenTelemetry null span 
     }
 
     void test()