Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HPCC-31755 Soapcall LOG multi-line separator #19145

Merged
merged 1 commit into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 57 additions & 6 deletions common/thorhelper/thorsoapcall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,40 @@ using roxiemem::OwnedRoxieString;
#define CONNECTION "Connection"

unsigned soapTraceLevel = 1;
static StringBuffer soapSepString;

void setSoapSepString(const char *_soapSepString)
{
soapSepString.set(_soapSepString);
}

static void multiLineAppendReplace(StringBuffer &origStr, StringBuffer &newStr)
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an opt, we could index for '\r' and then append chunks between newlines.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, if it was very time critical, might be worth two pass, so can allocate correct size once, reserve and use regular ptr's instead of calls to StringBuffer, but doubt worth it.

if (origStr.isEmpty())
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we newStr.ensureCapacity(origStr.length()) before starting loop so we dont have to expand it more than once ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if replacement separator was >1 char and existing separator was \n, it would still have to expand more than once.
an initial guess ensureCapacity wouldn't harm, but not sure worth it, unless this is going to intensively used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree preallocate to the original size.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added


newStr.ensureCapacity(origStr.length());

const char *cursor = origStr;
while (*cursor)
{
switch (*cursor)
{
case '\r':
newStr.append(soapSepString);
if ('\n' == *(cursor+1))
cursor++;
break;
case '\n':
newStr.append(soapSepString);
break;
default:
newStr.append(*cursor);
break;
}
++cursor;
}
}

#define WSCBUFFERSIZE 0x10000
#define MAXWSCTHREADS 50 //Max Web Service Call Threads
Expand Down Expand Up @@ -1951,10 +1985,18 @@ class CWSCAsyncFor : implements IWSCAsyncFor, public CInterface, public CAsyncFo
{
if (soapTraceLevel > 6 || master->logXML)
{
if (!contentEncoded)
master->logctx.mCTXLOG("%s: request(%s)", master->wscCallTypeText(), request.str());
StringBuffer contentStr;
if (contentEncoded)
contentStr.append(", content encoded.");
// Only do translation if soapcall LOG option set and soapSepString defined
if ( (master->logXML) && (soapSepString.length() > 0) )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check: I think the master->logXML test can be deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it and added comment - so we only do translation if LOG set in SOAPCALL args

{
StringBuffer request2;
multiLineAppendReplace(request, request2);
master->logctx.CTXLOG("%s: request(%s)%s", master->wscCallTypeText(), request2.str(), contentStr.str());
}
else
master->logctx.mCTXLOG("%s: request(%s), content encoded.", master->wscCallTypeText(), request.str());
master->logctx.mCTXLOG("%s: request(%s)%s", master->wscCallTypeText(), request.str(), contentStr.str());
}
}

Expand Down Expand Up @@ -2250,9 +2292,18 @@ class CWSCAsyncFor : implements IWSCAsyncFor, public CInterface, public CAsyncFo
if (checkContentDecoding(dbgheader, response, contentEncoding))
decodeContent(contentEncoding.str(), response);
if (soapTraceLevel > 6 || master->logXML)
master->logctx.mCTXLOG("%s: LEN=%d %sresponse(%s%s)", getWsCallTypeName(master->wscType),response.length(),chunked?"CHUNKED ":"", dbgheader.str(), response.str());
else if (soapTraceLevel > 8)
master->logctx.mCTXLOG("%s: LEN=%d %sresponse(%s)", getWsCallTypeName(master->wscType),response.length(),chunked?"CHUNKED ":"", response.str()); // not sure this is that useful but...
{
// Only do translation if soapcall LOG option set and soapSepString defined
if ( (master->logXML) && (soapSepString.length() > 0) )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

{
StringBuffer response2;
multiLineAppendReplace(dbgheader, response2);
multiLineAppendReplace(response, response2);
master->logctx.CTXLOG("%s: LEN=%d %sresponse(%s)", getWsCallTypeName(master->wscType),response.length(),chunked?"CHUNKED ":"", response2.str());
}
else
master->logctx.mCTXLOG("%s: LEN=%d %sresponse(%s%s)", getWsCallTypeName(master->wscType),response.length(),chunked?"CHUNKED ":"", dbgheader.str(), response.str());
}
return rval;
}

Expand Down
1 change: 1 addition & 0 deletions common/thorhelper/thorsoapcall.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,6 @@ interface IRoxieAbortMonitor
extern THORHELPER_API unsigned soapTraceLevel;
extern THORHELPER_API IWSCHelper * createSoapCallHelper(IWSCRowProvider *, IEngineRowAllocator * outputAllocator, const char *authToken, SoapCallMode scMode, ClientCertificate *clientCert, const IContextLogger &logctx, IRoxieAbortMonitor * roxieAbortMonitor);
extern THORHELPER_API IWSCHelper * createHttpCallHelper(IWSCRowProvider *, IEngineRowAllocator * outputAllocator, const char *authToken, SoapCallMode scMode, ClientCertificate *clientCert, const IContextLogger &logctx, IRoxieAbortMonitor * roxieAbortMonitor);
extern THORHELPER_API void setSoapSepString(const char *_soapSepString);

#endif /* __THORSOAPCALL_HPP_ */
4 changes: 4 additions & 0 deletions ecl/hthor/hthor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7738,6 +7738,10 @@ void CHThorWSCBaseActivity::init()
JBASE64_Encode(uidpair.str(), uidpair.length(), authToken, false);
}
soapTraceLevel = agent.queryWorkUnit()->getDebugValueInt("soapTraceLevel", 1);
StringBuffer soapSepStr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

future: Can use SCMStringBuffer to avoid the need for the adaptor.

StringBufferAdaptor soapSepAdaptor(soapSepStr);
agent.queryWorkUnit()->getDebugValue("soapLogSepString", soapSepAdaptor);
setSoapSepString(soapSepStr.str());
}

//---------------------------------------------------------------------------
Expand Down
6 changes: 6 additions & 0 deletions roxie/ccd/ccdmain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -888,6 +888,12 @@ int CCD_API roxie_main(int argc, const char *argv[], const char * defaultYaml)
udpTraceLevel = topology->getPropInt("@udpTraceLevel", runOnce ? 0 : 1);
roxiemem::setMemTraceLevel(topology->getPropInt("@memTraceLevel", runOnce ? 0 : 1));
soapTraceLevel = topology->getPropInt("@soapTraceLevel", runOnce ? 0 : 1);
if (topology->hasProp("@soapLogSepString"))
{
StringBuffer tmpSepString;
topology->getProp("@soapLogSepString", tmpSepString);
setSoapSepString(tmpSepString.str());
}
miscDebugTraceLevel = topology->getPropInt("@miscDebugTraceLevel", 0);
traceRemoteFiles = topology->getPropBool("@traceRemoteFiles", false);
testAgentFailure = topology->getPropInt("expert/@testAgentFailure", testAgentFailure);
Expand Down
3 changes: 3 additions & 0 deletions thorlcr/graph/thgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2750,6 +2750,9 @@ void CJobBase::init()
failOnLeaks = getOptBool(THOROPT_FAIL_ON_LEAKS);
maxLfnBlockTimeMins = getOptInt(THOROPT_MAXLFN_BLOCKTIME_MINS, DEFAULT_MAXLFN_BLOCKTIME_MINS);
soapTraceLevel = getOptInt(THOROPT_SOAP_TRACE_LEVEL, 1);
StringBuffer tmpSepString;
getOpt(THOROPT_SOAP_LOG_SEP_STRING, tmpSepString);
setSoapSepString(tmpSepString.str());

StringBuffer tracing("maxActivityCores = ");
if (maxActivityCores)
Expand Down
1 change: 1 addition & 0 deletions thorlcr/thorutil/thormisc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@
#define THOROPT_MEMORY_SPILL_AT "memorySpillAt" // The threshold (%) that roxiemem will request memory to be reduced (default=80)
#define THOROPT_FAIL_ON_LEAKS "failOnLeaks" // If any leaks are detected at the end of graph, fail the query (default=false)
#define THOROPT_SOAP_TRACE_LEVEL "soapTraceLevel" // The trace SOAP level (default=1)
#define THOROPT_SOAP_LOG_SEP_STRING "soapLogSepString" // The SOAP request/response separator string for logging (default="")
#define THOROPT_SORT_ALGORITHM "sortAlgorithm" // The algorithm used to sort records (quicksort/mergesort)
#define THOROPT_COMPRESS_ALLFILES "v9_4_compressAllOutputs" // Compress all output files (default: bare-metal=off, cloud=on)
#define THOROPT_AVOID_RENAME "avoidRename" // Avoid rename, write directly to target physical filenames (no temp file)
Expand Down
Loading