Skip to content

Commit

Permalink
HPCC-31343 Code review changes
Browse files Browse the repository at this point in the history
- Remove unnecessary logcontext methods
- Pass struc param by ref
- Remove debug code

Signed-off-by: Rodrigo Pastrana <[email protected]>
  • Loading branch information
rpastrana committed Mar 1, 2024
1 parent 0fe725e commit f1d5b08
Show file tree
Hide file tree
Showing 8 changed files with 7 additions and 78 deletions.
12 changes: 0 additions & 12 deletions common/thorhelper/thorcommon.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -744,18 +744,6 @@ class CStatsContextLogger : public CSimpleInterfaceOf<IContextLogger>
{
activeSpan->setSpanAttribute(name, value);
}
virtual void recordSpanException(IException * e, bool failed) const override
{
activeSpan->recordException(e, failed);
}
virtual void recordSpanError(const SpanError error, bool spanFailed) const override
{
activeSpan->recordError(error, spanFailed);
}
virtual void setSpanStatus(bool spanFailed, const char * statusMessage) const override
{
activeSpan->setSpanStatus(spanFailed, statusMessage);
}
virtual const char *queryGlobalId() const override
{
return activeSpan->queryGlobalId();
Expand Down
12 changes: 0 additions & 12 deletions roxie/ccd/ccd.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -766,18 +766,6 @@ class ContextLogger : implements IRoxieContextLogger, public CInterface
{
activeSpan->setSpanAttribute(name, value);
}
virtual void recordSpanException(IException * e, bool failed) const override
{
activeSpan->recordException(e, failed);
}
virtual void recordSpanError(const SpanError error, bool spanFailed) const override
{
activeSpan->recordError(error, spanFailed);
}
virtual void setSpanStatus(bool spanFailed, const char * statusMessage) const override
{
activeSpan->setSpanStatus(spanFailed, statusMessage);
}
virtual const char *queryGlobalId() const override
{
return activeSpan->queryGlobalId();
Expand Down
12 changes: 0 additions & 12 deletions roxie/ccd/ccdcontext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1406,18 +1406,6 @@ class CRoxieContextBase : implements IRoxieAgentContext, implements ICodeContext
{
logctx.setSpanAttribute(name, value);
}
virtual void recordSpanException(IException * e, bool failed) const override
{
logctx.recordSpanException(e, failed);
}
virtual void recordSpanError(const SpanError error, bool spanFailed) const override
{
logctx.recordSpanError(error, spanFailed);
}
virtual void setSpanStatus(bool spanFailed, const char * statusMessage) const override
{
logctx.setSpanStatus(spanFailed, statusMessage);
}
virtual const char *queryGlobalId() const override
{
return logctx.queryGlobalId();
Expand Down
16 changes: 2 additions & 14 deletions roxie/ccd/ccdserver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,18 +300,6 @@ class IndirectAgentContext : implements IRoxieAgentContext, public CInterface
{
ctx->setSpanAttribute(name, value);
}
virtual void recordSpanException(IException * e, bool failed) const override
{
ctx->recordSpanException(e, failed);
}
virtual void recordSpanError(const SpanError error, bool spanFailed) const override
{
ctx->recordSpanError(error, spanFailed);
}
virtual void setSpanStatus(bool spanFailed, const char * statusMessage) const override
{
ctx->setSpanStatus(spanFailed, statusMessage);
}
virtual const char *queryGlobalId() const
{
return ctx->queryGlobalId();
Expand Down Expand Up @@ -1464,7 +1452,7 @@ class CRoxieServerActivity : implements CInterfaceOf<IRoxieServerActivity>, impl
{
ctx->setSpanAttribute(name, value);
}
virtual void recordSpanException(IException * e, bool failed = true) const override
/*virtual void recordSpanException(IException * e, bool failed = true) const override
{
ctx->recordSpanException(e, failed);
}
Expand All @@ -1475,7 +1463,7 @@ class CRoxieServerActivity : implements CInterfaceOf<IRoxieServerActivity>, impl
virtual void setSpanStatus(bool spanFailed, const char * statusMessage) const override
{
ctx->setSpanStatus(spanFailed, statusMessage);
}
}*/
virtual const char *queryGlobalId() const override
{
return ctx ? ctx->queryGlobalId() : nullptr;
Expand Down
12 changes: 0 additions & 12 deletions system/jlib/jlog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2897,18 +2897,6 @@ class DummyLogCtx : implements IContextLogger
{
activeSpan->setSpanAttribute(name, value);
}
virtual void recordSpanException(IException * e, bool failed) const override
{
activeSpan->recordException(e, failed);
}
virtual void recordSpanError(const SpanError error, bool spanFailed) const override
{
activeSpan->recordError(error, spanFailed);
}
virtual void setSpanStatus(bool spanFailed, const char * statusMessage) const override
{
activeSpan->setSpanStatus(spanFailed, statusMessage);
}
virtual const char *queryGlobalId() const override
{
return activeSpan->queryGlobalId();
Expand Down
3 changes: 0 additions & 3 deletions system/jlib/jlog.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1277,9 +1277,6 @@ interface jlib_decl IContextLogger : extends IInterface
virtual IProperties * getClientHeaders() const = 0;
virtual void setSpanAttribute(const char *name, const char *value) const = 0;
virtual void setSpanAttribute(const char *name, __uint64 value) const = 0;
virtual void recordSpanException(IException * e, bool failed = true) const = 0;
virtual void recordSpanError(const SpanError error = SpanError(), bool spanFailed = true) const = 0;
virtual void setSpanStatus(bool spanFailed, const char * statusMessage = NO_STATUS_MESSAGE) const = 0;

virtual void recordStatistics(IStatisticGatherer &progress) const = 0;
virtual const LogMsgJobInfo & queryJob() const { return unknownJob; }
Expand Down
16 changes: 4 additions & 12 deletions system/jlib/jtrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,6 @@ class JLogSpanExporter final : public opentelemetry::sdk::trace::SpanExporter
out.appendf(", \"start\": %lld", (long long)(span->GetStartTime().time_since_epoch()).count());
out.appendf(", \"duration\": %lld", (long long)span->GetDuration().count());
out.appendf(", \"status\": \"%s\"", spanStatusToString(span->GetStatus()));
auto description = span->GetDescription().data();
if (!isEmptyString(description))
{
StringBuffer encoded;
encodeJSON(encoded, description);
out.appendf(", \"description\": \"%s\"", encoded.str());
}
out.appendf(", \"kind\": \"%s\"", spanKindToString(span->GetSpanKind()));

if (hasMask(logFlags, SpanLogFlags::LogParentInfo))
{
Expand All @@ -229,7 +221,7 @@ class JLogSpanExporter final : public opentelemetry::sdk::trace::SpanExporter

if (hasMask(logFlags, SpanLogFlags::LogSpanDetails))
{

out.appendf(", \"kind\": \"%s\"", spanKindToString(span->GetSpanKind()));
const char * description = span->GetDescription().data();
if (!isEmptyString(description))
{
Expand All @@ -243,7 +235,7 @@ class JLogSpanExporter final : public opentelemetry::sdk::trace::SpanExporter
if (hasMask(logFlags, SpanLogFlags::LogAttributes))
printAttributes(out, span->GetAttributes());

//if (hasMask(logFlags, SpanLogFlags::LogEvents))
if (hasMask(logFlags, SpanLogFlags::LogEvents))
printEvents(out, span->GetEvents());

if (hasMask(logFlags, SpanLogFlags::LogLinks))
Expand Down Expand Up @@ -750,7 +742,7 @@ class CSpan : public CInterfaceOf<ISpan>
}
}

virtual void recordError(const SpanError error, bool spanFailed)
virtual void recordError(const SpanError & error, bool spanFailed)
{
if (span != nullptr)
{
Expand Down Expand Up @@ -886,7 +878,7 @@ class CNullSpan : public CInterfaceOf<ISpan>
virtual bool isRecording() const { return false; }

virtual void recordException(IException * e, bool spanFailed) override {}
virtual void recordError(const SpanError error, bool spanFailed = true) override {};
virtual void recordError(const SpanError & error, bool spanFailed = true) override {};
virtual void setSpanStatus(bool spanFailed, const char * statusMessage) override {}

virtual const char* queryGlobalId() const override { return nullptr; }
Expand Down
2 changes: 1 addition & 1 deletion system/jlib/jtrace.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ interface ISpan : extends IInterface
virtual void getLogPrefix(StringBuffer & out) const = 0;
virtual bool isRecording() const = 0; // Is it worth adding any events/attributes to this span?
virtual void recordException(IException * e, bool spanFailed = true) = 0;
virtual void recordError(const SpanError error = SpanError(), bool spanFailed = true) = 0;
virtual void recordError(const SpanError & error = SpanError(), bool spanFailed = true) = 0;
virtual void setSpanStatus(bool spanFailed, const char * statusMessage = NO_STATUS_MESSAGE) = 0;

virtual ISpan * createClientSpan(const char * name) = 0;
Expand Down

0 comments on commit f1d5b08

Please sign in to comment.