Skip to content

Commit abc4477

Browse files
Ron Hemeta-codesync[bot]
authored andcommitted
Test remaining functionalities for stateDelta logger
Summary: **Background** While debugging issues with ECMP Resource Manager stress testing, we noticed several crashes that are hard to reproduced when there is constant BGP flaps. The inability to reproduce such issue makes it very hard to debug - we are only relying on our luck reproducing edge cases with ixia etc. Rather, we can log all the state delta transitions happening on the box and trace every single delta transition. This would help us step-by-step to reproduce the issue. Note that this debugging tool should be test-use only. In production, we should consider using FSDB as the oper deltas are already streamed to FSDB (or we might decide to log within agent). **This Diff** Add remaining functional tests for stateDelta logger. This includes 1. Logging for multiple state Deltas 2. Logging for list of state Deltas, 3. Construction/Deconstruction when flag is not enabled. 4. Verify Protocol 5. Verify Generation number Reviewed By: yebolai Differential Revision: D85705881 fbshipit-source-id: ded5ce7ac21e63162f0183b48a8d3f42933e5fab
1 parent 225b269 commit abc4477

File tree

2 files changed

+148
-0
lines changed

2 files changed

+148
-0
lines changed

fboss/agent/test/BUCK

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,21 @@ cpp_unittest(
423423
],
424424
)
425425

426+
cpp_unittest(
427+
name = "state_delta_logger_test",
428+
srcs = ["StateDeltaLoggerTest.cpp"],
429+
deps = [
430+
":utils",
431+
"//fboss/agent:agent_features",
432+
"//fboss/agent:state_delta_logger",
433+
"//fboss/agent/state:state",
434+
"//fboss/fsdb/if:fsdb_oper-cpp2-types",
435+
"//fboss/thrift_cow/nodes:serializer",
436+
"//folly:file_util",
437+
"//folly/testing:test_util",
438+
],
439+
)
440+
426441
cpp_library(
427442
name = "agent_test",
428443
srcs = [

fboss/agent/test/StateDeltaLoggerTest.cpp

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,3 +171,136 @@ TEST_F(StateDeltaLoggerTest, LogStateDeltaAndReconstruct) {
171171
<< protocolName;
172172
}
173173
}
174+
175+
// Test logging multiple state deltas
176+
TEST_F(StateDeltaLoggerTest, LogMultipleStateDeltas) {
177+
auto state1 = makeStateWithRoute("2001:db8:a::", 64);
178+
auto state2 = makeStateWithRoute("2001:db8:b::", 64);
179+
auto state3 = makeStateWithRoute("2001:db8:c::", 64);
180+
// Create deltas
181+
StateDelta delta1(state1, state2);
182+
StateDelta delta2(state2, state3);
183+
// Log both deltas
184+
{
185+
StateDeltaLogger logger;
186+
logger.logStateDelta(delta1, "first_change");
187+
logger.logStateDelta(delta2, "second_change");
188+
}
189+
// Read and verify log file contains both entries
190+
auto logContent = readLogFile();
191+
EXPECT_FALSE(logContent.empty());
192+
// Count occurrences of "reason"
193+
size_t count = 0;
194+
size_t pos = 0;
195+
while ((pos = logContent.find("\"reason\":", pos)) != std::string::npos) {
196+
count++;
197+
pos++;
198+
}
199+
EXPECT_GE(count, 2) << "Expected at least 2 log entries";
200+
// Verify both reasons are present
201+
EXPECT_NE(logContent.find("first_change"), std::string::npos);
202+
EXPECT_NE(logContent.find("second_change"), std::string::npos);
203+
}
204+
// Test logging vector of state deltas
205+
TEST_F(StateDeltaLoggerTest, LogStateDeltasVector) {
206+
std::vector<StateDelta> deltas;
207+
auto state1 = makeStateWithRoute("2001:db8:ac10::", 48);
208+
auto state2 = makeStateWithRoute("2001:db8:ac11::", 48);
209+
deltas.emplace_back(state1, state2);
210+
deltas.emplace_back(state1, state2);
211+
// Log vector of deltas
212+
{
213+
StateDeltaLogger logger;
214+
logger.logStateDeltas(deltas, "batch_change");
215+
}
216+
// Verify log file contains entries
217+
auto logContent = readLogFile();
218+
EXPECT_FALSE(logContent.empty());
219+
// Count "batch_change" occurrences
220+
size_t count = 0;
221+
size_t pos = 0;
222+
while ((pos = logContent.find("batch_change", pos)) != std::string::npos) {
223+
count++;
224+
pos++;
225+
}
226+
EXPECT_GE(count, 2) << "Expected at least 2 log entries for batch";
227+
}
228+
// Test different serialization protocols
229+
TEST_F(StateDeltaLoggerTest, SerializationProtocols) {
230+
auto testProtocol = [this](const std::string& protocol) {
231+
// Create new temp file for this protocol
232+
auto tempFile = std::make_unique<folly::test::TemporaryFile>();
233+
FLAGS_state_delta_log_file = tempFile->path().string();
234+
FLAGS_state_delta_log_protocol = protocol;
235+
auto state1 = makeStateWithRoute("2001:db8:c8::", 48);
236+
auto state2 = makeStateWithRoute("2001:db8:c9::", 48);
237+
StateDelta delta(state1, state2);
238+
{
239+
StateDeltaLogger logger;
240+
logger.logStateDelta(delta, "protocol_test");
241+
}
242+
// Read and verify
243+
std::string content;
244+
EXPECT_TRUE(folly::readFile(tempFile->path().string().c_str(), content));
245+
EXPECT_FALSE(content.empty());
246+
EXPECT_NE(content.find("protocol_test"), std::string::npos);
247+
};
248+
testProtocol("COMPACT");
249+
testProtocol("BINARY");
250+
testProtocol("SIMPLE_JSON");
251+
}
252+
// Test logging when disabled
253+
TEST_F(StateDeltaLoggerTest, LoggingDisabled) {
254+
FLAGS_enable_state_delta_logging = false;
255+
auto state1 = std::make_shared<SwitchState>();
256+
auto state2 = std::make_shared<SwitchState>();
257+
StateDelta delta(state1, state2);
258+
{
259+
StateDeltaLogger logger;
260+
logger.logStateDelta(delta, "disabled_test");
261+
}
262+
// Should still create file but with minimal content (just header)
263+
auto content = readLogFile();
264+
// When disabled, we should get minimal or no logging
265+
// The exact behavior depends on implementation
266+
}
267+
// Test getConfiguredSerializationProtocol
268+
TEST_F(StateDeltaLoggerTest, GetConfiguredProtocol) {
269+
FLAGS_state_delta_log_protocol = "BINARY";
270+
EXPECT_EQ(
271+
StateDeltaLogger::getConfiguredSerializationProtocol(),
272+
fsdb::OperProtocol::BINARY);
273+
FLAGS_state_delta_log_protocol = "COMPACT";
274+
EXPECT_EQ(
275+
StateDeltaLogger::getConfiguredSerializationProtocol(),
276+
fsdb::OperProtocol::COMPACT);
277+
FLAGS_state_delta_log_protocol = "SIMPLE_JSON";
278+
EXPECT_EQ(
279+
StateDeltaLogger::getConfiguredSerializationProtocol(),
280+
fsdb::OperProtocol::SIMPLE_JSON);
281+
// Test case-insensitive
282+
FLAGS_state_delta_log_protocol = "binary";
283+
EXPECT_EQ(
284+
StateDeltaLogger::getConfiguredSerializationProtocol(),
285+
fsdb::OperProtocol::BINARY);
286+
}
287+
// Test that generation numbers are logged
288+
TEST_F(StateDeltaLoggerTest, LogGenerationNumbers) {
289+
auto state1 = makeStateWithRoute("2001:db8:64::", 48);
290+
auto state2 = makeStateWithRoute("2001:db8:65::", 48);
291+
StateDelta delta(state1, state2);
292+
auto oldGen = delta.oldState()->getGeneration();
293+
auto newGen = delta.newState()->getGeneration();
294+
{
295+
StateDeltaLogger logger;
296+
logger.logStateDelta(delta, "generation_test");
297+
}
298+
auto logContent = readLogFile();
299+
// Verify generation numbers are in the log
300+
EXPECT_NE(
301+
logContent.find("\"old_generation\":" + std::to_string(oldGen)),
302+
std::string::npos);
303+
EXPECT_NE(
304+
logContent.find("\"new_generation\":" + std::to_string(newGen)),
305+
std::string::npos);
306+
}

0 commit comments

Comments
 (0)