From 84f16727e82d4da9721a2dbf267fe39d28c9b6e9 Mon Sep 17 00:00:00 2001
From: Gavin Halliday <gavin.halliday@lexisnexis.com>
Date: Tue, 14 Nov 2023 16:59:46 +0000
Subject: [PATCH] HPCC-30432 Add options to control if span start/finish are
 logged

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
---
 helm/examples/tracing/README.md |  3 +++
 helm/hpcc/values.schema.json    | 12 ++++++++-
 system/jlib/jtrace.cpp          | 45 +++++++++++++++++++++++++--------
 system/jlib/jtrace.hpp          |  8 +++---
 4 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/helm/examples/tracing/README.md b/helm/examples/tracing/README.md
index bd1778a4aa5..5caba096c10 100644
--- a/helm/examples/tracing/README.md
+++ b/helm/examples/tracing/README.md
@@ -10,6 +10,9 @@ All configuration options detailed here are part of the HPCC Systems Helm chart,
 ### Tracing cofiguration options
 - disabled - (default: false) disables tracking and reporting of internal traces and spans
 - alwaysCreateGlobalIds - If true, assign newly created global ID to any requests that do not supply one.
+- optAlwaysCreateTraceIds - If true components generate trace/span ids if none are provided by the remote caller.
+- logSpanStart - If true, generate a log entry whenever a span is started (default: false)
+- logSpanFinish - If true, generate a log entry whenever a span is finished (default: true)
 - exporter - Defines The type of exporter in charge of forwarding span data to target back-end
  - type - (defalt: NONE) "OTLP-HTTP" | "OTLP-GRCP" | "OS" | "NONE"
   - OTLP-HTTP
diff --git a/helm/hpcc/values.schema.json b/helm/hpcc/values.schema.json
index c57bd13ae45..d73687e2039 100644
--- a/helm/hpcc/values.schema.json
+++ b/helm/hpcc/values.schema.json
@@ -1114,7 +1114,7 @@
         },
         "alwaysCreateTraceIds": {
           "type": "boolean",
-          "description": "If true, components generate trace/span ids to aid in unit of worktracing"
+          "description": "If true, components generate trace/span ids if none are provided by the remote caller"
         },
         "exporter": {
           "type": "object",
@@ -1135,6 +1135,16 @@
               "description": "Defines the manner in which trace data is processed - in batches, or simple as available"
             }
           }
+        },
+        "logSpanStart": {
+          "type": "boolean",
+          "description": "If true, generate a log entry whenever a span is started",
+          "default": false
+        },
+        "logSpanFinish": {
+          "type": "boolean",
+          "description": "If true, generate a log entry whenever a span is finished",
+          "default": true
         }
       },
       "additionalProperties": { "type": ["integer", "string", "boolean"] }
diff --git a/system/jlib/jtrace.cpp b/system/jlib/jtrace.cpp
index 3914417ce8e..a776a5236dd 100644
--- a/system/jlib/jtrace.cpp
+++ b/system/jlib/jtrace.cpp
@@ -161,9 +161,12 @@ class CSpan : public CInterfaceOf<ISpan>
 
     virtual void beforeDispose() override
     {
-        StringBuffer out;
-        toLog(out);
-        LOG(MCmonitorEvent, "Span end: {%s}", out.str());
+        if (queryTraceManager().logSpanFinish())
+        {
+            StringBuffer out;
+            toLog(out);
+            LOG(MCmonitorEvent, "SpanFinish: {%s}", out.str());
+        }
     }
 
     const char * getSpanID() const
@@ -453,9 +456,12 @@ class CSpan : public CInterfaceOf<ISpan>
             {
                 storeSpanContext();
 
-                StringBuffer out;
-                toLog(out);
-                LOG(MCmonitorEvent, "Span start: {%s}", out.str());
+                if (queryTraceManager().logSpanStart())
+                {
+                    StringBuffer out;
+                    toLog(out);
+                    LOG(MCmonitorEvent, "SpanStart: {%s}", out.str());
+                }
             }
         }
     }
@@ -755,6 +761,8 @@ class CTraceManager : implements ITraceManager, public CInterface
     bool enabled = true;
     bool optAlwaysCreateGlobalIds = false;
     bool optAlwaysCreateTraceIds = true;
+    bool optLogSpanStart = false;
+    bool optLogSpanFinish = true;
     StringAttr moduleName;
 
     //Initializes the global trace provider which is required for all Otel based tracing operations.
@@ -942,6 +950,8 @@ class CTraceManager : implements ITraceManager, public CInterface
             //Non open-telemetry tracing configuration
             if (traceConfig)
             {
+                optLogSpanStart = traceConfig->getPropBool("@logSpanStart", optLogSpanStart);
+                optLogSpanFinish = traceConfig->getPropBool("@logSpanFinish", optLogSpanFinish);
                 optAlwaysCreateGlobalIds = traceConfig->getPropBool("@alwaysCreateGlobalIds", optAlwaysCreateGlobalIds);
                 optAlwaysCreateTraceIds = traceConfig->getPropBool("@alwaysCreateTraceIds", optAlwaysCreateTraceIds);
             }
@@ -991,28 +1001,31 @@ class CTraceManager : implements ITraceManager, public CInterface
         throw makeStringExceptionV(-1, "TraceManager must be intialized!");
     }
 
-    ISpan * createServerSpan(const char * name, StringArray & httpHeaders, SpanFlags flags) const override
+    virtual 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) const override
+    virtual ISpan * createServerSpan(const char * name, const IProperties * httpHeaders, SpanFlags flags) const override
     {
         return new CServerSpan(name, moduleName.get(), httpHeaders, flags);
     }
 
-    const char * getTracedComponentName() const override
+    virtual const char * getTracedComponentName() const override
     {
         return moduleName.get();
     }
 
-    bool isTracingEnabled() const override
+    virtual bool isTracingEnabled() const override
     {
         return enabled;
     }
 
-    virtual bool alwaysCreateGlobalIds() const
+    //These functions are only accessed within this module, and should not really be in the public interface
+    //Something to possibly revisit later - either by passing as flags to the spans, or having a static
+    //function to provide the result of queryTraceManager() as a CTraceManager.
+    virtual bool alwaysCreateGlobalIds() const override
     {
         return optAlwaysCreateGlobalIds;
     }
@@ -1021,6 +1034,16 @@ class CTraceManager : implements ITraceManager, public CInterface
     {
         return optAlwaysCreateTraceIds;
     }
+
+    virtual bool logSpanStart() const override
+    {
+        return optLogSpanStart;
+    }
+
+    virtual bool logSpanFinish() const override
+    {
+        return optLogSpanFinish;
+    }
 };
 
 static Singleton<CTraceManager> theTraceManager;
diff --git a/system/jlib/jtrace.hpp b/system/jlib/jtrace.hpp
index f305fe49829..2c1234b0622 100644
--- a/system/jlib/jtrace.hpp
+++ b/system/jlib/jtrace.hpp
@@ -32,9 +32,9 @@ static constexpr const char *kCallerIdOtelAttributeName = "hpcc.callerid";
 
 enum class SpanFlags : unsigned
 {
-    None            = 0x000000000,
-    EnsureGlobalId  = 0x000000001,
-    EnsureTraceId   = 0x000000010,
+    None            = 0x00000000,
+    EnsureGlobalId  = 0x00000001,
+    EnsureTraceId   = 0x00000002,
 };
 BITMASK_ENUM(SpanFlags);
 
@@ -66,6 +66,8 @@ interface ITraceManager : extends IInterface
     virtual bool isTracingEnabled() const = 0;
     virtual bool alwaysCreateGlobalIds() const = 0;
     virtual bool alwaysCreateTraceIds() const = 0;
+    virtual bool logSpanStart() const = 0;
+    virtual bool logSpanFinish() const = 0;
     virtual const char * getTracedComponentName() const = 0;
  };