Merge pull request #18313 from ghalliday/issue31306

HPCC-31306 Add support for non-string span attributes Reviewed-By: Rodrigo Pastrana <[email protected]> Merged-by: Gavin Halliday <[email protected]>
hpcc-systems · Feb 21, 2024 · 7c47138 · 7c47138
2 parents b226919 + 018b762
commit 7c47138
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 23 deletions.
diff --git a/common/thorhelper/thorcommon.hpp b/common/thorhelper/thorcommon.hpp
@@ -740,6 +740,10 @@ class CStatsContextLogger : public CSimpleInterfaceOf<IContextLogger>
     {
         activeSpan->setSpanAttribute(name, value);
     }
+    virtual void setSpanAttribute(const char *name, __uint64 value) const override
+    {
+        activeSpan->setSpanAttribute(name, value);
+    }
     virtual const char *queryGlobalId() const override
     {
         return activeSpan->queryGlobalId();

diff --git a/roxie/ccd/ccd.hpp b/roxie/ccd/ccd.hpp
@@ -762,6 +762,10 @@ class ContextLogger : implements IRoxieContextLogger, public CInterface
     {
         activeSpan->setSpanAttribute(name, value);
     }
+    virtual void setSpanAttribute(const char *name, __uint64 value) const override
+    {
+        activeSpan->setSpanAttribute(name, value);
+    }
     virtual const char *queryGlobalId() const override
     {
         return activeSpan->queryGlobalId();

diff --git a/roxie/ccd/ccdcontext.cpp b/roxie/ccd/ccdcontext.cpp
@@ -1402,6 +1402,10 @@ class CRoxieContextBase : implements IRoxieAgentContext, implements ICodeContext
     {
         logctx.setSpanAttribute(name, value);
     }
+    virtual void setSpanAttribute(const char *name, __uint64 value) const override
+    {
+        logctx.setSpanAttribute(name, value);
+    }
     virtual const char *queryGlobalId() const override
     {
         return logctx.queryGlobalId();

diff --git a/roxie/ccd/ccdlistener.cpp b/roxie/ccd/ccdlistener.cpp
@@ -1589,31 +1589,36 @@ class RoxieProtocolMsgContext : implements IHpccProtocolMsgContext, public CInte
             queryFactory->noteQuery(startTime, failed, elapsed, memused, agentsReplyLen, bytesOut);
             queryFactory.clear();
         }
-        if (logctx && logctx->queryTraceLevel() && (logFullQueries() || logctx->intercept))
+        if (logctx)
         {
-            if (queryName.get())
+            if (logctx->queryTraceLevel() && (logFullQueries() || logctx->intercept))
             {
-                StringBuffer s;
-                logctx->getStats(s);
-
-                const char * callerId = logctx->queryCallerId();
-                StringBuffer txIds;
-                if (!isEmptyString(callerId))
-                    txIds.appendf("caller: %s", callerId);
-                const char *localId = logctx->queryLocalId();
-                if (localId && *localId)
+                if (queryName.get())
                 {
+                    StringBuffer s;
+                    logctx->getStats(s);
+
+                    const char * callerId = logctx->queryCallerId();
+                    StringBuffer txIds;
+                    if (!isEmptyString(callerId))
+                        txIds.appendf("caller: %s", callerId);
+                    const char *localId = logctx->queryLocalId();
+                    if (localId && *localId)
+                    {
+                        if (txIds.length())
+                            txIds.append(", ");
+                        txIds.append("local: ").append(localId);
+                    }
                     if (txIds.length())
-                        txIds.append(", ");
-                    txIds.append("local: ").append(localId);
+                        txIds.insert(0, '[').append(']');
+                    if (requestArraySize > 1)
+                        logctx->CTXLOG("COMPLETE: %s(x%u) %s%s from %s complete in %u msecs memory=%u Mb priority=%d agentsreply=%u duplicatePackets=%u resentPackets=%u resultsize=%u continue=%d%s", queryName.get(), requestArraySize, uid.get(), txIds.str(), peer, elapsed, memused, getQueryPriority(), agentsReplyLen, agentsDuplicates, agentsResends, bytesOut, continuationNeeded, s.str());
+                    else
+                        logctx->CTXLOG("COMPLETE: %s %s%s from %s complete in %u msecs memory=%u Mb priority=%d agentsreply=%u duplicatePackets=%u resentPackets=%u resultsize=%u continue=%d%s", queryName.get(), uid.get(), txIds.str(), peer, elapsed, memused, getQueryPriority(), agentsReplyLen, agentsDuplicates, agentsResends, bytesOut, continuationNeeded, s.str());
+
                 }
-                if (txIds.length())
-                    txIds.insert(0, '[').append(']');
-                if (requestArraySize > 1)
-                    logctx->CTXLOG("COMPLETE: %s(x%u) %s%s from %s complete in %u msecs memory=%u Mb priority=%d agentsreply=%u duplicatePackets=%u resentPackets=%u resultsize=%u continue=%d%s", queryName.get(), requestArraySize, uid.get(), txIds.str(), peer, elapsed, memused, getQueryPriority(), agentsReplyLen, agentsDuplicates, agentsResends, bytesOut, continuationNeeded, s.str());
-                else
-                    logctx->CTXLOG("COMPLETE: %s %s%s from %s complete in %u msecs memory=%u Mb priority=%d agentsreply=%u duplicatePackets=%u resentPackets=%u resultsize=%u continue=%d%s", queryName.get(), uid.get(), txIds.str(), peer, elapsed, memused, getQueryPriority(), agentsReplyLen, agentsDuplicates, agentsResends, bytesOut, continuationNeeded, s.str());
             }
+            logctx->setSpanAttribute("stats.time_elapsed", elapsed);
         }
     }
 };

diff --git a/roxie/ccd/ccdserver.cpp b/roxie/ccd/ccdserver.cpp
@@ -296,6 +296,10 @@ class IndirectAgentContext : implements IRoxieAgentContext, public CInterface
     {
         ctx->setSpanAttribute(name, value);
     }
+    virtual void setSpanAttribute(const char *name, __uint64 value) const override
+    {
+        ctx->setSpanAttribute(name, value);
+    }
     virtual const char *queryGlobalId() const
     {
         return ctx->queryGlobalId();
@@ -1444,6 +1448,10 @@ class CRoxieServerActivity : implements CInterfaceOf<IRoxieServerActivity>, impl
     {
         ctx->setSpanAttribute(name, value);
     }
+    virtual void setSpanAttribute(const char *name, __uint64 value) const override
+    {
+        ctx->setSpanAttribute(name, value);
+    }
     virtual const char *queryGlobalId() const override
     {
         return ctx ? ctx->queryGlobalId() : nullptr;

diff --git a/system/jlib/jlog.cpp b/system/jlib/jlog.cpp
@@ -2893,6 +2893,10 @@ class DummyLogCtx : implements IContextLogger
     {
         activeSpan->setSpanAttribute(name, value);
     }
+    virtual void setSpanAttribute(const char *name, __uint64 value) const override
+    {
+        activeSpan->setSpanAttribute(name, value);
+    }
     virtual const char *queryGlobalId() const override
     {
         return activeSpan->queryGlobalId();

diff --git a/system/jlib/jlog.hpp b/system/jlib/jlog.hpp
@@ -1276,6 +1276,7 @@ interface jlib_decl IContextLogger : extends IInterface
     virtual IProperties * getSpanContext() const = 0;
     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 recordStatistics(IStatisticGatherer &progress) const = 0;
     virtual const LogMsgJobInfo & queryJob() const { return unknownJob; }
 };

diff --git a/system/jlib/jtrace.cpp b/system/jlib/jtrace.cpp
@@ -288,14 +288,42 @@ class JLogSpanExporter final : public opentelemetry::sdk::trace::SpanExporter
                 else
                     first = false;
 
+                const auto & value = kv.second;
                 std::ostringstream attsOS; //used to exploit OTel convenience functions for printing attribute values
-                opentelemetry::exporter::ostream_common::print_value(kv.second, attsOS);
+                opentelemetry::exporter::ostream_common::print_value(value, attsOS);
                 std::string val = attsOS.str();
                 if (val.size() > 0)
                 {
-                    StringBuffer encoded;
-                    encodeJSON(encoded, val.c_str());
-                    out.appendf("\"%s\": \"%s\"", kv.first.c_str(), encoded.str());
+                    switch (value.index())
+                    {
+                    case opentelemetry::sdk::common::kTypeBool:
+                    case opentelemetry::sdk::common::kTypeInt:
+                    case opentelemetry::sdk::common::kTypeUInt:
+                    case opentelemetry::sdk::common::kTypeInt64:
+                    case opentelemetry::sdk::common::kTypeDouble:
+                    case opentelemetry::sdk::common::kTypeUInt64:
+                        out.appendf("\"%s\": %s", kv.first.c_str(), val.c_str());
+                        break;
+                    case opentelemetry::sdk::common::kTypeString:
+                        {
+                            StringBuffer encoded;
+                            encodeJSON(encoded, val.c_str());
+                            out.appendf("\"%s\": \"%s\"", kv.first.c_str(), encoded.str());
+                            break;
+                        }
+                    case opentelemetry::sdk::common::kTypeSpanBool:
+                    case opentelemetry::sdk::common::kTypeSpanInt:
+                    case opentelemetry::sdk::common::kTypeSpanUInt:
+                    case opentelemetry::sdk::common::kTypeSpanInt64:
+                    case opentelemetry::sdk::common::kTypeSpanDouble:
+                    case opentelemetry::sdk::common::kTypeSpanString:
+                    case opentelemetry::sdk::common::kTypeSpanUInt64:
+                    case opentelemetry::sdk::common::kTypeSpanByte:
+                        //MORE: These should be output as lists of values.
+                        //Implement when we have a need.
+                    default:
+                        UNIMPLEMENTED;
+                    }
                 }
             }
             out.append(" }");
@@ -558,6 +586,12 @@ class CSpan : public CInterfaceOf<ISpan>
             span->SetAttribute(key, val);
     }
 
+    void setSpanAttribute(const char *name, __uint64 value) override
+    {
+        if (span && !isEmptyString(name))
+            span->SetAttribute(name, value);
+    }
+
     void addSpanEvent(const char * eventName, IProperties * attributes) override 
     {
         if (span && !isEmptyString(eventName))
@@ -790,6 +824,7 @@ class CNullSpan : public CInterfaceOf<ISpan>
     CNullSpan() = default;
 
     virtual void setSpanAttribute(const char * key, const char * val) override {}
+    virtual void setSpanAttribute(const char *name, __uint64 value) override {}
     virtual void setSpanAttributes(const IProperties * attributes) override {}
     virtual void addSpanEvent(const char * eventName) override {}
     virtual void addSpanEvent(const char * eventName, IProperties * attributes) override {};

diff --git a/system/jlib/jtrace.hpp b/system/jlib/jtrace.hpp
@@ -58,6 +58,7 @@ BITMASK_ENUM(SpanFlags);
 interface ISpan : extends IInterface
 {
     virtual void setSpanAttribute(const char * key, const char * val) = 0;
+    virtual void setSpanAttribute(const char *name, __uint64 value) = 0;
     virtual void setSpanAttributes(const IProperties * attributes) = 0;
     virtual void addSpanEvent(const char * eventName) = 0;
     virtual void addSpanEvent(const char * eventName, IProperties * attributes) = 0;