Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HPCC-30405 Jtrace CNullSpan Implementation #17904

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions common/thorhelper/thorcommon.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions esp/services/esdl_svc_engine/esdl_binding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 3 additions & 6 deletions esp/services/ws_ecl/ws_ecl_service.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
4 changes: 4 additions & 0 deletions helm/hpcc/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1102,6 +1102,10 @@
"alwaysCreateGlobalIds": {
"type": "boolean",
"description": "If true, allocate global ids to any requests that do not supply one"
},
"disabled": {
"type": "boolean",
"description": "If true, disable OTel based trace/span generation"
}
},
"additionalProperties": { "type": ["integer", "string", "boolean"] }
Expand Down
4 changes: 4 additions & 0 deletions helm/hpcc/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ global:
logging:
detail: 80

# tracing sets the default tracing information for all components. Can be overridden locally
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ghalliday Added the comment I initially had intended to expose here

tracing:
disabled: false

## resource settings for stub components
#stubInstanceResources:
# memory: "200Mi"
Expand Down
10 changes: 1 addition & 9 deletions roxie/ccd/ccd.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
10 changes: 1 addition & 9 deletions system/jlib/jlog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2829,7 +2829,7 @@ class CRuntimeStatisticCollection;
class DummyLogCtx : implements IContextLogger
{
private:
Owned<ISpan> activeSpan;
Owned<ISpan> activeSpan = getNullSpan();

public:
DummyLogCtx() {}
Expand Down Expand Up @@ -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
Expand Down
42 changes: 38 additions & 4 deletions system/jlib/jtrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -814,7 +841,7 @@ class CTraceManager : implements ITraceManager, public CInterface
Expected Configuration format:
global:
tracing: #optional - tracing enabled by default
disable: true #optional - disable OTel tracing
disabled: true #optional - disable OTel tracing
alwaysCreateGlobalIds : false #optional - should global ids always be created?
exporter: #optional - Controls how trace data is exported/reported
type: OTLP #OS|OTLP|Prometheus|HPCC (default: no export, jlog entry)
Expand Down Expand Up @@ -851,7 +878,7 @@ class CTraceManager : implements ITraceManager, public CInterface
toXML(traceConfig, xml);
DBGLOG("traceConfig tree: %s", xml.str());
#endif
bool disableTracing = traceConfig && traceConfig->getPropBool("@disable", false);
bool disableTracing = traceConfig && traceConfig->getPropBool("@disabled", false);

using namespace opentelemetry::trace;
if (disableTracing)
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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); });
Expand Down
5 changes: 3 additions & 2 deletions system/jlib/jtrace.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
21 changes: 21 additions & 0 deletions testing/unittests/jlibtests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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();
Expand Down
Loading