Merge pull request #17838 from ghalliday/issue30349

HPCC-30349 Add open telemetry support to esp Reviewed-By: Rodrigo Pastrana <[email protected]> Reviewed-By: Anthony Fishbeck <[email protected]> Merged-by: Gavin Halliday <[email protected]>
hpcc-systems · Sep 28, 2023 · e057926 · e057926
2 parents 66be01a + edd96ce
commit e057926
Show file tree

Hide file tree

Showing 16 changed files with 138 additions and 138 deletions.
diff --git a/common/workunit/workunit.cpp b/common/workunit/workunit.cpp
@@ -14644,6 +14644,9 @@ static_assert(_elements_in(traceDebugOptions) == _elements_in(traceHeaderNames),
 
 IProperties * extractTraceDebugOptions(IConstWorkUnit * source)
 {
+    if (!source)
+        return nullptr;
+
     Owned<IProperties> target = createProperties(true);
     SCMStringBuffer temp;
     for (unsigned i=0; i < _elements_in(traceDebugOptions); i++)
@@ -14662,6 +14665,9 @@ IProperties * extractTraceDebugOptions(IConstWorkUnit * source)
 
 IProperties * deserializeTraceDebugOptions(const IPropertyTree * debugOptions)
 {
+    if (!debugOptions)
+        return nullptr;
+
     Owned<IProperties> target = createProperties(true);
     if (debugOptions)
     {
@@ -14681,6 +14687,9 @@ IProperties * deserializeTraceDebugOptions(const IPropertyTree * debugOptions)
 
 void recordTraceDebugOptions(IWorkUnit * target, const IProperties * source)
 {
+    if (!source)
+        return;
+
     for (unsigned i=0; i < _elements_in(traceDebugOptions); i++)
     {
         const char * headerName = traceHeaderNames[i];

diff --git a/esp/bindings/http/platform/httpservice.cpp b/esp/bindings/http/platform/httpservice.cpp
@@ -257,7 +257,7 @@ int CEspHttpServer::processRequest()
         }
         ctx->addTraceSummaryValue(LogMin, "custom_fields.URL", url.str(), TXSUMMARY_GRP_ENTERPRISE);
 
-        m_response->setHeader(HTTP_HEADER_HPCC_GLOBAL_ID, ctx->getGlobalId());
+        m_response->setHeader(kGlobalIdHttpHeaderName, ctx->getGlobalId());
 
         if(strieq(method.str(), OPTIONS_METHOD))
             return onOptions();

diff --git a/esp/bindings/http/platform/httptransport.cpp b/esp/bindings/http/platform/httptransport.cpp
@@ -1940,17 +1940,11 @@ void CHttpRequest::updateContext()
         m_context->setUseragent(useragent.str());
         getHeader("Accept-Language", acceptLanguage);
         m_context->setAcceptLanguage(acceptLanguage.str());
-        StringBuffer callerId, globalId;
-        getHeader(HTTP_HEADER_HPCC_GLOBAL_ID, globalId);
-        if (globalId.isEmpty())
-            getHeader("hpcc-global-id", globalId);
-        if(globalId.length())
-            m_context->setGlobalId(globalId);
-        getHeader(HTTP_HEADER_HPCC_CALLER_ID, callerId);
-        if (callerId.isEmpty())
-            getHeader("hpcc-caller-id", callerId);
-        if(callerId.length())
-            m_context->setCallerId(callerId);
+
+        //MORE: The previous code would be better off querying httpHeaders...
+        Owned<IProperties> httpHeaders = getHeadersAsProperties(m_headers);
+        Owned<ISpan> requestSpan = queryTraceManager().createServerSpan("request", httpHeaders, SpanFlags::EnsureGlobalId);
+        m_context->setActiveSpan(requestSpan);
     }
 }
 

diff --git a/esp/bindings/http/platform/httptransport.ipp b/esp/bindings/http/platform/httptransport.ipp
@@ -53,8 +53,6 @@ enum MessageLogFlag
 #define HTTP_HEADER_CONTENT_ENCODING  "Content-Encoding"
 #define HTTP_HEADER_TRANSFER_ENCODING "Transfer-Encoding"
 #define HTTP_HEADER_ACCEPT_ENCODING   "Accept-Encoding"
-#define HTTP_HEADER_HPCC_GLOBAL_ID    "Global-Id"
-#define HTTP_HEADER_HPCC_CALLER_ID    "Caller-Id"
 
 class esp_http_decl CHttpMessage : implements IHttpMessage, public CInterface
 {

diff --git a/esp/platform/esp.hpp b/esp/platform/esp.hpp
@@ -235,11 +235,12 @@ interface IEspContext : extends IInterface
     virtual void setRequest(IHttpMessage* req) = 0;
     virtual IHttpMessage* queryRequest() = 0;
 
-    virtual void setGlobalId(const char* id)=0;
-    virtual const char* getGlobalId()=0;
-    virtual void setCallerId(const char* id)=0;
-    virtual const char* getCallerId()=0;
-    virtual const char* getLocalId()=0;
+    virtual void setActiveSpan(ISpan * span)=0;
+    virtual ISpan * queryActiveSpan() const = 0;
+    virtual IProperties * getClientSpanHeaders() const = 0;
+    virtual const char* getGlobalId() const = 0;
+    virtual const char* getCallerId() const = 0;
+    virtual const char* getLocalId() const = 0;
 };
 
 

diff --git a/esp/platform/espcontext.cpp b/esp/platform/espcontext.cpp
@@ -89,9 +89,7 @@ class CEspContext : public CInterface, implements IEspContext
     Owned<IEspSecureContextEx> m_secureContext;
 
     StringAttr   m_transactionID;
-    StringBuffer   m_globalId;
-    StringBuffer   m_localId;
-    StringBuffer   m_callerId;
+    Owned<ISpan> m_activeSpan;
     IHttpMessage* m_request;
 
 public:
@@ -116,9 +114,6 @@ class CEspContext : public CInterface, implements IEspContext
         updateTraceSummaryHeader();
         m_secureContext.setown(secureContext);
         m_SecurityHandler.setSecureContext(secureContext);
-        appendGloballyUniqueId(m_localId);
-        // use localId as globalId unless we receive another
-        m_globalId.set(m_localId);
     }
 
     ~CEspContext()
@@ -630,27 +625,38 @@ class CEspContext : public CInterface, implements IEspContext
     {
         return m_request;
     }
-
-    virtual void setGlobalId(const char* id)
+    virtual void setActiveSpan(ISpan * span) override
+    {
+        m_activeSpan.set(span);
+    }
+    virtual ISpan * queryActiveSpan() const override
     {
-        m_globalId.set(id);
+        return m_activeSpan;
     }
-    virtual const char* getGlobalId()
+    //GH Can these be deleted?
+    virtual const char* getGlobalId() const override
     {
-        return m_globalId.str();
+        if (!m_activeSpan)
+            return nullptr;
+        return m_activeSpan->queryGlobalId();
     }
-    virtual void setCallerId(const char* id)
+    virtual const char* getCallerId() const override
     {
-        m_callerId.set(id);
+        if (!m_activeSpan)
+            return nullptr;
+        return m_activeSpan->queryCallerId();
     }
-    virtual const char* getCallerId()
+    virtual const char* getLocalId() const override
     {
-        return m_callerId.str();
+        if (!m_activeSpan)
+            return nullptr;
+        return m_activeSpan->queryLocalId();
     }
-    // No setLocalId() - it should be set once only when constructed
-    virtual const char* getLocalId()
+    virtual IProperties * getClientSpanHeaders() const override
     {
-        return m_localId.str();
+        if (!m_activeSpan)
+            return nullptr;
+        return ::getClientHeaders(m_activeSpan);
     }
 };
 

diff --git a/esp/services/esdl_svc_engine/esdl_binding.cpp b/esp/services/esdl_svc_engine/esdl_binding.cpp
@@ -1571,9 +1571,12 @@ void EsdlServiceImpl::sendTargetSOAP(IEspContext & context,
         httpclient->setPassword(password.str());
     }
 
-    Owned<IProperties> headers = createProperties();
-    headers->setProp(HTTP_HEADER_HPCC_GLOBAL_ID, context.getGlobalId());
-    headers->setProp(HTTP_HEADER_HPCC_CALLER_ID, context.getLocalId());
+    Owned<ISpan> clientSpan;
+    ISpan * activeSpan = context.queryActiveSpan();
+    if (activeSpan)
+        clientSpan.setown(activeSpan->createClientSpan("soapcall"));
+
+    Owned<IProperties> headers = ::getClientHeaders(clientSpan);
     StringBuffer status;
     StringBuffer clreq(req);
 

diff --git a/esp/services/ws_ecl/ws_ecl_service.cpp b/esp/services/ws_ecl/ws_ecl_service.cpp
@@ -1993,20 +1993,18 @@ int CWsEclBinding::submitWsEclWorkunit(IEspContext & context, WsEclWuInfo &wsinf
     StringAttr wuid(workunit->queryWuid());  // NB queryWuid() not valid after workunit,clear()
 
     bool noTimeout = false;
-    if (httpreq)
+
+    Owned<ISpan> clientSpan;
+    ISpan * activeSpan = context.queryActiveSpan();
+    if (activeSpan)
     {
-        StringBuffer globalId, callerId;
-        wsecl->getHttpGlobalIdHeader(httpreq, globalId);
-        wsecl->getHttpCallerIdHeader(httpreq, callerId);
-        if (globalId.length())
-        {
-            workunit->setDebugValue("GlobalId", globalId.str(), true);
+        clientSpan.setown(activeSpan->createClientSpan("wsecl/SubmitWorkunit"));
+        Owned<IProperties> httpHeaders = ::getClientHeaders(clientSpan);
+        recordTraceDebugOptions(workunit, httpHeaders);
+    }
 
-            StringBuffer localId;
-            appendGloballyUniqueId(localId);
-            workunit->setDebugValue("CallerId", localId.str(), true); //our localId becomes caller id for the next hop
-            DBGLOG("GlobalId: %s, CallerId: %s, LocalId: %s, Wuid: %s", globalId.str(), callerId.str(), localId.str(), wuid.str());
-        }
+    if (httpreq)
+    {
         IProperties *params = httpreq->queryParameters();
         if (params)
             noTimeout = params->getPropBool(".noTimeout", false);
@@ -2087,27 +2085,12 @@ void CWsEclBinding::sendRoxieRequest(const char *target, StringBuffer &req, Stri
         if (!trim)
             url.append("?.trim=0");
 
-        Owned<IProperties> headers;
+        IEspContext * ctx = httpreq->queryContext();
+        Owned<IProperties> headers = ctx->getClientSpanHeaders();
         Owned<IHttpClient> httpclient = httpctx->createHttpClient(NULL, url);
         bool noTimeout = false;
         if (httpreq)
         {
-            StringBuffer globalId, callerId;
-            wsecl->getHttpGlobalIdHeader(httpreq, globalId);
-            wsecl->getHttpCallerIdHeader(httpreq, callerId);
-
-            if (globalId.length())
-            {
-                headers.setown(createProperties());
-                headers->setProp(kGlobalIdHttpHeaderName, globalId);
-
-                StringBuffer localId;
-                appendGloballyUniqueId(localId);
-                if (localId.length())
-                    headers->setProp(kCallerIdHttpHeaderName, localId);
-                DBGLOG("GlobalId: %s, CallerId: %s, LocalId: %s", globalId.str(), callerId.str(), localId.str());
-            }
-
             IProperties *params = httpreq->queryParameters();
             if (params)
                 noTimeout = params->getPropBool(".noTimeout", false);

diff --git a/esp/services/ws_ecl/ws_ecl_service.hpp b/esp/services/ws_ecl/ws_ecl_service.hpp
@@ -120,19 +120,6 @@ class CWsEclService : public CInterface,
         return false;
     }
 
-    StringBuffer &getHttpGlobalIdHeader(CHttpRequest *request, StringBuffer &value)
-    {
-        if (!getHttpIdHeader(request, kGlobalIdHttpHeaderName, value))
-            getHttpIdHeader(request, kLegacyGlobalIdHttpHeaderName, value);
-        return value;
-    }
-    StringBuffer &getHttpCallerIdHeader(CHttpRequest *request, StringBuffer &value)
-    {
-        if (!getHttpIdHeader(request, kCallerIdHttpHeaderName, value))
-            getHttpIdHeader(request, kLegacyCallerIdHttpHeaderName, value);
-        return value;
-    }
-
     bool unsubscribeServiceFromDali() override {return true;}
     bool subscribeServiceToDali() override {return false;}
     bool detachServiceFromDali() override

diff --git a/system/jlib/jprop.cpp b/system/jlib/jprop.cpp
@@ -280,6 +280,16 @@ class CPropertiesBase : implements IPROP, public CInterface
                 properties.remove(propname);
         }
     }
+    virtual void setNonEmptyProp(PTYPE propname, const char *val)
+    {
+        if (propname)
+        {
+            if (!isEmptyString(val))
+                properties.setValue(propname, val);
+            else
+                properties.remove(propname);
+        }
+    }
     virtual void appendProp(PTYPE propname, const char *val)
     {
         if (propname && val)
@@ -402,6 +412,40 @@ IProperties *cloneProperties(const IProperties * source, bool nocase)
     return clone.getClear();
 }
 
+//This works on arrays of string of the form x=y and x: y
+void extractHeaders(IProperties * target, const StringArray & httpHeaders, char separator)
+{
+    StringBuffer key;
+    ForEachItemIn(currentHeaderIndex, httpHeaders)
+    {
+        const char* httpHeader = httpHeaders.item(currentHeaderIndex);
+        if(isEmptyString(httpHeader))
+            continue;
+
+        const char* delineator = strchr(httpHeader, separator);
+        if ((delineator == nullptr) || (delineator == httpHeader))
+            continue;
+
+        const char * value = delineator + 1;
+        while (isspace(*value))
+            value++;
+
+        if (*value)
+        {
+            key.clear().append(delineator - httpHeader, httpHeader);
+            target->setProp(key, value);
+        }
+    }
+}
+
+IProperties * getHeadersAsProperties(const StringArray & httpHeaders, char separator)
+{
+    Owned<IProperties> properties = createProperties(true);
+    extractHeaders(properties, httpHeaders, separator);
+    return properties.getClear();
+}
+
+
 static CProperties *sysProps = NULL;
 
 extern jlib_decl IProperties *querySystemProperties()

diff --git a/system/jlib/jprop.hpp b/system/jlib/jprop.hpp
@@ -43,6 +43,7 @@ interface jlib_decl IPropertiesOf : extends serializable
     virtual const char *queryProp(PTYPE propname) const = 0;
     virtual void setProp(PTYPE propname, int val) = 0;
     virtual void setProp(PTYPE propname, const char *val) = 0;
+    virtual void setNonEmptyProp(PTYPE propname, const char *val) = 0;
     virtual void appendProp(PTYPE propname, const char *val) = 0;
     virtual bool hasProp(PTYPE propname) const = 0;
     virtual PITER *getIterator() const = 0;
@@ -75,6 +76,8 @@ extern jlib_decl IProperties *createProperties(const char *filename, bool nocase
 extern jlib_decl IProperties *cloneProperties(const IProperties * properties, bool nocase = false);
 extern jlib_decl IProperties *querySystemProperties();
 extern jlib_decl IProperties *getSystemProperties();
+extern jlib_decl void extractHeaders(IProperties * target, const StringArray & httpHeaders, char separator = ':');
+extern jlib_decl IProperties * getHeadersAsProperties(const StringArray & httpHeaders, char separator = ':');
 
 #endif