HPCC-32982 JTrace Add alterntative span socpes & OwnedSpanScope refac…

…toring - Added ActiveSpanScope to track only current active span - Renamed existing OwnedSpanScope to OwnedActiveSpanScope - Added OwnedSpanScope to control only span lifetime Signed-off-by: James McMullan [email protected]
hpcc-systems · Dec 5, 2024 · 5fd4640 · 5fd4640
1 parent 130a3a9
commit 5fd4640
Show file tree

Hide file tree

Showing 14 changed files with 167 additions and 64 deletions.
diff --git a/common/thorhelper/thorsoapcall.cpp b/common/thorhelper/thorsoapcall.cpp
@@ -2629,7 +2629,7 @@ class CWSCAsyncFor : implements IWSCAsyncFor, public CInterface, public CAsyncFo
 
                 StringBuffer spanName;
                 spanName.appendf("%s %s %s:%d", getWsCallTypeName(master->wscType), master->service.str(), url.host.str(), url.port);
-                OwnedSpanScope requestSpan = master->activitySpanScope->createClientSpan(spanName.str());
+                OwnedActiveSpanScope requestSpan = master->activitySpanScope->createClientSpan(spanName.str());
 
                 setSpanURLAttributes(requestSpan, url);
                 requestSpan->setSpanAttribute("request.type", getWsCallTypeName(master->wscType));

diff --git a/ecl/eclagent/eclagent.cpp b/ecl/eclagent/eclagent.cpp
@@ -2156,7 +2156,7 @@ void EclAgent::runProcess(IEclProcess *process)
     allocatorMetaCache.setown(createRowAllocatorCache(this));
 
     Owned<IProperties> traceHeaders = extractTraceDebugOptions(queryWorkUnit());
-    OwnedSpanScope requestSpan = queryTraceManager().createServerSpan("run_workunit", traceHeaders);
+    OwnedActiveSpanScope requestSpan = queryTraceManager().createServerSpan("run_workunit", traceHeaders);
     ContextSpanScope spanScope(updateDummyContextLogger(), requestSpan);
     requestSpan->setSpanAttribute("hpcc.wuid", queryWorkUnit()->queryWuid());
 

diff --git a/esp/platform/espcontext.cpp b/esp/platform/espcontext.cpp
@@ -89,7 +89,7 @@ class CEspContext : public CInterface, implements IEspContext
     Owned<IEspSecureContextEx> m_secureContext;
 
     StringAttr   m_transactionID;
-    OwnedSpanScope m_requestSpan;    // When the context is destroy the span will end.
+    OwnedActiveSpanScope m_requestSpan;    // When the context is destroy the span will end.
     IHttpMessage* m_request;
 
 public:

diff --git a/esp/services/esdl_svc_engine/esdl_binding.cpp b/esp/services/esdl_svc_engine/esdl_binding.cpp
@@ -1572,7 +1572,7 @@ void EsdlServiceImpl::sendTargetSOAP(IEspContext & context,
     }
 
     ISpan * activeSpan = context.queryActiveSpan();
-    OwnedSpanScope clientSpan(activeSpan->createClientSpan("soapcall"));
+    OwnedActiveSpanScope clientSpan(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
@@ -2069,7 +2069,7 @@ int CWsEclBinding::submitWsEclWorkunit(IEspContext & context, WsEclWuInfo &wsinf
     bool noTimeout = false;
 
     ISpan * activeSpan = context.queryActiveSpan();
-    OwnedSpanScope clientSpan(activeSpan->createClientSpan("run_workunit"));
+    OwnedActiveSpanScope clientSpan(activeSpan->createClientSpan("run_workunit"));
     Owned<IProperties> httpHeaders = ::getClientHeaders(clientSpan);
     recordTraceDebugOptions(workunit, httpHeaders);
 

diff --git a/esp/services/ws_workunits/ws_workunitsHelpers.cpp b/esp/services/ws_workunits/ws_workunitsHelpers.cpp
@@ -3793,7 +3793,7 @@ void WsWuHelpers::submitWsWorkunit(IEspContext& context, IConstWorkUnit* cw, con
         }
 
         ISpan * activeSpan = context.queryActiveSpan();
-        OwnedSpanScope clientSpan(activeSpan->createClientSpan("run_workunit"));
+        OwnedActiveSpanScope clientSpan(activeSpan->createClientSpan("run_workunit"));
         Owned<IProperties> httpHeaders = ::getClientHeaders(clientSpan);
         recordTraceDebugOptions(wu, httpHeaders);
 

diff --git a/plugins/fileservices/fileservices.cpp b/plugins/fileservices/fileservices.cpp
@@ -779,7 +779,7 @@ FILESERVICES_API char * FILESERVICES_CALL implementSprayFixed(ICodeContext *ctx,
 
     req->setNoCommon(noCommon);
 
-    OwnedSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Spray Fixed");
+    OwnedActiveSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Spray Fixed");
     clientSpan->setSpanAttribute("destinationFilename", logicalName);
     try
     {
@@ -925,7 +925,7 @@ static char * implementSprayVariable(ICodeContext *ctx, const char * sourceIP, c
         req->setNosplit(true);
     req->setNoCommon(noCommon);
 
-    OwnedSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Spray Variable");
+    OwnedActiveSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Spray Variable");
     clientSpan->setSpanAttribute("destinationFilename", logicalName);
     try
     {
@@ -1110,7 +1110,7 @@ FILESERVICES_API char * FILESERVICES_CALL implementSprayXml(ICodeContext *ctx, c
 
     req->setNoCommon(noCommon);
 
-    OwnedSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Spray Xml");
+    OwnedActiveSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Spray Xml");
     clientSpan->setSpanAttribute("destinationFilename", logicalName);
     try
     {
@@ -1266,7 +1266,7 @@ FILESERVICES_API char * FILESERVICES_CALL implementSprayJson(ICodeContext *ctx,
             req->setSrcPassword(userPw);
     }
 
-    OwnedSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Spray Json");
+    OwnedActiveSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Spray Json");
     clientSpan->setSpanAttribute("destinationFilename", logicalName);
     try
     {
@@ -1358,7 +1358,7 @@ static char * implementDespray(ICodeContext *ctx, const char * sourceLogicalName
     if (maxConnections != -1)
         req->setMaxConnections(maxConnections);
 
-    OwnedSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Despray");
+    OwnedActiveSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Despray");
     clientSpan->setSpanAttribute("sourceFilename", logicalName);
     try
     {
@@ -1457,7 +1457,7 @@ FILESERVICES_API char * FILESERVICES_CALL implementCopy(ICodeContext *ctx, const
         req->setWrap(true);
     req->setExpireDays(expireDays);
 
-    OwnedSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Copy");
+    OwnedActiveSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Copy");
     clientSpan->setSpanAttribute("sourceFilename", sourceLogicalName);
     clientSpan->setSpanAttribute("destinationFilename", destinationLogicalName);
     try
@@ -1565,7 +1565,7 @@ FILESERVICES_API char * FILESERVICES_CALL fsfReplicate(ICodeContext *ctx, const
 
     req->setSourceLogicalName(logicalName.str());
 
-    OwnedSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Spray Fixed");
+    OwnedActiveSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Spray Fixed");
     clientSpan->setSpanAttribute("destinationFilename", logicalName);
     try
     {
@@ -2126,7 +2126,7 @@ FILESERVICES_API char *  FILESERVICES_CALL fsfMonitorLogicalFileName(ICodeContex
     if (shotcount == 0)
         shotcount = -1;
 
-    OwnedSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Monitor Logical Filename");
+    OwnedActiveSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Monitor Logical Filename");
     clientSpan->setSpanAttribute("filename", lfn);
     try
     {
@@ -2167,7 +2167,7 @@ FILESERVICES_API char *  FILESERVICES_CALL fsfMonitorFile(ICodeContext *ctx, con
     if (shotcount == 0)
         shotcount = -1;
 
-    OwnedSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Monitor File");
+    OwnedActiveSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Monitor File");
     clientSpan->setSpanAttribute("filename", filename);
     try
     {
@@ -2503,7 +2503,7 @@ FILESERVICES_API char * FILESERVICES_CALL fsfRemotePull_impl(ICodeContext *ctx,
            req->setSrcpassword(userPw);
     }
 
-    OwnedSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Remote Pull");
+    OwnedActiveSpanScope clientSpan = queryThreadedActiveSpan()->createClientSpan("Dfu Remote Pull");
     clientSpan->setSpanAttribute("sourceFilename", sourceLogicalName);
     clientSpan->setSpanAttribute("destinationFilename", destinationLogicalName);
     try

diff --git a/roxie/ccd/ccdcontext.cpp b/roxie/ccd/ccdcontext.cpp
@@ -1658,7 +1658,7 @@ class CRoxieContextBase : implements IRoxieAgentContext, implements ICodeContext
         }
         else
         {
-            OwnedSpanScope graphScope = queryThreadedActiveSpan()->createInternalSpan(name);
+            OwnedActiveSpanScope graphScope = queryThreadedActiveSpan()->createInternalSpan(name);
             ProcessInfo startProcessInfo;
             if (workUnit || statsWu)
                 startProcessInfo.update(ReadAllInfo);

diff --git a/roxie/ccd/ccdlistener.cpp b/roxie/ccd/ccdlistener.cpp
@@ -1283,7 +1283,7 @@ class RoxieWorkUnitWorker : public RoxieQueryWorker
         Owned<StringContextLogger> logctx = new StringContextLogger(wuid.get());
 
         Owned<IProperties> traceHeaders = extractTraceDebugOptions(wu);
-        OwnedSpanScope requestSpan = queryTraceManager().createServerSpan("run_workunit", traceHeaders);
+        OwnedActiveSpanScope requestSpan = queryTraceManager().createServerSpan("run_workunit", traceHeaders);
         requestSpan->setSpanAttribute("hpcc.wuid", wuid);
         ContextSpanScope spanScope(*logctx, requestSpan);
 
@@ -1464,7 +1464,7 @@ class RoxieProtocolMsgContext : implements IHpccProtocolMsgContext, public CInte
     Owned<CDebugCommandHandler> debugCmdHandler;
     Owned<StringContextLogger> logctx;
     Owned<IQueryFactory> queryFactory;
-    OwnedSpanScope requestSpan;
+    OwnedActiveSpanScope requestSpan;
 
     SocketEndpoint ep;
     time_t startTime;

diff --git a/system/jlib/jtrace.cpp b/system/jlib/jtrace.cpp
@@ -1504,13 +1504,36 @@ ISpan * CTraceManager::createServerSpan(const char * name, const IProperties * h
 
 //---------------------------------------------------------------------------------------------------------------------
 
-OwnedSpanScope::OwnedSpanScope(ISpan * _ptr) : span(_ptr)
+ActiveSpanScope::ActiveSpanScope(ISpan * _ptr) : ActiveSpanScope(_ptr, queryThreadedActiveSpan()) {}
+ActiveSpanScope::ActiveSpanScope(ISpan * _ptr, ISpan * _prev) : span(_ptr), prevSpan(_prev)
+{
+    setThreadedActiveSpan(_ptr);
+}
+
+ActiveSpanScope::~ActiveSpanScope()
+{
+    ISpan* current = queryThreadedActiveSpan();
+    if (current != span)
+    {
+        const char* currSpanID = current != nullptr ? current->querySpanId() : "null";
+        const char* expectedSpanID = span != nullptr ? span->querySpanId() : "null";
+
+        IERRLOG("~ActiveSpanScope: threadActiveSpan has changed unexpectedly, expected: %s actual: %s", expectedSpanID, currSpanID);
+        return;
+    }
+
+    setThreadedActiveSpan(prevSpan);
+}
+
+//---------------------------------------------------------------------------------------------------------------------
+
+OwnedActiveSpanScope::OwnedActiveSpanScope(ISpan * _ptr) : span(_ptr)
 {
     if (_ptr)
         prevSpan = setThreadedActiveSpan(_ptr);
 }
 
-void OwnedSpanScope::setown(ISpan * _span)
+void OwnedActiveSpanScope::setown(ISpan * _span)
 {
     assertex(_span);
     //Just in case the span is already set, ensure it is ended and that the previous span is restored.
@@ -1519,6 +1542,35 @@ void OwnedSpanScope::setown(ISpan * _span)
     prevSpan = setThreadedActiveSpan(_span);
 }
 
+void OwnedActiveSpanScope::set(ISpan * _span)
+{
+    setown(LINK(_span));
+}
+
+void OwnedActiveSpanScope::clear()
+{
+    if (span)
+    {
+        span->endSpan();
+        setThreadedActiveSpan(prevSpan);
+        span.clear();
+    }
+}
+
+OwnedActiveSpanScope::~OwnedActiveSpanScope()
+{
+    clear();
+}
+
+//---------------------------------------------------------------------------------------------------------------------
+
+void OwnedSpanScope::setown(ISpan * _span)
+{
+    assertex(_span);
+    clear();
+    span.setown(_span);
+}
+
 void OwnedSpanScope::set(ISpan * _span)
 {
     setown(LINK(_span));
@@ -1529,7 +1581,6 @@ void OwnedSpanScope::clear()
     if (span)
     {
         span->endSpan();
-        setThreadedActiveSpan(prevSpan);
         span.clear();
     }
 }

diff --git a/system/jlib/jtrace.hpp b/system/jlib/jtrace.hpp
@@ -159,11 +159,64 @@ interface ISpan : extends IInterface
     virtual const char* queryLocalId() const = 0;
 };
 
+//------------------------------------------------------------------------------
+// ActiveSpanScope vs OwnedActiveSpanScope Usage:
+//------------------------------------------------------------------------------
+// The primary difference between OwnedActiveSpanScope and ActiveSpanScope is that
+// 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.
+//------------------------------------------------------------------------------
+
+class ActiveSpanScope
+{
+public:
+    // Captures current threadActiveSpan for prevSpan
+    ActiveSpanScope(ISpan * _ptr);
+    ActiveSpanScope(ISpan * _ptr, ISpan * _prev);
+
+    ActiveSpanScope(const ActiveSpanScope& rhs) = delete;
+    ~ActiveSpanScope();
+
+    inline ISpan * operator -> () const         { return span; }
+    inline operator ISpan *() const             { return span; }
+
+    inline ActiveSpanScope& operator=(ISpan * ptr) = delete;
+    inline ActiveSpanScope& operator=(const ActiveSpanScope& rhs) = delete;
+
+    inline bool operator == (ISpan * _ptr) const       { return span == _ptr; }
+    inline bool operator != (ISpan * _ptr) const       { return span != _ptr; }
+private:
+    ISpan * span = nullptr;
+    ISpan * prevSpan = nullptr;
+};
+
+class jlib_decl OwnedActiveSpanScope
+{
+public:
+    OwnedActiveSpanScope() = default;
+    OwnedActiveSpanScope(ISpan * _ptr);
+    ~OwnedActiveSpanScope();
+
+    inline ISpan * operator -> () const         { return span; }
+    inline operator ISpan *() const             { return span; }
+
+    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
 {
 public:
     OwnedSpanScope() = default;
-    OwnedSpanScope(ISpan * _ptr);
+    OwnedSpanScope(ISpan * _ptr) : span(_ptr) {}
     ~OwnedSpanScope();
 
     inline ISpan * operator -> () const         { return span; }
@@ -176,7 +229,6 @@ class jlib_decl OwnedSpanScope
 
 private:
     Owned<ISpan> span;
-    ISpan * prevSpan = nullptr;
 };
 
 extern jlib_decl IProperties * getClientHeaders(const ISpan * span);