Skip to content

Commit 2d09ad4

Browse files
zbud-msftzbud-msft
authored
Fix pr comments (#11)
* Fix PR comments * Update with PR review comments * Fix LGTM warnings of publish tool * Fix bug in unit test * Change to ip to match yang model * Add changes per peer review
1 parent b6cf34b commit 2d09ad4

6 files changed

Lines changed: 139 additions & 88 deletions

File tree

‎dockers/docker-fpm-frr/bgp_regex.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
{
33
"tag": "bgp-state",
44
"regex": "Peer .default\\|([0-9a-f:.]*[0-9a-f]*). admin state is set to .(up|down).",
5-
"params": [ "peer_ip", "status" ]
5+
"params": [ "ip", "status" ]
66
}
77
]
88

‎src/sonic-eventd/rsyslog_plugin/rsyslog_plugin.cpp‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,50 +25,80 @@ bool RsyslogPlugin::onMessage(string msg, lua_State* luaState) {
2525
}
2626
}
2727

28+
void parseParams(vector<string> params, vector<EventParam>& eventParams) {
29+
for(long unsigned int i = 0; i < params.size(); i++) {
30+
if(params[i].empty()) {
31+
SWSS_LOG_ERROR("Empty param provided in regex file\n");
32+
continue;
33+
}
34+
EventParam ep = EventParam();
35+
auto delimPos = params[i].find(':');
36+
if(delimPos == string::npos) { // no lua code
37+
ep.paramName = params[i];
38+
} else {
39+
ep.paramName = params[i].substr(0, delimPos);
40+
ep.luaCode = params[i].substr(delimPos + 1);
41+
if(ep.luaCode.empty()) {
42+
SWSS_LOG_ERROR("Lua code missing after :\n");
43+
}
44+
}
45+
eventParams.push_back(ep);
46+
}
47+
}
48+
2849
bool RsyslogPlugin::createRegexList() {
2950
fstream regexFile;
51+
json jsonList = json::array();
3052
regexFile.open(m_regexPath, ios::in);
3153
if (!regexFile) {
3254
SWSS_LOG_ERROR("No such path exists: %s for source %s\n", m_regexPath.c_str(), m_moduleName.c_str());
3355
return false;
3456
}
3557
try {
36-
regexFile >> m_parser->m_regexList;
58+
regexFile >> jsonList;
3759
} catch (invalid_argument& iaException) {
3860
SWSS_LOG_ERROR("Invalid JSON file: %s, throws exception: %s\n", m_regexPath.c_str(), iaException.what());
3961
return false;
4062
}
4163

4264
string regexString;
65+
string timestampRegex = "^([a-zA-Z]{3})?\\s*([0-9]{1,2})?\\s*([0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{0,6})?\\s*";
4366
regex expression;
67+
vector<RegexStruct> regexList;
4468

45-
for(long unsigned int i = 0; i < m_parser->m_regexList.size(); i++) {
69+
for(long unsigned int i = 0; i < jsonList.size(); i++) {
70+
RegexStruct rs = RegexStruct();
71+
vector<EventParam> eventParams;
4672
try {
47-
string timestampRegex = "^([a-zA-Z]{3})?\\s*([0-9]{1,2})?\\s*([0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{0,6})?\\s*";
48-
string eventRegex = m_parser->m_regexList[i]["regex"];
73+
string eventRegex = jsonList[i]["regex"];
4974
regexString = timestampRegex + eventRegex;
50-
string tag = m_parser->m_regexList[i]["tag"];
51-
vector<string> params = m_parser->m_regexList[i]["params"];
75+
string tag = jsonList[i]["tag"];
76+
vector<string> params = jsonList[i]["params"];
5277
vector<string> timestampParams = { "month", "day", "time" };
5378
params.insert(params.begin(), timestampParams.begin(), timestampParams.end());
54-
m_parser->m_regexList[i]["params"] = params;
5579
regex expr(regexString);
5680
expression = expr;
57-
} catch (domain_error& deException) {
81+
parseParams(params, eventParams);
82+
rs.params = eventParams;
83+
rs.tag = tag;
84+
rs.regexExpression = expression;
85+
regexList.push_back(rs);
86+
} catch (domain_error& deException) {
5887
SWSS_LOG_ERROR("Missing required key, throws exception: %s\n", deException.what());
5988
return false;
6089
} catch (regex_error& reException) {
6190
SWSS_LOG_ERROR("Invalid regex, throws exception: %s\n", reException.what());
6291
return false;
6392
}
64-
m_parser->m_expressions.push_back(expression);
6593
}
6694

67-
if(m_parser->m_expressions.empty()) {
95+
if(regexList.empty()) {
6896
SWSS_LOG_ERROR("Empty list of regex expressions.\n");
6997
return false;
7098
}
7199

100+
m_parser->m_regexList = regexList;
101+
72102
regexFile.close();
73103
return true;
74104
}

‎src/sonic-eventd/rsyslog_plugin/syslog_parser.cpp‎

Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -14,49 +14,45 @@
1414
bool SyslogParser::parseMessage(string message, string& eventTag, event_params_t& paramMap, lua_State* luaState) {
1515
for(long unsigned int i = 0; i < m_regexList.size(); i++) {
1616
smatch matchResults;
17-
vector<string> params = m_regexList[i]["params"];
18-
if(!regex_search(message, matchResults, m_expressions[i]) || params.size() != matchResults.size() - 1 || matchResults.size() < 4) {
17+
if(!regex_search(message, matchResults, m_regexList[i].regexExpression) || m_regexList[i].params.size() != matchResults.size() - 1 || matchResults.size() < 4) {
1918
continue;
2019
}
21-
20+
string formattedTimestamp;
2221
if(!matchResults[1].str().empty() && !matchResults[2].str().empty() && !matchResults[3].str().empty()) { // found timestamp components
23-
string formattedTimestamp = m_timestampFormatter->changeTimestampFormat({ matchResults[1].str(), matchResults[2].str(), matchResults[3].str() });
24-
if(!formattedTimestamp.empty()) {
25-
paramMap["timestamp"] = formattedTimestamp;
26-
} else {
27-
SWSS_LOG_ERROR("Timestamp is invalid and is not able to be formatted");
28-
}
22+
formattedTimestamp = m_timestampFormatter->changeTimestampFormat({ matchResults[1].str(), matchResults[2].str(), matchResults[3].str() });
2923
}
24+
if(!formattedTimestamp.empty()) {
25+
paramMap["timestamp"] = formattedTimestamp;
26+
} else {
27+
SWSS_LOG_ERROR("Timestamp is invalid and is not able to be formatted");
28+
}
29+
3030
// found matching regex
31-
eventTag = m_regexList[i]["tag"];
31+
eventTag = m_regexList[i].tag;
3232
// check params for lua code
33-
for(long unsigned int j = 3; j < params.size(); j++) {
34-
auto delimPos = params[j].find(':');
33+
for(long unsigned int j = 3; j < m_regexList[i].params.size(); j++) {
3534
string resultValue = matchResults[j + 1].str();
36-
if(delimPos == string::npos) { // no lua code
37-
paramMap[params[j]] = resultValue;
35+
string paramName = m_regexList[i].params[j].paramName;
36+
const char* luaCode = m_regexList[i].params[j].luaCode.c_str();
37+
38+
if(luaCode == NULL || *luaCode == 0) {
39+
SWSS_LOG_INFO("Invalid lua code, empty or missing");
40+
paramMap[paramName] = resultValue;
3841
continue;
3942
}
40-
// have to execute lua script
41-
string param = params[j].substr(0, delimPos);
42-
string luaString = params[j].substr(delimPos + 1);
43-
if(luaString.empty()) { // empty lua code
44-
SWSS_LOG_INFO("Lua code missing after :, skipping operation");
45-
paramMap[param] = resultValue;
46-
continue;
47-
}
48-
const char* luaCode = luaString.c_str();
43+
44+
// execute lua code
4945
lua_pushstring(luaState, resultValue.c_str());
5046
lua_setglobal(luaState, "arg");
5147
if(luaL_dostring(luaState, luaCode) == 0) {
5248
lua_pop(luaState, lua_gettop(luaState));
53-
} else {
49+
} else { // error in lua code
5450
SWSS_LOG_ERROR("Invalid lua code, unable to do operation.\n");
55-
paramMap[param] = resultValue;
51+
paramMap[paramName] = resultValue;
5652
continue;
5753
}
5854
lua_getglobal(luaState, "ret");
59-
paramMap[param] = lua_tostring(luaState, -1);
55+
paramMap[paramName] = lua_tostring(luaState, -1);
6056
lua_pop(luaState, 1);
6157
}
6258
return true;

‎src/sonic-eventd/rsyslog_plugin/syslog_parser.h‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,27 @@ extern "C"
1818
using namespace std;
1919
using json = nlohmann::json;
2020

21+
struct EventParam {
22+
string paramName;
23+
string luaCode;
24+
};
25+
26+
struct RegexStruct {
27+
regex regexExpression;
28+
vector<EventParam> params;
29+
string tag;
30+
};
31+
2132
/**
2233
* Syslog Parser is responsible for parsing log messages fed by rsyslog.d and returns
2334
* matched result to rsyslog_plugin to use with events publish API
2435
*
2536
*/
2637

2738
class SyslogParser {
28-
public:
39+
public:
2940
unique_ptr<TimestampFormatter> m_timestampFormatter;
30-
vector<regex> m_expressions;
31-
json m_regexList = json::array();
41+
vector<RegexStruct> m_regexList;
3242
bool parseMessage(string message, string& tag, event_params_t& paramDict, lua_State* luaState);
3343
SyslogParser();
3444
};

‎src/sonic-eventd/rsyslog_plugin_tests/rsyslog_plugin_ut.cpp‎

Lines changed: 62 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,30 @@ using namespace std;
1919
using namespace swss;
2020
using json = nlohmann::json;
2121

22+
vector<EventParam> createEventParams(vector<string> params, vector<string> luaCodes) {
23+
vector<EventParam> eventParams;
24+
for(long unsigned int i = 0; i < params.size(); i++) {
25+
EventParam ep = EventParam();
26+
ep.paramName = params[i];
27+
ep.luaCode = luaCodes[i];
28+
eventParams.push_back(ep);
29+
}
30+
return eventParams;
31+
}
32+
2233
TEST(syslog_parser, matching_regex) {
2334
json jList = json::array();
24-
vector<regex> testExpressions;
35+
vector<RegexStruct> regexList;
2536
string regexString = "^([a-zA-Z]{3})?\\s*([0-9]{1,2})?\\s*([0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{0,6})?\\s*message (.*) other_data (.*) even_more_data (.*)";
26-
json jTest;
27-
jTest["tag"] = "test_tag";
28-
jTest["regex"] = regexString;
29-
jTest["params"] = { "month", "day", "time", "message", "other_data", "even_more_data" };
30-
jList.push_back(jTest);
37+
vector<string> params = { "month", "day", "time", "message", "other_data", "even_more_data" };
38+
vector<string> luaCodes = { "", "", "", "", "", "" };
3139
regex expression(regexString);
32-
testExpressions.push_back(expression);
40+
41+
RegexStruct rs = RegexStruct();
42+
rs.tag = "test_tag";
43+
rs.regexExpression = expression;
44+
rs.params = createEventParams(params, luaCodes);
45+
regexList.push_back(rs);
3346

3447
string tag;
3548
event_params_t paramDict;
@@ -40,8 +53,7 @@ TEST(syslog_parser, matching_regex) {
4053
expectedDict["even_more_data"] = "test_data";
4154

4255
unique_ptr<SyslogParser> parser(new SyslogParser());
43-
parser->m_expressions = testExpressions;
44-
parser->m_regexList = jList;
56+
parser->m_regexList = regexList;
4557
lua_State* luaState = luaL_newstate();
4658
luaL_openlibs(luaState);
4759

@@ -55,15 +67,17 @@ TEST(syslog_parser, matching_regex) {
5567

5668
TEST(syslog_parser, matching_regex_timestamp) {
5769
json jList = json::array();
58-
vector<regex> testExpressions;
70+
vector<RegexStruct> regexList;
5971
string regexString = "^([a-zA-Z]{3})?\\s*([0-9]{1,2})?\\s*([0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{0,6})?\\s*message (.*) other_data (.*)";
60-
json jTest;
61-
jTest["tag"] = "test_tag";
62-
jTest["regex"] = regexString;
63-
jTest["params"] = { "month", "day", "time", "message", "other_data" };
64-
jList.push_back(jTest);
72+
vector<string> params = { "month", "day", "time", "message", "other_data" };
73+
vector<string> luaCodes = { "", "", "", "", "" };
6574
regex expression(regexString);
66-
testExpressions.push_back(expression);
75+
76+
RegexStruct rs = RegexStruct();
77+
rs.tag = "test_tag";
78+
rs.regexExpression = expression;
79+
rs.params = createEventParams(params, luaCodes);
80+
regexList.push_back(rs);
6781

6882
string tag;
6983
event_params_t paramDict;
@@ -74,8 +88,7 @@ TEST(syslog_parser, matching_regex_timestamp) {
7488
expectedDict["timestamp"] = "2022-07-21T02:10:00.000000Z";
7589

7690
unique_ptr<SyslogParser> parser(new SyslogParser());
77-
parser->m_expressions = testExpressions;
78-
parser->m_regexList = jList;
91+
parser->m_regexList = regexList;
7992
lua_State* luaState = luaL_newstate();
8093
luaL_openlibs(luaState);
8194

@@ -89,22 +102,23 @@ TEST(syslog_parser, matching_regex_timestamp) {
89102

90103
TEST(syslog_parser, no_matching_regex) {
91104
json jList = json::array();
92-
vector<regex> testExpressions;
93-
string regexString = "^([a-zA-Z]{3})?\\s*([0-9]{1,2})?\\s*([0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{0,6})?\s*no match";
94-
json jTest;
95-
jTest["tag"] = "test_tag";
96-
jTest["regex"] = regexString;
97-
jTest["params"] = { "month", "day", "time" };
98-
jList.push_back(jTest);
105+
vector<RegexStruct> regexList;
106+
string regexString = "^([a-zA-Z]{3})?\\s*([0-9]{1,2})?\\s*([0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{0,6})?\\s*no match";
107+
vector<string> params = { "month", "day", "time" };
108+
vector<string> luaCodes = { "", "", "" };
99109
regex expression(regexString);
100-
testExpressions.push_back(expression);
110+
111+
RegexStruct rs = RegexStruct();
112+
rs.tag = "test_tag";
113+
rs.regexExpression = expression;
114+
rs.params = createEventParams(params, luaCodes);
115+
regexList.push_back(rs);
101116

102117
string tag;
103118
event_params_t paramDict;
104119

105120
unique_ptr<SyslogParser> parser(new SyslogParser());
106-
parser->m_expressions = testExpressions;
107-
parser->m_regexList = jList;
121+
parser->m_regexList = regexList;
108122
lua_State* luaState = luaL_newstate();
109123
luaL_openlibs(luaState);
110124

@@ -116,15 +130,17 @@ TEST(syslog_parser, no_matching_regex) {
116130

117131
TEST(syslog_parser, lua_code_valid_1) {
118132
json jList = json::array();
119-
vector<regex> testExpressions;
133+
vector<RegexStruct> regexList;
120134
string regexString = "^([a-zA-Z]{3})?\\s*([0-9]{1,2})?\\s*([0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{0,6})?\\s*.* (sent|received) (?:to|from) .* ([0-9]{2,3}.[0-9]{2,3}.[0-9]{2,3}.[0-9]{2,3}) active ([1-9]{1,3})/([1-9]{1,3}) .*";
121-
json jTest;
122-
jTest["tag"] = "test_tag";
123-
jTest["regex"] = regexString;
124-
jTest["params"] = { "month", "day", "time", "is-sent:ret=tostring(arg==\"sent\")", "ip", "major-code", "minor-code" };
125-
jList.push_back(jTest);
135+
vector<string> params = { "month", "day", "time", "is-sent", "ip", "major-code", "minor-code" };
136+
vector<string> luaCodes = { "", "", "", "ret=tostring(arg==\"sent\")", "", "", "" };
126137
regex expression(regexString);
127-
testExpressions.push_back(expression);
138+
139+
RegexStruct rs = RegexStruct();
140+
rs.tag = "test_tag";
141+
rs.regexExpression = expression;
142+
rs.params = createEventParams(params, luaCodes);
143+
regexList.push_back(rs);
128144

129145
string tag;
130146
event_params_t paramDict;
@@ -136,8 +152,7 @@ TEST(syslog_parser, lua_code_valid_1) {
136152
expectedDict["minor-code"] = "2";
137153

138154
unique_ptr<SyslogParser> parser(new SyslogParser());
139-
parser->m_expressions = testExpressions;
140-
parser->m_regexList = jList;
155+
parser->m_regexList = regexList;
141156
lua_State* luaState = luaL_newstate();
142157
luaL_openlibs(luaState);
143158

@@ -151,15 +166,17 @@ TEST(syslog_parser, lua_code_valid_1) {
151166

152167
TEST(syslog_parser, lua_code_valid_2) {
153168
json jList = json::array();
154-
vector<regex> testExpressions;
169+
vector<RegexStruct> regexList;
155170
string regexString = "([a-zA-Z]{3})?\\s*([0-9]{1,2})?\\s*([0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{0,6})?\\s*.* (sent|received) (?:to|from) .* ([0-9]{2,3}.[0-9]{2,3}.[0-9]{2,3}.[0-9]{2,3}) active ([1-9]{1,3})/([1-9]{1,3}) .*";
156-
json jTest;
157-
jTest["tag"] = "test_tag";
158-
jTest["regex"] = regexString;
159-
jTest["params"] = { "month", "day", "time", "is-sent:ret=tostring(arg==\"sent\")", "ip", "major-code", "minor-code" };
160-
jList.push_back(jTest);
171+
vector<string> params = { "month", "day", "time", "is-sent", "ip", "major-code", "minor-code" };
172+
vector<string> luaCodes = { "", "", "", "ret=tostring(arg==\"sent\")", "", "", "" };
161173
regex expression(regexString);
162-
testExpressions.push_back(expression);
174+
175+
RegexStruct rs = RegexStruct();
176+
rs.tag = "test_tag";
177+
rs.regexExpression = expression;
178+
rs.params = createEventParams(params, luaCodes);
179+
regexList.push_back(rs);
163180

164181
string tag;
165182
event_params_t paramDict;
@@ -172,8 +189,7 @@ TEST(syslog_parser, lua_code_valid_2) {
172189
expectedDict["timestamp"] = "2022-12-03T12:36:24.503424Z";
173190

174191
unique_ptr<SyslogParser> parser(new SyslogParser());
175-
parser->m_expressions = testExpressions;
176-
parser->m_regexList = jList;
192+
parser->m_regexList = regexList;
177193
lua_State* luaState = luaL_newstate();
178194
luaL_openlibs(luaState);
179195

0 commit comments

Comments
 (0)