Skip to content

Commit 711a6f1

Browse files
authored
Merge pull request #2880 from adamkewley/feature-optionally-disable-file-logging
Implemented legacy-compatible support for disabling file logging
2 parents c04cd30 + 5776f83 commit 711a6f1

File tree

4 files changed

+157
-79
lines changed

4 files changed

+157
-79
lines changed

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ v4.2
3434
- The new Matlab CustomStaticOptimization.m guides the user to build their own custom static optimization code.
3535
- Dropped support for separate Kinematics for application of External Loads. ([PR #2770] (https://github.com/opensim-org/opensim-core/pull/2770)).
3636
- Refactored InverseKinematicsSolver to allow for adding (live) Orientation data to track, introduced BufferedOrientationsReference to queue data (PR #2855)
37+
- `opensim.log` will only be created/opened when the first message is logged to it (PR #2880):
38+
- Previously, `opensim.log` would always be created, even if nothing was logged
39+
- Added a CMake option, `OPENSIM_DISABLE_LOG_FILE` (PR #2880):
40+
- When set, disables `opensim.log` from being used by the logger by default when the first message is written to the log
41+
- Log messages are still written to the standard output/error streams
42+
- Previously, `opensim.log` would always be created - even if nothing was written to it (fixed above)
43+
- Setting `OPENSIM_DISABLE_LOG_FILE` only disables the automatic creation of `opensim.log`. File logging can still be manually be enabled by calling `Logger::addFileSink()`
44+
- This flag is `OFF` by default. So standard builds will still observe the existing behavior (`opensim.log` is created).
3745

3846
v4.1
3947
====

CMakeLists.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,17 @@ mark_as_advanced(OPENSIM_PYTHON_STANDALONE) # Mainly for packagers; not users.
111111
option(BUILD_API_ONLY "Build/install only headers, libraries,
112112
wrapping, tests; not applications (opensim, ik, rra, etc.)." OFF)
113113

114+
option(OPENSIM_DISABLE_LOG_FILE
115+
"Disable OpenSim from automatically creating 'opensim.log'.
116+
117+
By default, any application linking to osimCommon will create an
118+
'opensim.log' file in the current working directory. All log messages
119+
written by OpenSim (incl. during static initialization) are written to
120+
both this file and the standard output streams." OFF)
121+
122+
if(OPENSIM_DISABLE_LOG_FILE)
123+
add_definitions(-DOPENSIM_DISABLE_LOG_FILE=1)
124+
endif()
114125

115126
set(OPENSIM_BUILD_INDIVIDUAL_APPS_DEFAULT OFF)
116127
if(WIN32)

OpenSim/Common/Logger.cpp

Lines changed: 115 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -30,23 +30,97 @@
3030

3131
using namespace OpenSim;
3232

33-
std::shared_ptr<spdlog::logger> Logger::m_cout_logger =
33+
static void initializeLogger(spdlog::logger& l, const char* pattern) {
34+
l.set_level(spdlog::level::info);
35+
l.set_pattern(pattern);
36+
}
37+
38+
// cout logger will be initialized during static initialization time
39+
static std::shared_ptr<spdlog::logger> coutLogger =
3440
spdlog::stdout_color_mt("cout");
35-
std::shared_ptr<spdlog::sinks::basic_file_sink_mt> Logger::m_filesink = {};
36-
std::shared_ptr<spdlog::logger> Logger::m_default_logger;
37-
38-
// Force creation of the Logger instane to initialize spdlog::loggers
39-
std::shared_ptr<OpenSim::Logger> Logger::m_osimLogger = Logger::getInstance();
40-
41-
Logger::Logger() {
42-
m_default_logger = spdlog::default_logger();
43-
m_default_logger->set_level(spdlog::level::info);
44-
m_default_logger->set_pattern("[%l] %v");
45-
m_cout_logger->set_level(spdlog::level::info);
46-
m_cout_logger->set_pattern("%v");
47-
// This ensures log files are updated regularly, instead of only when the
48-
// program shuts down.
41+
42+
// default logger will be initialized during static initialization time
43+
static std::shared_ptr<spdlog::logger> defaultLogger =
44+
spdlog::default_logger();
45+
46+
// this function returns a dummy value so that it can be used in an assignment
47+
// expression (below) that *must* be executed in-order at static init time
48+
static bool initializeLogging() {
49+
initializeLogger(*coutLogger, "%v");
50+
initializeLogger(*defaultLogger, "[%l] %v");
4951
spdlog::flush_on(spdlog::level::info);
52+
return true;
53+
}
54+
55+
// initialization of this variable will have the side-effect of completing the
56+
// initialization of logging
57+
static bool otherStaticInit = initializeLogging();
58+
59+
// the file log sink (e.g. `opensim.log`) is lazily initialized.
60+
//
61+
// it is only initialized when the first log message is about to be written to
62+
// it. Users *may* disable this functionality before the first log message is
63+
// written (or disable it statically, by setting OPENSIM_DISABLE_LOG_FILE)
64+
static std::shared_ptr<spdlog::sinks::basic_file_sink_mt> m_filesink = nullptr;
65+
66+
// if a user manually calls `Logger::(remove|add)FileSink`, auto-initialization
67+
// should be disabled. Manual usage "overrides" lazy auto-initialization.
68+
static bool fileSinkAutoInitDisabled = false;
69+
70+
// attempt to auto-initialize the file log, if applicable
71+
static bool initFileLoggingAsNeeded() {
72+
#ifdef OPENSIM_DISABLE_LOG_FILE
73+
// software builders may want to statically ensure that automatic file logging
74+
// *cannot* happen - even during static initialization. This compiler define
75+
// outright disables the behavior, which is important in Windows applications
76+
// that run multiple instances of OpenSim-linked binaries. In Windows, the
77+
// logs files collide and cause a "multiple processes cannot open the same
78+
// file" error).
79+
return true;
80+
#else
81+
static bool initialized = []() {
82+
if (fileSinkAutoInitDisabled) {
83+
return true;
84+
}
85+
Logger::addFileSink();
86+
return true;
87+
}();
88+
89+
return initialized;
90+
#endif
91+
}
92+
93+
// this function is only called when the caller is about to log something, so
94+
// it should perform lazy initialization of the file sink
95+
spdlog::logger& Logger::getCoutLogger() {
96+
initFileLoggingAsNeeded();
97+
return *coutLogger;
98+
}
99+
100+
// this function is only called when the caller is about to log something, so
101+
// it should perform lazy initialization of the file sink
102+
spdlog::logger& Logger::getDefaultLogger() {
103+
initFileLoggingAsNeeded();
104+
return *defaultLogger;
105+
}
106+
107+
static void addSinkInternal(std::shared_ptr<spdlog::sinks::sink> sink) {
108+
coutLogger->sinks().push_back(sink);
109+
defaultLogger->sinks().push_back(sink);
110+
}
111+
112+
static void removeSinkInternal(const std::shared_ptr<spdlog::sinks::sink> sink)
113+
{
114+
{
115+
auto& sinks = defaultLogger->sinks();
116+
auto new_end = std::remove(sinks.begin(), sinks.end(), sink);
117+
sinks.erase(new_end, sinks.end());
118+
}
119+
{
120+
auto& sinks = coutLogger->sinks();
121+
auto new_end = std::remove(sinks.begin(), sinks.end(), sink);
122+
sinks.erase(new_end, sinks.end());
123+
}
50124
}
51125

52126
void Logger::setLevel(Level level) {
@@ -79,8 +153,7 @@ void Logger::setLevel(Level level) {
79153
}
80154

81155
Logger::Level Logger::getLevel() {
82-
const auto level = m_default_logger->level();
83-
switch (level) {
156+
switch (defaultLogger->level()) {
84157
case spdlog::level::off: return Level::Off;
85158
case spdlog::level::critical: return Level::Critical;
86159
case spdlog::level::err: return Level::Error;
@@ -140,23 +213,32 @@ bool Logger::shouldLog(Level level) {
140213
default:
141214
OPENSIM_THROW(Exception, "Internal error.");
142215
}
143-
return m_default_logger->should_log(spdlogLevel);
216+
return defaultLogger->should_log(spdlogLevel);
144217
}
145218

146219
void Logger::addFileSink(const std::string& filepath) {
220+
// this method is either called by the file log auto-initializer, which
221+
// should now be disabled, or by downstream code trying to manually specify
222+
// a file sink
223+
//
224+
// downstream callers would find it quite surprising if the auto-initializer
225+
// runs *after* they manually specify a log, so just disable it
226+
fileSinkAutoInitDisabled = true;
227+
147228
if (m_filesink) {
148-
warn("Already logging to file '{}'; log file not added. Call "
229+
defaultLogger->warn("Already logging to file '{}'; log file not added. Call "
149230
"removeFileSink() first.", m_filesink->filename());
150231
return;
151232
}
233+
152234
// check if file can be opened at the specified path if not return meaningful
153235
// warning rather than bubble the exception up.
154236
try {
155237
m_filesink =
156238
std::make_shared<spdlog::sinks::basic_file_sink_mt>(filepath);
157239
}
158240
catch (...) {
159-
warn("Can't open file '{}' for writing. Log file will not be created. "
241+
defaultLogger->warn("Can't open file '{}' for writing. Log file will not be created. "
160242
"Check that you have write permissions to the specified path.",
161243
filepath);
162244
return;
@@ -165,6 +247,19 @@ void Logger::addFileSink(const std::string& filepath) {
165247
}
166248

167249
void Logger::removeFileSink() {
250+
// if this method is called, then we are probably at a point in the
251+
// application's lifetime where automatic log allocation is going to cause
252+
// confusion.
253+
//
254+
// callers will be surpised if, after calling this method, auto
255+
// initialization happens afterwards and the log file still exists - even
256+
// if they called it to remove some manually-specified log
257+
fileSinkAutoInitDisabled = true;
258+
259+
if (m_filesink == nullptr) {
260+
return;
261+
}
262+
168263
removeSinkInternal(
169264
std::static_pointer_cast<spdlog::sinks::sink>(m_filesink));
170265
m_filesink.reset();
@@ -178,24 +273,4 @@ void Logger::removeSink(const std::shared_ptr<LogSink> sink) {
178273
removeSinkInternal(std::static_pointer_cast<spdlog::sinks::sink>(sink));
179274
}
180275

181-
void Logger::addSinkInternal(std::shared_ptr<spdlog::sinks::sink> sink) {
182-
m_default_logger->sinks().push_back(sink);
183-
m_cout_logger->sinks().push_back(sink);
184-
}
185-
186-
void Logger::removeSinkInternal(const std::shared_ptr<spdlog::sinks::sink> sink)
187-
{
188-
{
189-
auto& sinks = m_default_logger->sinks();
190-
auto to_erase = std::find(sinks.cbegin(), sinks.cend(), sink);
191-
if (to_erase != sinks.cend()) sinks.erase(to_erase);
192-
}
193-
{
194-
auto& sinks = m_cout_logger->sinks();
195-
auto to_erase = std::find(sinks.cbegin(), sinks.cend(), sink);
196-
if (to_erase != sinks.cend()) sinks.erase(to_erase);
197-
}
198-
}
199-
200-
201276

OpenSim/Common/Logger.h

Lines changed: 23 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ class LogSink;
3737
/// controlling how those messages are presented to the user.
3838
class OSIMCOMMON_API Logger {
3939
public:
40-
Logger(Logger const&) = delete;
41-
Logger& operator=(Logger const&) = delete;
40+
/// This is a static singleton class: there is no way of constructing it
41+
Logger() = delete;
4242

4343
/// This enum lists the types of messages that should be logged. These
4444
/// levels match those of the spdlog logging library that OpenSim uses for
@@ -104,32 +104,44 @@ class OSIMCOMMON_API Logger {
104104

105105
template <typename... Args>
106106
static void critical(spdlog::string_view_t fmt, const Args&... args) {
107-
m_default_logger->critical(fmt, args...);
107+
if (shouldLog(Level::Critical)) {
108+
getDefaultLogger().critical(fmt, args...);
109+
}
108110
}
109111

110112
template <typename... Args>
111113
static void error(spdlog::string_view_t fmt, const Args&... args) {
112-
m_default_logger->error(fmt, args...);
114+
if (shouldLog(Level::Error)) {
115+
getDefaultLogger().error(fmt, args...);
116+
}
113117
}
114118

115119
template <typename... Args>
116120
static void warn(spdlog::string_view_t fmt, const Args&... args) {
117-
m_default_logger->warn(fmt, args...);
121+
if (shouldLog(Level::Warn)) {
122+
getDefaultLogger().warn(fmt, args...);
123+
}
118124
}
119125

120126
template <typename... Args>
121127
static void info(spdlog::string_view_t fmt, const Args&... args) {
122-
m_default_logger->info(fmt, args...);
128+
if (shouldLog(Level::Info)) {
129+
getDefaultLogger().info(fmt, args...);
130+
}
123131
}
124132

125133
template <typename... Args>
126134
static void debug(spdlog::string_view_t fmt, const Args&... args) {
127-
m_default_logger->debug(fmt, args...);
135+
if (shouldLog(Level::Debug)) {
136+
getDefaultLogger().debug(fmt, args...);
137+
}
128138
}
129139

130140
template <typename... Args>
131141
static void trace(spdlog::string_view_t fmt, const Args&... args) {
132-
m_default_logger->trace(fmt, args...);
142+
if (shouldLog(Level::Trace)) {
143+
getDefaultLogger().trace(fmt, args...);
144+
}
133145
}
134146

135147
/// Use this function to log messages that would normally be sent to
@@ -141,7 +153,7 @@ class OSIMCOMMON_API Logger {
141153
/// give users control over what gets logged.
142154
template <typename... Args>
143155
static void cout(spdlog::string_view_t fmt, const Args&... args) {
144-
m_cout_logger->log(spdlog::level::info, fmt, args...);
156+
getCoutLogger().log(spdlog::level::info, fmt, args...);
145157
}
146158

147159
/// @}
@@ -170,37 +182,9 @@ class OSIMCOMMON_API Logger {
170182
/// @note This function is not thread-safe. Do not invoke this function
171183
/// concurrently, or concurrently with addLogFile() or addSink().
172184
static void removeSink(const std::shared_ptr<LogSink> sink);
173-
174-
/// This returns the singleton instance of the Log class, but users never
175-
/// need to invoke this function. The member functions in this class are
176-
/// static.
177-
static const std::shared_ptr<OpenSim::Logger> getInstance() {
178-
if (!m_osimLogger) {
179-
m_osimLogger =
180-
std::shared_ptr<OpenSim::Logger>(new OpenSim::Logger());
181-
addFileSink();
182-
}
183-
return m_osimLogger;
184-
}
185185
private:
186-
/// Initialize spdlog.
187-
Logger();
188-
189-
static void addSinkInternal(std::shared_ptr<spdlog::sinks::sink> sink);
190-
191-
static void removeSinkInternal(
192-
const std::shared_ptr<spdlog::sinks::sink> sink);
193-
194-
/// This is the logger used in log_cout.
195-
static std::shared_ptr<spdlog::logger> m_cout_logger;
196-
197-
/// This is the singleton OpenSim::Logger
198-
static std::shared_ptr<OpenSim::Logger> m_osimLogger;
199-
200-
static std::shared_ptr<spdlog::logger> m_default_logger;
201-
202-
/// Keep track of the file sink.
203-
static std::shared_ptr<spdlog::sinks::basic_file_sink_mt> m_filesink;
186+
static spdlog::logger& getCoutLogger();
187+
static spdlog::logger& getDefaultLogger();
204188
};
205189

206190
/// @name Logging functions

0 commit comments

Comments
 (0)