HPCC-31370 SoapCall Instrumentation code review

- Remove unnecessary commented out code
- Rename setSpanStatus param to spanSucceeded
- Adds escapeScope param
- Adds method to setSpanURL attributes
- Creates new span wrapping socket operations
- Creates soapcall activity level span

Signed-off-by: Rodrigo Pastrana <[email protected]>

common/thorhelper/thorsoapcall.cpp

@@ -254,7 +254,7 @@ class Url : public CInterface, implements IInterface
     }
 };
 
-//if Url was globally accessible we can define this in jtrace instead
+//If the above Url class was centrally located, we could define this in jtrace instead.
 //http span standards documented here: https://opentelemetry.io/docs/specs/semconv/http/http-spans/
 void setSpanURLAttributes(ISpan * clientSpan, const Url & url)
 {
@@ -988,6 +988,7 @@ class CWSCHelper : implements IWSCHelper, public CInterface
                const IContextLogger &_logctx, IRoxieAbortMonitor *_roxieAbortMonitor, WSCType _wscType)
         : logctx(_logctx), outputAllocator(_outputAllocator), clientCert(_clientCert), roxieAbortMonitor(_roxieAbortMonitor)
     {
+        activitySpanScope.setown(logctx.queryActiveSpan()->createInternalSpan(_wscType == STsoap ? "SoapCall Activity": "HTTPCall Activity"));
         wscMode = _wscMode;
         wscType = _wscType;
         done = 0;
@@ -1053,11 +1054,11 @@ class CWSCHelper : implements IWSCHelper, public CInterface
             s.setown(helper->getXpathHintsXml());
             xpathHints.setown(createPTreeFromXMLString(s.get()));
         }
-        VStringBuffer spanName("SoapCallActivity %s", helper->getService());
-        activitySpanScope.setown(logctx.queryActiveSpan()->createInternalSpan(spanName));
+
         if (wscType == STsoap)
         {
             soapaction.set(s.setown(helper->getSoapAction()));
+            activitySpanScope->setSpanAttribute("activity.soapaction", soapaction.str());
             if(soapaction.get() && !isValidHttpValue(soapaction.get()))
                 throw MakeStringException(-1, "SOAPAction value contained illegal characters: %s", soapaction.get());
 
@@ -1105,7 +1106,7 @@ class CWSCHelper : implements IWSCHelper, public CInterface
 
         service.set(s.setown(helper->getService()));
         service.trim();
-
+        activitySpanScope->setSpanAttribute("activity.service", service.str());
         if (wscType == SThttp)
         {
             service.toUpperCase();  //GET/PUT/POST
@@ -1127,9 +1128,9 @@ class CWSCHelper : implements IWSCHelper, public CInterface
 
         OwnedRoxieString hostsString(helper->getHosts());
         const char *hosts = hostsString.get();
-
         if (isEmptyString(hosts))
             throw MakeStringException(0, "%s specified no URLs", getWsCallTypeName(wscType));
+        activitySpanScope->setSpanAttribute("activity.hosts", hosts);
         if (0==strncmp(hosts, "mtls:", 5))
         {
             clientCertIssuer.set("local");
@@ -1202,6 +1203,7 @@ class CWSCHelper : implements IWSCHelper, public CInterface
 
         if (wscMode == SCrow)
         {
+            activitySpanScope->setSpanAttribute("activity.mode", "SCrow");
             numRowThreads = 1;
 
             numUrlThreads = helper->numParallelThreads();
@@ -1214,6 +1216,7 @@ class CWSCHelper : implements IWSCHelper, public CInterface
         }
         else
         {
+            activitySpanScope->setSpanAttribute("activity.mode", "SCdataset");
             unsigned totThreads = helper->numParallelThreads();
             if (totThreads < 1)
                 totThreads = 2; // default to 2 threads
@@ -2454,9 +2457,7 @@ class CWSCAsyncFor : implements IWSCAsyncFor, public CInterface, public CAsyncFo
         unsigned retryInterval = 0;
 
         Url &url = master->urlArray.item(idx);
-
         createHttpRequest(url, request);
-
         unsigned startidx = idx;
         while (!master->aborted)
         {
@@ -2478,9 +2479,8 @@ class CWSCAsyncFor : implements IWSCAsyncFor, public CInterface, public CAsyncFo
 
                     checkTimeLimitExceeded(&remainingMS);  // after ep.set which might make a potentially long getaddrinfo lookup ...
                     if (strieq(url.method, "https"))
-                    {
                         proto = PersistentProtocol::ProtoTLS;
-                    }
+
                     bool shouldClose = false;
                     Owned<ISocket> psock = master->usePersistConnections() ? persistentHandler->getAvailable(&ep, &shouldClose, proto) : nullptr;
                     if (psock)
@@ -2711,7 +2711,6 @@ class CWSCAsyncFor : implements IWSCAsyncFor, public CInterface, public CAsyncFo
                 if (master->usePersistConnections() && isReused)
                     persistentHandler->doneUsing(socket, false);
 
-
                 master->activitySpanScope->recordError(SpanError("Unknown exception in processQuery", -1, true, true));
                 throw MakeStringException(-1, "Unknown exception in processQuery");
             }

dali/ft/filecopy.cpp

@@ -3573,17 +3573,30 @@ void FileSprayer::updateTargetProperties()
             }
 
             // Update @writeCost and @numWrites in subfile properties and update totalWriteCost
-            if (superTgt)
+            if (superTgt && superTgt->numSubFiles() > 0)
             {
                 if (cur.whichOutput != (unsigned)-1)
                 {
-                    unsigned targetPartNum = targets.item(cur.whichOutput).partNum;
-                    IDistributedFile &subfile = superTgt->querySubFile(targetPartNum, true);
-                    DistributedFilePropertyLock lock(&subfile);
+                    IDistributedFile *subfile;
+                    if (superTgt->numSubFiles() > 1)
+                    {
+                        Owned<IFileDescriptor> fDesc = superTgt->getFileDescriptor();
+                        ISuperFileDescriptor *superFDesc = fDesc->querySuperFileDescriptor();
+                        unsigned subfileNum, subFilePartNum;
+                        superFDesc->mapSubPart(targets.item(cur.whichOutput).partNum, subfileNum, subFilePartNum);
+                        subfile = superTgt->querySubPart(subfileNum, subFilePartNum);
+                    }
+                    else
+                    {
+                        // If there is a single subfile, it is not necessary to map part to subfile
+                        // (also, querySuperFileDescriptor return nullptr if num subfile == 1)
+                        subfile = &superTgt->querySubFile(0);
+                    }
+                    DistributedFilePropertyLock lock(subfile);
                     IPropertyTree &subFileProps = lock.queryAttributes();
                     cost_type prevNumWrites = subFileProps.getPropInt64(getDFUQResultFieldName(DFUQRFnumDiskWrites));
                     cost_type prevWriteCost = subFileProps.getPropInt64(getDFUQResultFieldName(DFUQRFwriteCost));
-                    cost_type curWriteCost = calcFileAccessCost(&subfile, curProgress.numWrites, 0);
+                    cost_type curWriteCost = calcFileAccessCost(subfile, curProgress.numWrites, 0);
                     subFileProps.setPropInt64(getDFUQResultFieldName(DFUQRFwriteCost), prevWriteCost + curWriteCost);
                     subFileProps.setPropInt64(getDFUQResultFieldName(DFUQRFnumDiskWrites), prevNumWrites + curProgress.numWrites);
                     totalWriteCost += curWriteCost;
@@ -3786,23 +3799,35 @@ void FileSprayer::updateTargetProperties()
     if (distributedSource)
     {
         IDistributedSuperFile * superSrc = distributedSource->querySuperFile();
-        if (superSrc)
+        if (superSrc && superSrc->numSubFiles() > 0)
         {
+            Owned<IFileDescriptor> fDesc = superSrc->getFileDescriptor();
+            ISuperFileDescriptor *superFDesc = fDesc->querySuperFileDescriptor();
             ForEachItemIn(idx, partition)
             {
                 PartitionPoint & cur = partition.item(idx);
                 OutputProgress & curProgress = progress.item(idx);
-
                 if (cur.whichInput != (unsigned)-1)
                 {
-                    unsigned sourcePartNum = sources.item(cur.whichInput).partNum;
-                    IDistributedFile &subfile = superSrc->querySubFile(sourcePartNum, true);
-                    DistributedFilePropertyLock lock(&subfile);
+                    IDistributedFile *subfile;
+                    if (superFDesc)
+                    {
+                        unsigned subfileNum, subFilePartNum;
+                        superFDesc->mapSubPart(sources.item(cur.whichInput).partNum, subfileNum, subFilePartNum);
+                        subfile = superSrc->querySubPart(subfileNum, subFilePartNum);
+                    }
+                    else
+                    {
+                        // superFDesc==nullptr if there is a single file
+                        // so query the first (and only) subfile
+                        subfile = &superSrc->querySubFile(0);
+                    }
+                    DistributedFilePropertyLock lock(subfile);
                     IPropertyTree &subFileProps = lock.queryAttributes();
                     stat_type prevNumReads = subFileProps.getPropInt64(getDFUQResultFieldName(DFUQRFnumDiskReads), 0);
-                    cost_type legacyReadCost = getLegacyReadCost(subfile.queryAttributes(), &subfile);
+                    cost_type legacyReadCost = getLegacyReadCost(subfile->queryAttributes(), subfile);
                     cost_type prevReadCost = subFileProps.getPropInt64(getDFUQResultFieldName(DFUQRFreadCost), 0);
-                    cost_type curReadCost = calcFileAccessCost(&subfile, 0, curProgress.numReads);
+                    cost_type curReadCost = calcFileAccessCost(subfile, 0, curProgress.numReads);
                     subFileProps.setPropInt64(getDFUQResultFieldName(DFUQRFnumDiskReads), prevNumReads + curProgress.numReads);
                     subFileProps.setPropInt64(getDFUQResultFieldName(DFUQRFreadCost), legacyReadCost + prevReadCost + curReadCost);
                     totalReadCost += curReadCost;

ecl/hql/CMakeLists.txt

@@ -167,6 +167,9 @@ if (CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_CLANG)
 
   #Compiler complains about an unused function yyunput, and make sure that any warnings are not reported as errors
   set_source_files_properties (hqllex.cpp PROPERTIES COMPILE_FLAGS " -Wno-unused-function -Wno-error")
+
+  #Compiler complains about a variable is set but not used 
+  set_source_files_properties (hqlgram.cpp PROPERTIES COMPILE_FLAGS " -Wno-error=unused-but-set-variable -Wno-error=class-memaccess")
 endif()
 
 if (WIN32)

ecl/hql/hqlexpr.cpp

@@ -1146,8 +1146,8 @@ void HqlParseContext::finishMeta(bool isSeparateFile, bool success, bool generat
     {
 #if 0
         IPropertyTree* tos = curMeta().meta;
-# This is disabled as the location of the cache file needs to be
-# in the original location for now.  This may re-visited in the future.
+// This is disabled as the location of the cache file needs to be
+// in the original location for now.  This may re-visited in the future.
         if (isSeparateFile && hasCacheLocation())
         {
             StringBuffer fullName;

ecl/hqlcpp/hqlresource.cpp

@@ -5549,15 +5549,19 @@ void EclResourcer::removeDuplicateIndependentLinks(CSplitterInfo & connections,
                 ResourcerInfo & sinkInfo = *queryResourceInfo(sink);
                 if (allInputsPulledIndependently(sink))
                 {
+#ifdef TRACE_BALANCED
                     unsigned numRemoved = 0;
+#endif
                     for (unsigned j=info.balancedLinks.ordinality()-1; j > i; j--)
                     {
                         CSplitterLink & next = info.balancedLinks.item(j);
                         if (next.hasSource(expr) && next.hasSink(sink))
                         {
                             info.balancedLinks.remove(j);
                             sinkInfo.balancedLinks.zap(next);
+#ifdef TRACE_BALANCED
                             numRemoved++;
+#endif
                         }
                     }
 

esp/src/src-react/components/Frame.tsx

@@ -91,7 +91,7 @@ export const Frame: React.FunctionComponent<FrameProps> = () => {
     }, []);
 
     React.useEffect(() => {
-        document.title = `${showEnvironmentTitle && environmentTitle.length ? environmentTitle : "ECL Watch "}${locationPathname.split("/").join(" | ")}`;
+        document.title = `${(showEnvironmentTitle && environmentTitle) ? environmentTitle : "ECL Watch "}${locationPathname.split("/").join(" | ")}`;
     }, [environmentTitle, locationPathname, showEnvironmentTitle]);
 
     React.useEffect(() => {

esp/src/src-react/components/Title.tsx

@@ -247,11 +247,6 @@ export const DevTitle: React.FunctionComponent<DevTitleProps> = ({
         }
     }, [currentUser]);
 
-    React.useEffect(() => {
-        if (!environmentTitle) return;
-        document.title = environmentTitle;
-    }, [environmentTitle]);
-
     return <div style={{ backgroundColor: titlebarColorSet ? titlebarColor : theme.palette.themeLight }}>
         <BannerMessageBar />
         <Stack horizontal verticalAlign="center" horizontalAlign="space-between">
@@ -264,7 +259,7 @@ export const DevTitle: React.FunctionComponent<DevTitleProps> = ({
                         <Link href="#/activities">
                             <Text variant="large" nowrap block >
                                 <b title="ECL Watch" style={{ color: titlebarColorSet ? Utility.textColor(titlebarColor) : theme.palette.themeDarker }}>
-                                    {showEnvironmentTitle && environmentTitle.length ? environmentTitle : "ECL Watch"}
+                                    {(showEnvironmentTitle && environmentTitle) ? environmentTitle : "ECL Watch"}
                                 </b>
                             </Text>
                         </Link>

helm/hpcc/Chart.yaml

@@ -6,9 +6,9 @@ type: application
 
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
-version: 9.4.41-closedown0
+version: 9.4.43-closedown0
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application.
 
-appVersion: 9.4.41-closedown0
+appVersion: 9.4.43-closedown0

helm/hpcc/templates/_helpers.tpl

@@ -1501,7 +1501,7 @@ kind: Service
 metadata:
   name: {{ $lvars.serviceName | quote }}
   labels:
-    helmVersion: 9.4.41-closedown0
+    helmVersion: 9.4.43-closedown0
     {{- include "hpcc.addStandardLabels" (dict "root" $.root "instance" $lvars.serviceName ) | indent 4 }}
 {{- if $lvars.labels }}
 {{ toYaml $lvars.labels | indent 4 }}

helm/hpcc/templates/dafilesrv.yaml

@@ -51,7 +51,7 @@ spec:
       labels:
         {{- include "hpcc.addStandardLabels" (dict "root" $ "component" "dafilesrv" "name" "dafilesrv" "instance" .name) | indent 8 }}
         server: {{ .name | quote }}
-        helmVersion: 9.4.41-closedown0
+        helmVersion: 9.4.43-closedown0
       annotations:
         checksum/config: {{ $configSHA }}
         {{- include "hpcc.generateAnnotations" $commonCtx | indent 8 }}

helm/hpcc/templates/dali.yaml

@@ -84,7 +84,7 @@ spec:
         run: {{ $dali.name | quote }}
         server: {{ $dali.name | quote }}
         app: dali
-        helmVersion: 9.4.41-closedown0
+        helmVersion: 9.4.43-closedown0
 {{- if hasKey $.Values.global "metrics" }}
  {{- include "hpcc.generateMetricsReporterLabel" $.Values.global.metrics | nindent 8 }}
 {{- end }}

helm/hpcc/templates/dfuserver.yaml

@@ -57,7 +57,7 @@ spec:
         {{- include "hpcc.addStandardLabels" (dict "root" $ "component" "dfuserver" "name" "dfuserver" "instance" .name) | indent 8 }}
         run: {{ .name | quote }}
         accessDali: "yes"
-        helmVersion: 9.4.41-closedown0
+        helmVersion: 9.4.43-closedown0
 {{- if hasKey . "labels" }}
 {{ toYaml .labels | indent 8 }}
 {{- end }}

helm/hpcc/templates/eclagent.yaml

@@ -60,7 +60,7 @@ data:
             {{- include "hpcc.addStandardLabels" (dict "root" $ "component" $apptype "name" "eclagent" "instance" $appJobName "instanceOf" (printf "%s-job" .me.name)) | indent 12 }}
             accessDali: "yes"
             accessEsp: "yes"
-            helmVersion: 9.4.41-closedown0
+            helmVersion: 9.4.43-closedown0
 {{- if hasKey .me "labels" }}
 {{ toYaml .me.labels | indent 12 }}
 {{- end }}
@@ -137,7 +137,7 @@ spec:
         run: {{ .name | quote }}
         accessDali: "yes"
         accessEsp: {{ .useChildProcesses | default false | ternary "yes" "no" | quote }}
-        helmVersion: 9.4.41-closedown0
+        helmVersion: 9.4.43-closedown0
 {{- if hasKey . "labels" }}
 {{ toYaml .labels | indent 8 }}
 {{- end }}

helm/hpcc/templates/eclccserver.yaml

@@ -58,7 +58,7 @@ data:
             {{- include "hpcc.addStandardLabels" (dict "root" $ "component" "eclccserver" "name" "eclccserver" "instance" $compileJobName "instanceOf" (printf "%s-job" .me.name)) | indent 12 }}
             accessDali: "yes"
             accessEsp: "yes"
-            helmVersion: 9.4.41-closedown0
+            helmVersion: 9.4.43-closedown0
 {{- if hasKey .me "labels" }}
 {{ toYaml .me.labels | indent 12 }}
 {{- end }}
@@ -143,7 +143,7 @@ spec:
         run: {{ .name | quote }}
         accessDali: "yes"
         accessEsp: {{ .useChildProcesses | default false | ternary "yes" "no" | quote }}
-        helmVersion: 9.4.41-closedown0
+        helmVersion: 9.4.43-closedown0
 {{- if hasKey . "labels" }}
 {{ toYaml .labels | indent 8 }}
 {{- end }}

helm/hpcc/templates/eclscheduler.yaml

@@ -65,7 +65,7 @@ spec:
         run: {{ .name | quote }}
         accessDali: "yes"
         accessEsp: "no"
-        helmVersion: 9.4.41-closedown0
+        helmVersion: 9.4.43-closedown0
 {{- if hasKey . "labels" }}
 {{ toYaml .labels | indent 8 }}
 {{- end }}

helm/hpcc/templates/esp.yaml

@@ -122,7 +122,7 @@ spec:
         accessSasha: "yes"
 {{- end }}
         app: {{ $application }}
-        helmVersion: 9.4.41-closedown0
+        helmVersion: 9.4.43-closedown0
         {{- include "hpcc.addStandardLabels" (dict "root" $ "name" $application "component" "esp" "instance" .name) | indent 8 }}
 {{- if hasKey $.Values.global "metrics" }}
  {{- include "hpcc.generateMetricsReporterLabel" $.Values.global.metrics | nindent 8 }}

helm/hpcc/templates/localroxie.yaml

@@ -73,7 +73,7 @@ spec:
         server: {{ $servername | quote }}
         accessDali: "yes"
         accessEsp: "yes"
-        helmVersion: 9.4.41-closedown0
+        helmVersion: 9.4.43-closedown0
         {{- include "hpcc.addStandardLabels" (dict "root" $ "component" "roxie-server" "name" "roxie" "instance" $roxie.name) | indent 8 }}
 {{- if hasKey . "labels" }}
 {{ toYaml .labels | indent 8 }}

helm/hpcc/templates/roxie.yaml

@@ -125,7 +125,7 @@ spec:
         {{- include "hpcc.addStandardLabels" (dict "root" $ "component" "topology-server" "name" "roxie" "instance" $commonCtx.toponame) | indent 8 }}
         run: {{ $commonCtx.toponame | quote }}
         roxie-cluster: {{ $roxie.name | quote }}
-        helmVersion: 9.4.41-closedown0
+        helmVersion: 9.4.43-closedown0
 {{- if hasKey $.Values.global "metrics" }}
  {{- include "hpcc.generateMetricsReporterLabel"  $.Values.global.metrics | nindent 8}}
 {{- end }}
@@ -183,7 +183,7 @@ kind: Service
 metadata:
   name: {{ $commonCtx.toponame | quote }}
   labels:
-    helmVersion: 9.4.41-closedown0
+    helmVersion: 9.4.43-closedown0
     {{- include "hpcc.addStandardLabels" (dict "root" $ "component" "topology-server" "name" "roxie" "instance" $commonCtx.toponame) | indent 4 }}
 spec:
   ports:
@@ -245,7 +245,7 @@ spec:
         roxie-cluster: {{ $roxie.name | quote }}
         accessDali: "yes"
         accessEsp: "yes"
-        helmVersion: 9.4.41-closedown0
+        helmVersion: 9.4.43-closedown0
         {{- include "hpcc.addStandardLabels" (dict "root" $ "component" "roxie-server" "name" "roxie" "instance" $servername) | indent 8 }}
 {{- if hasKey $.Values.global "metrics" }}
  {{- include "hpcc.generateMetricsReporterLabel" $.Values.global.metrics | nindent 8}}
@@ -353,7 +353,7 @@ spec:
         roxie-cluster: {{ $roxie.name | quote }}
         accessDali: "yes"
         accessEsp: "yes"
-        helmVersion: 9.4.41-closedown0
+        helmVersion: 9.4.43-closedown0
 {{- if hasKey $.Values.global "metrics" }}
  {{- include "hpcc.generateMetricsReporterLabel" $.Values.global.metrics | nindent 8}}
 {{- end }}