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.

Sign up for GitHub

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

3 ➡️ 4 logging #151

Merged
merged 2 commits into from
May 22, 2020
Merged

3 ➡️ 4 logging #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
70c79be
strip <path> specified by user
mabelzhang May 22, 2020
7adf761
update logging tests to ignore SDF <path>
mabelzhang May 22, 2020
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
29 changes: 19 additions & 10 deletions src/ServerPrivate.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,14 +207,23 @@ void ServerPrivate::AddRecordPlugin(const ServerConfig &_config)
// SDF - except for the record path.)
if (pluginName->GetAsString() == LoggingPlugin::RecordPluginName())
{
// Explicitly allowing empty record paths through, always override
// <path>
sdf::ElementPtr pathElem = std::make_shared<sdf::Element>();
pathElem->SetName("path");
pluginElem->AddElementDescription(pathElem);
pathElem = pluginElem->GetElement("path");
pathElem->AddValue("string", "", false, "");
pathElem->Set<std::string>(_config.LogRecordPath());
// TODO(anyone) Specify paths below in a Component instead of
// injecting SDF, so as to not confuse the <path> specified by user
// stripped here, and the one injected below. With Components,
// would not need to strip user-specified <path>, can simply
// ignore it when setting component values.

if (!_config.LogRecordPath().empty())
{
// Explicitly allowing empty record paths through, always override
// <path>
sdf::ElementPtr pathElem = std::make_shared<sdf::Element>();
pathElem->SetName("record_path");
pluginElem->AddElementDescription(pathElem);
pathElem = pluginElem->GetElement("record_path");
pathElem->AddValue("string", "", false, "");
pathElem->Set<std::string>(_config.LogRecordPath());
}

// If resource flag specified on command line, replace in SDF
if (_config.LogRecordResources())
Expand Down Expand Up @@ -274,9 +283,9 @@ void ServerPrivate::AddRecordPlugin(const ServerConfig &_config)
if (!_config.LogRecordPath().empty())
{
sdf::ElementPtr pathElem = std::make_shared<sdf::Element>();
pathElem->SetName("path");
pathElem->SetName("record_path");
recordElem->AddElementDescription(pathElem);
pathElem = recordElem->GetElement("path");
pathElem = recordElem->GetElement("record_path");
pathElem->AddValue("string", "", false, "");
pathElem->Set<std::string>(_config.LogRecordPath());
}
Expand Down
10 changes: 1 addition & 9 deletions src/ign.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,13 +185,6 @@ extern "C" IGNITION_GAZEBO_VISIBLE int runServer(const char *_sdfString,
{
ignLogInit(recordPathMod, "server_console.log");
}
// TODO(anyone) In Ignition-D, to be moved to outside and after this
// if-else statement, after all ignLogInit() calls have been finalized,
// so that <path> in SDF will always be ignored in favor of logging both
// console logs and LogRecord recordings to common::ignLogDirectory().
// In Blueprint and Citadel, LogRecord will record to <path> if no
// --record-path is specified on command line.
serverConfig.SetLogRecordPath(recordPathMod);
}
// Empty record path specified. Use default.
else
Expand All @@ -200,9 +193,8 @@ extern "C" IGNITION_GAZEBO_VISIBLE int runServer(const char *_sdfString,
ignLogInit(recordPathMod, "server_console.log");
ignmsg << "Recording states to default path [" << recordPathMod << "]"
<< std::endl;

serverConfig.SetLogRecordPath(recordPathMod);
}
serverConfig.SetLogRecordPath(recordPathMod);
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion src/systems/log/LogRecord.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ void LogRecord::Configure(const Entity &_entity,
// activate one recorder.
if (!LogRecordPrivate::started)
{
auto logPath = _sdf->Get<std::string>("path");
auto logPath = _sdf->Get<std::string>("record_path");
// Path is initialized by server if record is set from command line options.
// Otherwise, path is loaded from SDF. If a path is not specified in
// SDF, initialize to default here.
Expand Down
180 changes: 22 additions & 158 deletions test/integration/log_system.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,12 @@ class LogSystemTest : public ::testing::Test
// \param[in] _recordSdfRoot SDF Root element of the world to load
// \param[in] _recordPath Path for SDF state file
// \param[in] _cmpPath Path for compressed file
public: void RunCompress(sdf::Root &_recordSdfRoot,
public: void RunCompress(const std::string &_recordSdfPath,
const std::string &_recordPath, const std::string &_cmpPath)
{
// Pass changed SDF to server
ServerConfig recordServerConfig;
recordServerConfig.SetSdfString(_recordSdfRoot.Element()->ToString(""));
recordServerConfig.SetSdfFile(_recordSdfPath);

// Set record path
recordServerConfig.SetLogRecordPath(_recordPath);
Expand Down Expand Up @@ -404,6 +404,9 @@ TEST_F(LogSystemTest, LogDefaults)
EXPECT_TRUE(common::exists(common::joinPaths(timestampPath,
"state.tlog")));
EXPECT_EQ(2, entryCount(timestampPath));

// Remove artifacts. Recreate new directory
this->RemoveLogsDir();
#endif

// Revert environment variable after test is done
Expand Down Expand Up @@ -572,7 +575,7 @@ TEST_F(LogSystemTest, LogPaths)
this->CreateLogsDir();

// Test case 4:
// A path is specified in SDF.
// A path is specified in SDF - a feature removed in Ignition Dome.
// A different path is specified via C++ API.
// Should take C++ API path. State log should be stored here. Console log is
// not initialized because ign.cc is not triggered.
Expand Down Expand Up @@ -638,7 +641,7 @@ TEST_F(LogSystemTest, LogPaths)
this->CreateLogsDir();

// Test case 6:
// A path is specified in SDF.
// A path is specified in SDF - a feature removed in Ignition Dome.
// A path is specified by --record-path on command line.
// Path in SDF should be ignored. Both state and console logs should be
// stored to --record-path path.
Expand Down Expand Up @@ -695,18 +698,13 @@ TEST_F(LogSystemTest, RecordAndPlayback)
std::string(PROJECT_SOURCE_PATH), "test", "worlds",
"log_record_dbl_pendulum.sdf");

// Change log path in SDF to build directory
sdf::Root recordSdfRoot;
this->ChangeLogPath(recordSdfRoot, recordSdfPath, "LogRecord",
this->logDir);
EXPECT_EQ(1u, recordSdfRoot.WorldCount());

// Pass changed SDF to server
ServerConfig recordServerConfig;
recordServerConfig.SetSdfString(recordSdfRoot.Element()->ToString(""));
Server recordServer(recordServerConfig);
recordServerConfig.SetSdfFile(recordSdfPath);
recordServerConfig.SetUseLogRecord(true);
recordServerConfig.SetLogRecordPath(this->logDir);

// Run for a few seconds to record different poses
Server recordServer(recordServerConfig);
recordServer.Run(true, 1000, false);
}

Expand Down Expand Up @@ -1022,14 +1020,10 @@ TEST_F(LogSystemTest, LogOverwrite)

// Record something to create some files
{
// Change log path in SDF to build directory
sdf::Root recordSdfRoot;
this->ChangeLogPath(recordSdfRoot, recordSdfPath, "LogRecord",
this->logDir);

// Pass changed SDF to server
ServerConfig recordServerConfig;
recordServerConfig.SetSdfString(recordSdfRoot.Element()->ToString(""));
recordServerConfig.SetSdfFile(recordSdfPath);
recordServerConfig.SetUseLogRecord(true);
recordServerConfig.SetLogRecordPath(this->logDir);

// Run for a few seconds to record different poses
Server recordServer(recordServerConfig);
Expand Down Expand Up @@ -1057,45 +1051,7 @@ TEST_F(LogSystemTest, LogOverwrite)
// Test case 1:
// Path exists, no overwrite flag. LogRecord.cc should still overwrite by
// default behavior whenever the specified path already exists.
// Path is set by SDF.
{
EXPECT_TRUE(common::exists(this->logDir));

// Change log path in SDF to build directory
sdf::Root recordSdfRoot;
this->ChangeLogPath(recordSdfRoot, recordSdfPath, "LogRecord",
this->logDir);

// Pass changed SDF to server
ServerConfig recordServerConfig;
recordServerConfig.SetSdfString(recordSdfRoot.Element()->ToString(""));

// Run for a few seconds to record different poses
Server recordServer(recordServerConfig);
recordServer.Run(true, 100, false);
}

// Log files still exist
EXPECT_TRUE(common::exists(tlogPath));
EXPECT_TRUE(common::exists(clogPath));

#ifndef __APPLE__
// No new files were created
EXPECT_EQ(2, entryCount(this->logsDir));
EXPECT_EQ(2, entryCount(this->logDir));

// Test timestamp is newer
EXPECT_GT(std::filesystem::last_write_time(tlogStdPath), tlogPrevTime);
// Update timestamp for next test
tlogPrevTime = std::filesystem::last_write_time(tlogStdPath);
#endif

// Test case 2:
// Path exists, no overwrite flag. LogRecord.cc should still overwrite by
// default behavior whenever the specified path already exists.
// Path is set by C++ API.
{
// Pass SDF file to server
ServerConfig recordServerConfig;
recordServerConfig.SetSdfFile(recordSdfPath);
recordServerConfig.SetUseLogRecord(true);
Expand All @@ -1119,86 +1075,8 @@ TEST_F(LogSystemTest, LogOverwrite)
EXPECT_GT(std::filesystem::last_write_time(tlogStdPath), tlogPrevTime);
// Update timestamp for next test
tlogPrevTime = std::filesystem::last_write_time(tlogStdPath);
#endif

// Test case 3:
// Path exists, no overwrite flag. LogRecord.cc should still overwrite by
// default behavior whenever the specified path already exists.
// Path is set by SDF.
// Server is run from command line, ign.cc should initialize new default
// timestamp directory, where console log should be recorded. State log should
// be recorded to the path in SDF.

// Change environment variable so that test files aren't written to $HOME
std::string homeOrig;
common::env(IGN_HOMEDIR, homeOrig);
std::string homeFake = common::joinPaths(this->logsDir, "default");
EXPECT_EQ(setenv(IGN_HOMEDIR, homeFake.c_str(), 1), 0);

// Store number of files before running
auto logPath = common::joinPaths(homeFake.c_str(), ".ignition", "gazebo",
"log");
#ifndef __APPLE__
int nEntries = entryCount(logPath);
std::vector<std::string> entriesBefore;
entryList(logPath, entriesBefore);

std::string tmpRecordSdfPath = common::joinPaths(this->logsDir,
"with_record_path.sdf");

{
// Change log path in SDF to build directory
sdf::Root recordSdfRoot;
this->ChangeLogPath(recordSdfRoot, recordSdfPath, "LogRecord",
this->logDir);
EXPECT_EQ(1u, recordSdfRoot.WorldCount());

// Save changed SDF to temporary file
// TODO(anyone): Does this work on Apple?
std::ofstream ofs(tmpRecordSdfPath);
ofs << recordSdfRoot.Element()->ToString("").c_str();
ofs.close();

// Command line triggers ign.cc, which handles initializing ignLogDirectory
std::string cmd = kIgnCommand + " -r -v 4 --iterations 5 "
+ tmpRecordSdfPath;
std::cout << "Running command [" << cmd << "]" << std::endl;

// Run
std::string output = customExecStr(cmd);
std::cout << output << std::endl;
}

// State log file still exists
EXPECT_TRUE(common::exists(tlogPath));

// Check the diff of list of files and assume there is a single diff, it
// being the newly created log directory from the run above.
EXPECT_EQ(nEntries + 1, entryCount(logPath));
std::vector<std::string> entriesAfter;
entryList(logPath, entriesAfter);
std::vector<std::string> entriesDiff;
entryDiff(entriesBefore, entriesAfter, entriesDiff);
EXPECT_EQ(1ul, entriesDiff.size());
// This should be $HOME/.ignition/..., default path
std::string timestampPath = entriesDiff[0];

EXPECT_FALSE(timestampPath.empty());
EXPECT_EQ(0, timestampPath.compare(0, logPath.length(), logPath));
EXPECT_TRUE(common::exists(timestampPath));
EXPECT_TRUE(common::exists(common::joinPaths(timestampPath,
"server_console.log")));
EXPECT_EQ(1, entryCount(timestampPath));

// Cleanup
common::removeFile(tmpRecordSdfPath);
common::removeAll(homeFake);
common::removeAll(timestampPath);

// Revert environment variable after test is done
EXPECT_EQ(setenv(IGN_HOMEDIR, homeOrig.c_str(), 1), 0);

// Test case 4:
// Test case 2:
// Path exists, command line --log-overwrite, should overwrite by
// command-line logic in ign.cc
{
Expand Down Expand Up @@ -1226,7 +1104,7 @@ TEST_F(LogSystemTest, LogOverwrite)
// Update timestamp for next test
tlogPrevTime = std::filesystem::last_write_time(tlogStdPath);

// Test case 5:
// Test case 3:
// Path exists, no --log-overwrite, should create new files by command-line
// logic in ign.cc
{
Expand Down Expand Up @@ -1436,14 +1314,10 @@ TEST_F(LogSystemTest, LogCompress)
std::string(PROJECT_SOURCE_PATH), "test", "worlds",
"log_record_dbl_pendulum.sdf");

// Change log path in SDF to build directory
sdf::Root recordSdfRoot;
this->ChangeLogPath(recordSdfRoot, recordSdfPath, "LogRecord",
recordPath);

// Pass changed SDF to server
ServerConfig recordServerConfig;
recordServerConfig.SetSdfString(recordSdfRoot.Element()->ToString(""));
recordServerConfig.SetSdfFile(recordSdfPath);
recordServerConfig.SetUseLogRecord(true);
recordServerConfig.SetLogRecordPath(recordPath);

// Set compress flag
recordServerConfig.SetLogRecordCompressPath(defaultCmpPath);
Expand Down Expand Up @@ -1548,11 +1422,6 @@ TEST_F(LogSystemTest, LogCompressOverwrite)
std::string(PROJECT_SOURCE_PATH), "test", "worlds",
"log_record_dbl_pendulum.sdf");

// Change log path in SDF to build directory
sdf::Root recordSdfRoot;
this->ChangeLogPath(recordSdfRoot, recordSdfPath, "LogRecord",
recordPath);

// Compress + overwrite, recorded directory exists, compressed file does not
{
// Create recording directory so that it exists
Expand All @@ -1562,7 +1431,7 @@ TEST_F(LogSystemTest, LogCompressOverwrite)
EXPECT_TRUE(common::exists(recordPath));
EXPECT_FALSE(common::exists(defaultCmpPath));

this->RunCompress(recordSdfRoot, recordPath, defaultCmpPath);
this->RunCompress(recordSdfPath, recordPath, defaultCmpPath);
}

EXPECT_TRUE(common::exists(defaultCmpPath));
Expand All @@ -1574,7 +1443,7 @@ TEST_F(LogSystemTest, LogCompressOverwrite)
EXPECT_FALSE(common::exists(recordPath));
EXPECT_TRUE(common::exists(defaultCmpPath));

this->RunCompress(recordSdfRoot, recordPath, defaultCmpPath);
this->RunCompress(recordSdfPath, recordPath, defaultCmpPath);
}

EXPECT_TRUE(common::exists(defaultCmpPath));
Expand All @@ -1600,15 +1469,10 @@ TEST_F(LogSystemTest, LogCompressCmdLine)
std::string(PROJECT_SOURCE_PATH), "test", "worlds",
"log_record_dbl_pendulum.sdf");

// Change log path in SDF to build directory
sdf::Root recordSdfRoot;
this->ChangeLogPath(recordSdfRoot, recordSdfPath, "LogRecord",
recordPath);

// Compress only, both recorded directory and compressed file exist
{
// Create compressed file
this->RunCompress(recordSdfRoot, recordPath, defaultCmpPath);
this->RunCompress(recordSdfPath, recordPath, defaultCmpPath);

// Recreate recording directory so that it exists
common::createDirectories(recordPath);
Expand Down