Skip to content

Commit

Permalink
Additional review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
Tim Klemm authored and Tim Klemm committed Jan 7, 2025
1 parent 80b6799 commit 236557c
Showing 1 changed file with 82 additions and 75 deletions.
157 changes: 82 additions & 75 deletions esp/bindings/http/platform/httpservice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,12 @@ void CEspHttpServer::traceRequest(IEspContext* ctx, const char* normalizeMethod)
span->setSpanAttribute("url.full", full);
}

// Enumeration of "esp" service methods that are never traced. Add new values as additional
// requests become relevant.
enum class UntracedGetMethod
// Enumeration of "esp" service methods. These are generally requests for files or form markup, in
// other words, they are web service overhead not specific to any ESP instance. Add new values as
// additional requests become relevant.
enum class EspGetMethod
{
None, // empty name
Files,
Xslt,
Body,
Expand All @@ -312,35 +314,42 @@ enum class UntracedGetMethod
NavData,
NavMenuEvent,
SoapReq,
// DO NOT MAP NAMES TO THE FOLLOWING VALUES:
Unhandled, // catch-all for any method name not explicitly handled by processRequest
NotApplicable, // request not associated with the "esp" service
};
struct UntracedGetMethodNameComparator
struct EspGetMethodNameComparator
{
bool operator()(const char* lhs, const char* rhs) const { return stricmp(lhs, rhs) < 0; }
};
using UntracedGetMethodMap = std::map<const char*, UntracedGetMethod, UntracedGetMethodNameComparator>;
// Association of method name to a specific "esp" request that will not be traced. Multiple names
// may map to the same request, and all redundant names (e.g., "files" and "files_") must be in the
// map. Tracing will be suppressed for all method names in the map.
static const UntracedGetMethodMap getRequests{
{"files", UntracedGetMethod::Files},
{"xslt", UntracedGetMethod::Xslt},
{"body", UntracedGetMethod::Body},
{"frame", UntracedGetMethod::Frame},
{"titlebar", UntracedGetMethod::TitleBar},
{"nav", UntracedGetMethod::Nav},
{"navdata", UntracedGetMethod::NavData},
{"navmenuevent", UntracedGetMethod::NavMenuEvent},
{"soapreq", UntracedGetMethod::SoapReq},
using EspGetMethodMap = std::map<const char*, EspGetMethod, EspGetMethodNameComparator>;
// Association of method names to specific "esp" service requests. This mapping allows
// pprocessRequest to decide if the request should be traced before processing the request, without
// repeating the method name string comparisons. Multiple names may map to the same request, and
// all redundant names (e.g., "files" and "files_") must be included in the map.
static const EspGetMethodMap getRequests{
// esp
{"", EspGetMethod::None},
// esp/<key>
{"files", EspGetMethod::Files},
{"xslt", EspGetMethod::Xslt},
{"body", EspGetMethod::Body},
{"frame", EspGetMethod::Frame},
{"titlebar", EspGetMethod::TitleBar},
{"nav", EspGetMethod::Nav},
{"navdata", EspGetMethod::NavData},
{"navmenuevent", EspGetMethod::NavMenuEvent},
{"soapreq", EspGetMethod::SoapReq},
// same as above but with trailing underscore
{"files_", UntracedGetMethod::Files},
{"xslt_", UntracedGetMethod::Xslt},
{"body_", UntracedGetMethod::Body},
{"frame_", UntracedGetMethod::Frame},
{"titlebar_", UntracedGetMethod::TitleBar},
{"nav_", UntracedGetMethod::Nav},
{"navdata_", UntracedGetMethod::NavData},
{"navmenuevent_", UntracedGetMethod::NavMenuEvent},
{"soapreq_", UntracedGetMethod::SoapReq},
{"files_", EspGetMethod::Files},
{"xslt_", EspGetMethod::Xslt},
{"body_", EspGetMethod::Body},
{"frame_", EspGetMethod::Frame},
{"titlebar_", EspGetMethod::TitleBar},
{"nav_", EspGetMethod::Nav},
{"navdata_", EspGetMethod::NavData},
{"navmenuevent_", EspGetMethod::NavMenuEvent},
{"soapreq_", EspGetMethod::SoapReq},
};

int CEspHttpServer::processRequest()
Expand Down Expand Up @@ -402,29 +411,30 @@ int CEspHttpServer::processRequest()
// so maximize the amount of request processing that can be traced. Specifically, user
// authentication and authorization may generate trace output.
bool wantTracing = true;
UntracedGetMethodMap::const_iterator untracedRequestIt = getRequests.end();
if (!queryTraceManager().isTracingEnabled())
wantTracing = false;
else if (streq(method, GET_METHOD))
EspGetMethod espGetMethod = EspGetMethod::NotApplicable;
if (streq(method, GET_METHOD))
{
if (sub_serv_root == stype)
wantTracing = false;
else if (strieq(serviceName, "esp"))
{
if (methodName.isEmpty())
wantTracing = false;
else
// At this time, the presence of a method name in the get request map is sufficient
// to suppress trace output. The mapped value will be used later.
EspGetMethodMap::const_iterator it = getRequests.find(methodName);
if (it != getRequests.end())
{
// The presence of a method name in the get request map is sufficient to
// suppress trace output. The enumerated value will be used later.
untracedRequestIt = getRequests.find(methodName);
if (untracedRequestIt != getRequests.end())
wantTracing = false;
espGetMethod = it->second;
wantTracing = false;
}
else
espGetMethod = EspGetMethod::Unhandled;
}
}
else if (!m_apport)
wantTracing = false;
// Check last to prevent necessary side effects of other checks from occurring.
else if (!queryTraceManager().isTracingEnabled())
wantTracing = false;
Owned<ISpan> serverSpan;
if (wantTracing)
{
Expand Down Expand Up @@ -481,43 +491,40 @@ int CEspHttpServer::processRequest()
return onGetApplicationFrame(m_request.get(), m_response.get(), ctx);
}

if (!stricmp(serviceName.str(), "esp"))
// Use the previously identified method selector to dispatch the request.
switch (espGetMethod)
{
if (!methodName.length())
return 0;

if (untracedRequestIt != getRequests.end())
{
// Use the previously identified method selector to dispatch the request.
switch (untracedRequestIt->second)
{
case UntracedGetMethod::Files:
if (!getTxSummaryResourceReq())
ctx->cancelTxSummary();
checkInitEclIdeResponse(m_request, m_response);
return onGetFile(m_request.get(), m_response.get(), pathEx.str());
case UntracedGetMethod::Xslt:
if (!getTxSummaryResourceReq())
ctx->cancelTxSummary();
return onGetXslt(m_request.get(), m_response.get(), pathEx.str());
case UntracedGetMethod::Body:
return onGetMainWindow(m_request.get(), m_response.get());
case UntracedGetMethod::Frame:
return onGetApplicationFrame(m_request.get(), m_response.get(), ctx);
case UntracedGetMethod::TitleBar:
return onGetTitleBar(m_request.get(), m_response.get());
case UntracedGetMethod::Nav:
return onGetNavWindow(m_request.get(), m_response.get());
case UntracedGetMethod::NavData:
return onGetDynNavData(m_request.get(), m_response.get());
case UntracedGetMethod::NavMenuEvent:
return onGetNavEvent(m_request.get(), m_response.get());
case UntracedGetMethod::SoapReq:
return onGetBuildSoapRequest(m_request.get(), m_response.get());
default:
break;
}
}
case EspGetMethod::None:
return 0;
case EspGetMethod::Files:
if (!getTxSummaryResourceReq())
ctx->cancelTxSummary();
checkInitEclIdeResponse(m_request, m_response);
return onGetFile(m_request.get(), m_response.get(), pathEx.str());
case EspGetMethod::Xslt:
if (!getTxSummaryResourceReq())
ctx->cancelTxSummary();
return onGetXslt(m_request.get(), m_response.get(), pathEx.str());
case EspGetMethod::Body:
return onGetMainWindow(m_request.get(), m_response.get());
case EspGetMethod::Frame:
return onGetApplicationFrame(m_request.get(), m_response.get(), ctx);
case EspGetMethod::TitleBar:
return onGetTitleBar(m_request.get(), m_response.get());
case EspGetMethod::Nav:
return onGetNavWindow(m_request.get(), m_response.get());
case EspGetMethod::NavData:
return onGetDynNavData(m_request.get(), m_response.get());
case EspGetMethod::NavMenuEvent:
return onGetNavEvent(m_request.get(), m_response.get());
case EspGetMethod::SoapReq:
return onGetBuildSoapRequest(m_request.get(), m_response.get());
case EspGetMethod::Unhandled:
case EspGetMethod::NotApplicable:
break;
default:
IERRLOG("unexpected EspGetMethod value: %d", (int)espGetMethod);
break;
}
}

Expand Down

0 comments on commit 236557c

Please sign in to comment.