HPCC-30470 codereviewz

- Conditionally exposes jtrace test functions - Corrects typo - Adds comment regarding blind cast of recordable objects Signed-off-by: Rodrigo Pastrana <[email protected]>
hpcc-systems · Dec 7, 2023 · da6ba2f · da6ba2f
1 parent cc7ff82
commit da6ba2f
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 13 deletions.
diff --git a/helm/examples/tracing/README.md b/helm/examples/tracing/README.md
@@ -12,7 +12,7 @@ All configuration options detailed here are part of the HPCC Systems Helm chart,
 - 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.
 - exporter - Defines The type of exporter in charge of forwarding span data to target back-end
-  - type - (defalt: JLOG) "OTLP-HTTP" | "OTLP-GRCP" | "OS" | "JLOG" | "NONE"
+  - type - (default: JLOG) "OTLP-HTTP" | "OTLP-GRCP" | "OS" | "JLOG" | "NONE"
   - JLOG
     - logSpanDetails - Log span details such as description, status, kind
     - logParentInfo  - Log the span's parent info such as ParentSpanId, and TraceState

diff --git a/system/jlib/jtrace.cpp b/system/jlib/jtrace.cpp
@@ -184,6 +184,8 @@ class JLogSpanExporter final : public opentelemetry::sdk::trace::SpanExporter
 
         for (auto &recordable : recordables)
         {
+            //Casting the recordable object to the type of the object that was previously created by
+            //JLogSpanExporter::MakeRecordable() - 
             auto span = std::unique_ptr<opentelemetry::sdk::trace::SpanData>(
             static_cast<opentelemetry::sdk::trace::SpanData *>(recordable.release()));
 
@@ -211,7 +213,10 @@ class JLogSpanExporter final : public opentelemetry::sdk::trace::SpanExporter
                         out.append(", \"parent_span_id\": \"").append(16, parentSpanID).append("\"");
                     }
 
-                    out.appendf(", \"trace_state\": \"%s\"", span->GetSpanContext().trace_state()->ToHeader().c_str());
+                    StringAttr ts;
+                    ts.set(span->GetSpanContext().trace_state()->ToHeader().c_str());
+                    if (!ts.isEmpty())
+                        out.appendf(", \"trace_state\": \"%s\"", span->GetSpanContext().trace_state()->ToHeader().c_str());
                 }
 
                 if (hasMask(logFlags, SpanLogFlags::LogSpanDetails))
@@ -367,6 +372,18 @@ class JLogSpanExporter final : public opentelemetry::sdk::trace::SpanExporter
     std::atomic_bool shutDown;
 };
 
+#ifdef _USE_CPPUNIT
+void testJLogExporterPrintAttributes(StringBuffer & out, const std::unordered_map<std::string, opentelemetry::sdk::common::OwnedAttributeValue> & map, const char * attsContainerName)
+{
+    JLogSpanExporter::printAttributes(out, map, attsContainerName);
+}
+
+void testJLogExporterPrintResources(StringBuffer & out, const opentelemetry::sdk::resource::Resource &resources)
+{
+    JLogSpanExporter::printResources(out, resources);
+}
+#endif
+
 class JLogSpanExporterFactory
 {
 public:

diff --git a/system/jlib/jtrace.hpp b/system/jlib/jtrace.hpp
@@ -17,7 +17,6 @@
 
 #ifndef JTRACE_HPP
 #define JTRACE_HPP
-
 /**
  * @brief This follows open telemetry's span attribute naming conventions
  *  Known HPCC span Keys could be added here
@@ -85,6 +84,14 @@ interface ITraceManager : extends IInterface
 extern jlib_decl ISpan * getNullSpan();
 extern jlib_decl void initTraceManager(const char * componentName, const IPropertyTree * componentConfig, const IPropertyTree * globalConfig);
 extern jlib_decl ITraceManager & queryTraceManager();
+#ifdef _USE_CPPUNIT
+#include "opentelemetry/sdk/common/attribute_utils.h"
+#include "opentelemetry/sdk/resource/resource.h"
+
+extern jlib_decl void testJLogExporterPrintResources(StringBuffer & out, const opentelemetry::sdk::resource::Resource &resources);
+extern jlib_decl void testJLogExporterPrintAttributes(StringBuffer & out, const std::unordered_map<std::string, opentelemetry::sdk::common::OwnedAttributeValue> & map, const char * attsContainerName);
+#endif
+
 
 //The following class is responsible for ensuring that the active span is restored in a context when the scope is exited
 //Use a template class so it can be reused for IContextLogger and IEspContext

diff --git a/testing/unittests/jlibtests.cpp b/testing/unittests/jlibtests.cpp
@@ -36,9 +36,6 @@
 #include "jutil.hpp"
 #include "junicode.hpp"
 
-#include "lnuid.h"
-#undef UNIMPLEMENTED //opentelemetry defines UNIMPLEMENTED
-#include "jtrace.cpp"
 
 #include "unittests.hpp"
 
@@ -114,11 +111,11 @@ class JlibTraceTest : public CppUnit::TestFixture
     void testJTraceJLOGExporterprintAttributes()
     {
         StringBuffer out;
-        JLogSpanExporter::printAttributes(out, {}, "attributes");
+        testJLogExporterPrintAttributes(out, {}, "attributes");
         CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected non-empty printattributes", true, out.length() == 0);
 
 
-        JLogSpanExporter::printAttributes(out, {{"url", "https://localhost"}, {"content-length", 562}, {"content-type", "html/text"}}, "attributes");
+        testJLogExporterPrintAttributes(out, {{"url", "https://localhost"}, {"content-length", 562}, {"content-type", "html/text"}}, "attributes");
         CPPUNIT_ASSERT_EQUAL_MESSAGE("Unexpected empty printattributes", false, out.length() == 0);
 
         Owned<IPropertyTree> jtraceAsTree;
@@ -130,10 +127,6 @@ class JlibTraceTest : public CppUnit::TestFixture
             out.append("}");
 
             jtraceAsTree.setown(createPTreeFromJSONString(out.str()));
-
-            StringBuffer msg;
-            toXML(jtraceAsTree, msg);
-            DBGLOG("#####testJTraceJLOGExporterprintAttributes: %s", msg.str());
         }
         catch (IException *e)
         {
@@ -161,7 +154,7 @@ class JlibTraceTest : public CppUnit::TestFixture
         };
         auto dummyResources = opentelemetry::sdk::resource::Resource::Create(dummyAttributes);
 
-        JLogSpanExporter::printResources(out, dummyResources);
+        testJLogExporterPrintResources(out, dummyResources);
 
         Owned<IPropertyTree> jtraceAsTree;
         try