Skip to content

Commit

Permalink
HPCC-32281 Code review 1
Browse files Browse the repository at this point in the history 
- Adds epoch log field type
- Ensures logfield type default is set to string
- Fixes invalid XML tag
- Uses queryPropValues rather than queryprop(key)
- Uses queryProp(key) rather than queryProp
- Protects null reference to interatro in processValues method
- Utilizes cpp17 switch case [[fallthrough]]
- Generalizes logMap configuration logic

Signed-off-by: Rodrigo Pastrana <[email protected]>
  • Loading branch information
@rpastrana
rpastrana committed Jul 30, 2024
1 parent 60807c9 commit b18c15a
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 82 deletions.
3 changes: 2 additions & 1 deletion esp/scm/ws_logaccess.ecm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ ESPenum LogColumnValueType : string
str("string"),
numeric("numeric"),
datetime("datetime"),
enum("enum")
enum("enum"),
epoch("epoch")
};

ESPStruct [nil_remove] LogColumn
Expand Down
4 changes: 4 additions & 0 deletions esp/services/ws_logaccess/WsLogAccessService.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ bool Cws_logaccessEx::onGetLogAccessInfo(IEspContext &context, IEspGetLogAccessI
WARNLOG("Invalid col type found in logaccess logmap config");
}
}
else
{
espLogColumn->setColumnType("string");
}
logColumns.append(*espLogColumn.getClear());
}
else
Expand Down
119 changes: 38 additions & 81 deletions system/logaccess/Grafana/CurlClient/GrafanaCurlClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,12 @@ void formatResultLine(StringBuffer & returnbuf, const IProperties * resultLine,
Owned<IPropertyIterator> fieldsIter = resultLine->getIterator();
ForEach(*fieldsIter)
{
const char * prop = resultLine->queryProp(fieldsIter->getPropKey());
const char * prop = fieldsIter->queryPropValue();
returnbuf.appendf("<%s>", fieldsIter->getPropKey());
encodeXML(prop, returnbuf);
returnbuf.appendf("</%s>", fieldsIter->getPropKey());
}
returnbuf.appendf("</lin>");
returnbuf.appendf("</line>");
isFirstLine = false;
break;
}
Expand All @@ -227,7 +227,7 @@ void formatResultLine(StringBuffer & returnbuf, const IProperties * resultLine,
else
firstField = false;

const char * prop = resultLine->queryProp(fieldsIter->getPropKey());
const char * prop = fieldsIter->queryPropValue();
returnbuf.appendf("{\"%s\":\"", fieldsIter->getPropKey());
encodeJSON(returnbuf,prop);
returnbuf.append("\"}");
Expand All @@ -248,9 +248,8 @@ void formatResultLine(StringBuffer & returnbuf, const IProperties * resultLine,
else
firstField = false;

StringBuffer prop;
resultLine->getProp(fieldsIter->getPropKey(), prop);
encodeCSVColumn(returnbuf, prop);
const char * fieldValue = resultLine->queryProp(fieldsIter->getPropKey());
encodeCSVColumn(returnbuf, fieldValue);
}

returnbuf.newline();
Expand All @@ -269,6 +268,9 @@ void formatResultLine(StringBuffer & returnbuf, const IProperties * resultLine,
*/
void GrafanaLogAccessCurlClient::processValues(StringBuffer & returnbuf, IPropertyTreeIterator * valuesIter, IPropertyTree * stream, LogAccessLogFormat format, const LogAccessReturnColsMode retcolmode, bool & isFirstLine)
{
if (!valuesIter)
return;

Owned<IProperties> fieldValues = createProperties(true);

//extract the requested fields from the stream if it's available
Expand All @@ -281,10 +283,12 @@ void GrafanaLogAccessCurlClient::processValues(StringBuffer & returnbuf, IProper
fieldValues->setProp(m_nodeColumn.name, stream->queryProp(m_nodeColumn.name));
fieldValues->setProp(m_containerColumn.name, stream->queryProp(m_containerColumn.name));
fieldValues->setProp(m_instanceColumn.name, stream->queryProp(m_instanceColumn.name));
[[fallthrough]];
}
case RETURNCOLS_MODE_default:
{
fieldValues->setProp(m_podColumn.name, stream->queryProp(m_podColumn.name));
[[fallthrough]];
}
case RETURNCOLS_MODE_min:
{
Expand Down Expand Up @@ -416,10 +420,12 @@ void GrafanaLogAccessCurlClient::processQueryJsonResp(LogQueryResultDetails & re

if (result.hasProp("values"))
{
logLineIter.setown(result.getElements("values"));
}
logLineIter.setown(result.getElements("values")); // if no values elements found, will get NullPTreeIterator
if (!logLineIter)
IERRLOG("%s: Grafana response does not contain expected 'values' elements", COMPONENT_NAME);

processValues(returnbuf, logLineIter, result.queryPropTree("stream"), format, retcolmode, isFirstLine);
processValues(returnbuf, logLineIter, result.queryPropTree("stream"), format, retcolmode, isFirstLine);
}
}

//Adds the format postfix to the return buffer
Expand Down Expand Up @@ -717,6 +723,19 @@ bool GrafanaLogAccessCurlClient::fetchLog(LogQueryResultDetails & resultDetails,
return false;
}

void processLogMapConfig(const IPropertyTree * logMapConfig, LogField * targetField)
{
if (!logMapConfig || !targetField)
return;

if (logMapConfig->hasProp(logMapIndexPatternAtt))
if (strcmp(logMapConfig->queryProp(logMapIndexPatternAtt), "stream")==0)
targetField->isStream = true;

if (logMapConfig->hasProp(logMapSearchColAtt))
targetField->name = logMapConfig->queryProp(logMapSearchColAtt);
}

GrafanaLogAccessCurlClient::GrafanaLogAccessCurlClient(IPropertyTree & logAccessPluginConfig)
{
m_pluginCfg.set(&logAccessPluginConfig);
Expand Down Expand Up @@ -810,91 +829,29 @@ GrafanaLogAccessCurlClient::GrafanaLogAccessCurlClient(IPropertyTree & logAccess
IPropertyTree & logMap = logMapIter->query();
const char * logMapType = logMap.queryProp("@type");
if (streq(logMapType, "global"))
{
if (logMap.hasProp(logMapIndexPatternAtt))
if (strcmp(logMap.queryProp(logMapIndexPatternAtt), "stream")==0)
m_globalSearchCol.isStream = true;

if (logMap.hasProp(logMapSearchColAtt))
m_globalSearchCol.name = logMap.queryProp(logMapSearchColAtt);
}
processLogMapConfig(&logMap, &m_globalSearchCol);
else if (streq(logMapType, "workunits"))
{
if (logMap.hasProp(logMapSearchColAtt))
m_workunitsColumn = logMap.queryProp(logMapSearchColAtt);
}
processLogMapConfig(&logMap, &m_workunitsColumn);
else if (streq(logMapType, "components"))
{
if (logMap.hasProp(logMapIndexPatternAtt))
if (strcmp(logMap.queryProp(logMapIndexPatternAtt), "stream")==0)
m_componentsColumn.isStream = true;

if (logMap.hasProp(logMapSearchColAtt))
m_componentsColumn.name = logMap.queryProp(logMapSearchColAtt);
}
processLogMapConfig(&logMap, &m_componentsColumn);
else if (streq(logMapType, "class"))
{
if (logMap.hasProp(logMapSearchColAtt))
m_classColumn = logMap.queryProp(logMapSearchColAtt);
}
processLogMapConfig(&logMap, &m_classColumn);
else if (streq(logMapType, "audience"))
{
if (logMap.hasProp(logMapSearchColAtt))
m_audienceColumn = logMap.queryProp(logMapSearchColAtt);
}
processLogMapConfig(&logMap, &m_audienceColumn);
else if (streq(logMapType, "instance"))
{
if (logMap.hasProp(logMapIndexPatternAtt))
if (strcmp(logMap.queryProp(logMapIndexPatternAtt), "stream")==0)
m_instanceColumn.isStream = true;

if (logMap.hasProp(logMapSearchColAtt))
m_instanceColumn.name = logMap.queryProp(logMapSearchColAtt);
}
processLogMapConfig(&logMap, &m_instanceColumn);
else if (streq(logMapType, "node"))
{
if (logMap.hasProp(logMapIndexPatternAtt))
if (strcmp(logMap.queryProp(logMapIndexPatternAtt), "stream")==0)
m_nodeColumn.isStream = true;

if (logMap.hasProp(logMapSearchColAtt))
m_nodeColumn.name = logMap.queryProp(logMapSearchColAtt);
}
processLogMapConfig(&logMap, &m_nodeColumn);
else if (streq(logMapType, "host"))
{
OWARNLOG("%s: 'host' LogMap entry is NOT supported!", COMPONENT_NAME);
}
else if (streq(logMapType, "pod"))
{
if (logMap.hasProp(logMapIndexPatternAtt))
if (strcmp(logMap.queryProp(logMapIndexPatternAtt), "stream")==0)
m_podColumn.isStream = true;

if (logMap.hasProp(logMapSearchColAtt))
m_podColumn.name = logMap.queryProp(logMapSearchColAtt);
}
processLogMapConfig(&logMap, &m_podColumn);
else if (streq(logMapType, "message"))
{
if (logMap.hasProp(logMapIndexPatternAtt))
if (strcmp(logMap.queryProp(logMapIndexPatternAtt), "stream")==0)
m_messageColumn.isStream = true;

if (logMap.hasProp(logMapSearchColAtt))
m_messageColumn.name = logMap.queryProp(logMapSearchColAtt);
}
processLogMapConfig(&logMap, &m_messageColumn);
else if (streq(logMapType, "timestamp"))
{
if (logMap.hasProp(logMapIndexPatternAtt))
if (strcmp(logMap.queryProp(logMapIndexPatternAtt), "stream")==0)
m_logDateTimstampColumn.isStream = true;

if (logMap.hasProp(logMapSearchColAtt))
m_logDateTimstampColumn.name = logMap.queryProp(logMapSearchColAtt);
}
processLogMapConfig(&logMap, &m_logDateTimstampColumn);
else
{
ERRLOG("Encountered invalid LogAccess field map type: '%s'", logMapType);
}
}

DBGLOG("%s: targeting: '%s' - datasource: '%s'", COMPONENT_NAME, m_grafanaConnectionStr.str(), m_dataSourcesAPIURI.str());
Expand Down

0 comments on commit b18c15a

Please sign in to comment.