Skip to content

Commit

Permalink
Merge pull request #19342 from jpmcmu/HPCC-32982
Browse files Browse the repository at this point in the history 
HPCC-32982 Add alternative span scopes & OwnedSpanScope refactoring

Reviewed-by: Gavin Halliday <[email protected]>
Merged-by: Gavin Halliday <[email protected]>
  • Loading branch information
@ghalliday
ghalliday authored Dec 16, 2024
2 parents e40bfee + f542263 commit 0bb5480
Show file tree
Hide file tree
Showing 14 changed files with 206 additions and 78 deletions.
2 changes: 1 addition & 1 deletion common/thorhelper/thorsoapcall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion ecl/eclagent/eclagent.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down
2 changes: 1 addition & 1 deletion esp/platform/espcontext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion esp/services/esdl_svc_engine/esdl_binding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion esp/services/ws_ecl/ws_ecl_service.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion esp/services/ws_workunits/ws_workunitsHelpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
20 changes: 10 additions & 10 deletions plugins/fileservices/fileservices.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion roxie/ccd/ccdcontext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions roxie/ccd/ccdlistener.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down
72 changes: 62 additions & 10 deletions system/jlib/jtrace.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down Expand Up @@ -1504,13 +1505,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->querySpanId();
const char* expectedSpanID = span != nullptr ? span->querySpanId() : "0000000000000000";

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.
Expand All @@ -1519,12 +1543,12 @@ void OwnedSpanScope::setown(ISpan * _span)
prevSpan = setThreadedActiveSpan(_span);
}

void OwnedSpanScope::set(ISpan * _span)
void OwnedActiveSpanScope::set(ISpan * _span)
{
setown(LINK(_span));
}

void OwnedSpanScope::clear()
void OwnedActiveSpanScope::clear()
{
if (span)
{
Expand All @@ -1534,7 +1558,35 @@ void OwnedSpanScope::clear()
}
}

OwnedSpanScope::~OwnedSpanScope()
OwnedActiveSpanScope::~OwnedActiveSpanScope()
{
clear();
}

//---------------------------------------------------------------------------------------------------------------------

void OwnedSpanLifetime::setown(ISpan * _span)
{
assertex(_span);
clear();
span.setown(_span);
}

void OwnedSpanLifetime::set(ISpan * _span)
{
setown(LINK(_span));
}

void OwnedSpanLifetime::clear()
{
if (span)
{
span->endSpan();
span.clear();
}
}

OwnedSpanLifetime::~OwnedSpanLifetime()
{
clear();
}
Expand Down
83 changes: 74 additions & 9 deletions system/jlib/jtrace.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,21 +159,85 @@ interface ISpan : extends IInterface
virtual const char* queryLocalId() const = 0;
};

class jlib_decl OwnedSpanScope
//------------------------------------------------------------------------------
// 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
// 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 jlib_decl ActiveSpanScope
{
public:
OwnedSpanScope() = default;
OwnedSpanScope(ISpan * _ptr);
OwnedSpanScope(const OwnedSpanScope& rhs) = delete;
OwnedSpanScope(OwnedSpanScope&& rhs) = default;
~OwnedSpanScope();
// 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 OwnedSpanScope& operator=(ISpan * ptr) = delete;
inline OwnedSpanScope& operator=(const OwnedSpanScope& rhs) = delete;
inline OwnedSpanScope& operator=(OwnedSpanScope&& rhs) = delete;
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 OwnedSpanLifetime
{
public:
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;
};

class jlib_decl OwnedActiveSpanScope
{
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;

inline ISpan * operator -> () const { return span; }
inline operator ISpan *() const { return span; }

void clear();
ISpan * query() const { return span; }
Expand All @@ -185,6 +249,7 @@ class jlib_decl OwnedSpanScope
ISpan * prevSpan = nullptr;
};


extern jlib_decl IProperties * getClientHeaders(const ISpan * span);
extern jlib_decl IProperties * getSpanContext(const ISpan * span);

Expand Down
Loading

0 comments on commit 0bb5480

Please sign in to comment.