Skip to content

[QC-871] Add log rotate parameters and refactor how we pass the discard file options #1488

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

Merged
Barthelemy merged 5 commits into from
Oct 10, 2022
Merged

[QC-871] Add log rotate parameters and refactor how we pass the discard file options #1488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c1945e3
[QC-871] Refactor how we pass the discard file options to avoid an ex…
Barthelemy Oct 4, 2022
6c44a04
format
Barthelemy Oct 4, 2022
3483991
Update CMakeLists.txt
Barthelemy Oct 10, 2022
06492fb
Update SkeletonTask.cxx
Barthelemy Oct 10, 2022
1439fa5
Merge branch 'master' into additional-discard-options
Barthelemy Oct 10, 2022
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
4 changes: 2 additions & 2 deletions Framework/basic.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
"url": "ccdb-test.cern.ch:8080"
},
"infologger": { "": "Configuration of the Infologger (optional).",
"filterDiscardDebug": "true", "": "Set to true to discard debug and trace messages (default: false)",
"filterDiscardDebug": "false", "": "Set to true to discard debug and trace messages (default: false)",
"filterDiscardLevel": "11", "": "Message at this level or above are discarded (default: 21 - Trace)",
"filterDiscardFile": "/tmp/new-test.txt", "": ["If set, the messages discarded because of filterDiscardLevel",
"filterDiscardFile": "/tmp/_ID_.txt", "": ["If set, the messages discarded because of filterDiscardLevel",
"will go to this file (default: <none>); The keyword _ID_ is replaced by the device id. Discarded Debug ",
"messages won't go there."]
}
Expand Down
5 changes: 2 additions & 3 deletions Framework/include/QualityControl/AggregatorRunnerConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <string>
#include <Framework/DataProcessorSpec.h>
#include "QualityControl/Activity.h"
#include "QualityControl/QcInfoLogger.h"

namespace o2::quality_control::checker
{
Expand All @@ -29,9 +30,7 @@ struct AggregatorRunnerConfig {
std::unordered_map<std::string, std::string> database;
std::string consulUrl{};
std::string monitoringUrl{};
bool infologgerFilterDiscardDebug = false;
int infologgerDiscardLevel = 21;
std::string infologgerDiscardFile{};
core::DiscardFileParameters infologgerDiscardParameters;
core::Activity fallbackActivity;
framework::Options options{};
};
Expand Down
5 changes: 2 additions & 3 deletions Framework/include/QualityControl/CheckRunnerConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <unordered_map>

#include "QualityControl/Activity.h"
#include "QualityControl/QcInfoLogger.h"

namespace o2::quality_control::checker
{
Expand All @@ -28,9 +29,7 @@ struct CheckRunnerConfig {
std::unordered_map<std::string, std::string> database;
std::string consulUrl{};
std::string monitoringUrl{};
bool infologgerFilterDiscardDebug = false;
int infologgerDiscardLevel = 21;
std::string infologgerDiscardFile{};
core::DiscardFileParameters infologgerDiscardParameters;
core::Activity fallbackActivity;
framework::Options options{};
};
Expand Down
5 changes: 2 additions & 3 deletions Framework/include/QualityControl/CommonSpec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include <string>
#include <unordered_map>
#include "QualityControl/QcInfoLogger.h"

namespace o2::quality_control::core
{
Expand All @@ -36,9 +37,7 @@ struct CommonSpec {
std::string monitoringUrl = "infologger:///debug?qc";
std::string consulUrl;
std::string conditionDBUrl = "http://ccdb-test.cern.ch:8080";
bool infologgerFilterDiscardDebug = false;
int infologgerDiscardLevel = 21;
std::string infologgerDiscardFile;
DiscardFileParameters infologgerDiscardParameters;
double postprocessingPeriod = 10.0;
};

Expand Down
5 changes: 2 additions & 3 deletions Framework/include/QualityControl/PostProcessingRunnerConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <unordered_map>
#include <string>
#include <boost/property_tree/ptree.hpp>
#include "QualityControl/QcInfoLogger.h"

namespace o2::quality_control::postprocessing
{
Expand All @@ -28,9 +29,7 @@ struct PostProcessingRunnerConfig {
std::string taskName;
std::unordered_map<std::string, std::string> database;
std::string consulUrl{};
bool infologgerFilterDiscardDebug = false;
int infologgerDiscardLevel = 21;
std::string infologgerDiscardFile{};
core::DiscardFileParameters infologgerDiscardParameters;
double periodSeconds = 10.0;
std::string configKeyValues; // These are for ConfigurableParams, not for override-values!
boost::property_tree::ptree configTree{};
Expand Down
12 changes: 9 additions & 3 deletions Framework/include/QualityControl/QcInfoLogger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ typedef AliceO2::InfoLogger::InfoLoggerContext infoContext;
namespace o2::quality_control::core
{

struct DiscardFileParameters {
bool debug = false;
int fromLevel = 21 /* Discard Trace */;
std::string discardFile;
unsigned long rotateMaxBytes = 0;
unsigned int rotateMaxFiles = 0;
};

Comment on lines +30 to +37
Copy link
Copy Markdown
Collaborator

@knopers8 knopers8 Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as a future improvement, I would propose to move this structure to a separate file, so we do not include QcInfologger header in configuration structures.

Copy link
Copy Markdown
Collaborator Author

@Barthelemy Barthelemy Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, will do

/// \brief Singleton class that any class in the QC can use to log.
///
/// The aim of this class is to avoid every class in the package to define
Expand Down Expand Up @@ -59,9 +67,7 @@ class QcInfoLogger
static void setRun(int run);
static void setPartition(const std::string& partitionName);
static void init(const std::string& facility,
bool discardDebug = false,
int discardFromLevel = 21 /* Discard Trace */,
const std::string& filterDiscardFile = "",
const DiscardFileParameters& discardFileParameters,
AliceO2::InfoLogger::InfoLogger* dplInfoLogger = nullptr,
AliceO2::InfoLogger::InfoLoggerContext* dplContext = nullptr,
int run = -1,
Expand Down
5 changes: 2 additions & 3 deletions Framework/include/QualityControl/TaskRunnerConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <Framework/DataProcessorSpec.h>
#include "QualityControl/Activity.h"
#include "QualityControl/QcInfoLogger.h"

namespace o2::quality_control::core
{
Expand All @@ -45,9 +46,7 @@ struct TaskRunnerConfig {
int parallelTaskID = 0; // ID to differentiate parallel local Tasks from one another. 0 means this is the only one.
std::string saveToFile{};
int resetAfterCycles = 0;
bool infologgerFilterDiscardDebug = false;
int infologgerDiscardLevel = 21;
std::string infologgerDiscardFile{};
core::DiscardFileParameters infologgerDiscardParameters;
Activity fallbackActivity;
};

Expand Down
6 changes: 2 additions & 4 deletions Framework/src/AggregatorRunner.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,9 @@ void AggregatorRunner::initInfoLogger(InitContext& iCtx)
ILOG(Error) << "Could not find the DPL InfoLogger." << ENDM;
}

mRunnerConfig.infologgerDiscardFile = templateILDiscardFile(mRunnerConfig.infologgerDiscardFile, iCtx);
mRunnerConfig.infologgerDiscardParameters.discardFile = templateILDiscardFile(mRunnerConfig.infologgerDiscardParameters.discardFile, iCtx);
QcInfoLogger::init("aggregator",
mRunnerConfig.infologgerFilterDiscardDebug,
mRunnerConfig.infologgerDiscardLevel,
mRunnerConfig.infologgerDiscardFile,
mRunnerConfig.infologgerDiscardParameters,
il,
ilContext);
}
Expand Down
4 changes: 1 addition & 3 deletions Framework/src/AggregatorRunnerFactory.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ AggregatorRunnerConfig AggregatorRunnerFactory::extractRunnerConfig(const core::
commonSpec.database,
commonSpec.consulUrl,
commonSpec.monitoringUrl,
commonSpec.infologgerFilterDiscardDebug,
commonSpec.infologgerDiscardLevel,
commonSpec.infologgerDiscardFile,
commonSpec.infologgerDiscardParameters,
fallbackActivity,
options
};
Expand Down
6 changes: 2 additions & 4 deletions Framework/src/CheckRunner.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,11 +488,9 @@ void CheckRunner::initInfologger(framework::InitContext& iCtx)
ILOG(Error) << "Could not find the DPL InfoLogger." << ENDM;
}

mConfig.infologgerDiscardFile = templateILDiscardFile(mConfig.infologgerDiscardFile, iCtx);
mConfig.infologgerDiscardParameters.discardFile = templateILDiscardFile(mConfig.infologgerDiscardParameters.discardFile, iCtx);
QcInfoLogger::init(createCheckRunnerFacility(mDeviceName),
mConfig.infologgerFilterDiscardDebug,
mConfig.infologgerDiscardLevel,
mConfig.infologgerDiscardFile,
mConfig.infologgerDiscardParameters,
il,
ilContext);
}
Expand Down
4 changes: 1 addition & 3 deletions Framework/src/CheckRunnerFactory.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ CheckRunnerConfig CheckRunnerFactory::extractConfig(const CommonSpec& commonSpec
commonSpec.database,
commonSpec.consulUrl,
commonSpec.monitoringUrl,
commonSpec.infologgerFilterDiscardDebug,
commonSpec.infologgerDiscardLevel,
commonSpec.infologgerDiscardFile,
commonSpec.infologgerDiscardParameters,
fallbackActivity,
options
};
Expand Down
10 changes: 7 additions & 3 deletions Framework/src/InfrastructureSpecReader.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,13 @@ CommonSpec InfrastructureSpecReader::readSpecEntry<CommonSpec>(std::string, cons
spec.monitoringUrl = commonTree.get<std::string>("monitoring.url", spec.monitoringUrl);
spec.consulUrl = commonTree.get<std::string>("consul.url", spec.consulUrl);
spec.conditionDBUrl = commonTree.get<std::string>("conditionDB.url", spec.conditionDBUrl);
spec.infologgerFilterDiscardDebug = commonTree.get<bool>("infologger.filterDiscardDebug", spec.infologgerFilterDiscardDebug);
spec.infologgerDiscardLevel = commonTree.get<int>("infologger.filterDiscardLevel", spec.infologgerDiscardLevel);
spec.infologgerDiscardFile = commonTree.get<std::string>("infologger.filterDiscardFile", spec.infologgerDiscardFile);
spec.infologgerDiscardParameters = {
commonTree.get<bool>("infologger.filterDiscardDebug", spec.infologgerDiscardParameters.debug),
commonTree.get<int>("infologger.filterDiscardLevel", spec.infologgerDiscardParameters.fromLevel),
commonTree.get<std::string>("infologger.filterDiscardFile", spec.infologgerDiscardParameters.discardFile),
commonTree.get<u_long>("infologger.filterRotateMaxBytes", spec.infologgerDiscardParameters.rotateMaxBytes),
commonTree.get<u_int>("infologger.filterRotateMaxFiles", spec.infologgerDiscardParameters.rotateMaxFiles)
};
spec.postprocessingPeriod = commonTree.get<double>("postprocessing.periodSeconds", spec.postprocessingPeriod);

return spec;
Expand Down
6 changes: 2 additions & 4 deletions Framework/src/PostProcessingRunner.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void PostProcessingRunner::init(const PostProcessingRunnerConfig& runnerConfig,
mRunnerConfig = runnerConfig;
mTaskConfig = taskConfig;

QcInfoLogger::init("post/" + mName, runnerConfig.infologgerFilterDiscardDebug, runnerConfig.infologgerDiscardLevel, runnerConfig.infologgerDiscardFile);
QcInfoLogger::init("post/" + mName, mRunnerConfig.infologgerDiscardParameters);
ILOG(Info, Support) << "Initializing PostProcessingRunner" << ENDM;

root_class_factory::loadLibrary(mTaskConfig.moduleName);
Expand Down Expand Up @@ -251,9 +251,7 @@ PostProcessingRunnerConfig PostProcessingRunner::extractConfig(const CommonSpec&
ppTaskSpec.taskName,
commonSpec.database,
commonSpec.consulUrl,
commonSpec.infologgerFilterDiscardDebug,
commonSpec.infologgerDiscardLevel,
commonSpec.infologgerDiscardFile,
commonSpec.infologgerDiscardParameters,
commonSpec.postprocessingPeriod,
"",
ppTaskSpec.tree
Expand Down
28 changes: 14 additions & 14 deletions Framework/src/QcInfoLogger.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ void QcInfoLogger::setPartition(const std::string& partitionName)
}

void QcInfoLogger::init(const std::string& facility,
bool discardDebug,
int discardFromLevel,
const std::string& discardToFile,
const DiscardFileParameters& discardFileParameters,
AliceO2::InfoLogger::InfoLogger* dplInfoLogger,
AliceO2::InfoLogger::InfoLoggerContext* dplContext,
int run,
Expand All @@ -75,15 +73,16 @@ void QcInfoLogger::init(const std::string& facility,
}

// Set the proper discard filters
ILOG_INST.filterDiscardDebug(discardDebug);
ILOG_INST.filterDiscardLevel(discardFromLevel);
if (!discardToFile.empty()) {
ILOG_INST.filterDiscardSetFile(discardToFile.c_str(), 1000000000, 10, 0, true /*Do not store Debug messages in file*/);
ILOG_INST.filterDiscardDebug(discardFileParameters.debug);
ILOG_INST.filterDiscardLevel(discardFileParameters.fromLevel);
if (!discardFileParameters.discardFile.empty()) {
ILOG_INST.filterDiscardSetFile(discardFileParameters.discardFile.c_str(), discardFileParameters.rotateMaxBytes, discardFileParameters.rotateMaxFiles, true /*Do not store Debug messages in file*/);
}
ILOG(Debug, Ops) << "QC infologger initialized" << ENDM;
ILOG(Debug, Support) << " Discard debug ? " << discardDebug << ENDM;
ILOG(Debug, Support) << " Discard from level ? " << discardFromLevel << ENDM;
ILOG(Debug, Support) << " Discard to file ? " << (!discardToFile.empty() ? discardToFile : "No") << ENDM;
ILOG(Debug, Support) << " Discard debug ? " << discardFileParameters.debug << ENDM;
ILOG(Debug, Support) << " Discard from level ? " << discardFileParameters.fromLevel << ENDM;
ILOG(Debug, Support) << " Discard to file ? " << (!discardFileParameters.discardFile.empty() ? discardFileParameters.discardFile : "No") << ENDM;
ILOG(Debug, Support) << " Discard max bytes and files ? " << discardFileParameters.rotateMaxBytes << " = " << discardFileParameters.rotateMaxFiles << ENDM;

setFacility(facility);
setRun(run);
Expand All @@ -97,11 +96,12 @@ void QcInfoLogger::init(const std::string& facility,
int run,
const std::string& partitionName)
{
DiscardFileParameters discardFileParameters;
std::string discardDebugStr = config.get<std::string>("qc.config.infologger.filterDiscardDebug", "false");
bool discardDebug = discardDebugStr == "true";
int discardLevel = config.get<int>("qc.config.infologger.filterDiscardLevel", 21 /* Discard Trace */);
std::string discardToFile = config.get<std::string>("qc.config.infologger.filterDiscardFile", "");
init(facility, discardDebug, discardLevel, discardToFile, dplInfoLogger, dplContext, run, partitionName);
discardFileParameters.debug = discardDebugStr == "true";
discardFileParameters.fromLevel = config.get<int>("qc.config.infologger.filterDiscardLevel", 21 /* Discard Trace */);
discardFileParameters.discardFile = config.get<std::string>("qc.config.infologger.filterDiscardFile", "");
init(facility, discardFileParameters, dplInfoLogger, dplContext, run, partitionName);
Comment on lines +101 to +104
Copy link
Copy Markdown
Collaborator

@knopers8 knopers8 Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you not reloading all the parameters?

Copy link
Copy Markdown
Collaborator Author

@Barthelemy Barthelemy Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, will fix.

}

} // namespace o2::quality_control::core
6 changes: 2 additions & 4 deletions Framework/src/TaskRunner.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,9 @@ void TaskRunner::initInfologger(InitContext& iCtx)
ILOG(Error, Devel) << "Could not find the DPL InfoLogger" << ENDM;
}

mTaskConfig.infologgerDiscardFile = templateILDiscardFile(mTaskConfig.infologgerDiscardFile, iCtx);
mTaskConfig.infologgerDiscardParameters.discardFile = templateILDiscardFile(mTaskConfig.infologgerDiscardParameters.discardFile, iCtx);
QcInfoLogger::init("task/" + mTaskConfig.taskName,
mTaskConfig.infologgerFilterDiscardDebug,
mTaskConfig.infologgerDiscardLevel,
mTaskConfig.infologgerDiscardFile,
mTaskConfig.infologgerDiscardParameters,
il,
ilContext);
QcInfoLogger::setDetector(mTaskConfig.detectorName);
Expand Down
4 changes: 1 addition & 3 deletions Framework/src/TaskRunnerFactory.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,7 @@ TaskRunnerConfig TaskRunnerFactory::extractConfig(const CommonSpec& globalConfig
parallelTaskID,
taskSpec.saveObjectsToFile,
resetAfterCycles.value_or(taskSpec.resetAfterCycles),
globalConfig.infologgerFilterDiscardDebug,
globalConfig.infologgerDiscardLevel,
globalConfig.infologgerDiscardFile,
globalConfig.infologgerDiscardParameters,
fallbackActivity
};
}
Expand Down
2 changes: 1 addition & 1 deletion Framework/test/testQcInfoLogger.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ BOOST_AUTO_TEST_CASE(qc_info_logger_dplil)
auto dplContext = new AliceO2::InfoLogger::InfoLoggerContext();
dplContext->setField(infoContext::FieldName::Facility, "dplfacility");
dplContext->setField(infoContext::FieldName::System, "dplsystem");
QcInfoLogger::init("facility", false, 21, "", &dplInfoLogger, dplContext);
QcInfoLogger::init("facility", { false, 21, "" }, &dplInfoLogger, dplContext);
}

} // namespace o2::quality_control::core
6 changes: 4 additions & 2 deletions doc/Advanced.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,8 +935,10 @@ should not be present in real configuration files.
"filterDiscardDebug": "false", "": "Set to 1 to discard debug and trace messages (default: false)",
"filterDiscardLevel": "2", "": "Message at this level or above are discarded (default: 21 - Trace)",
"filterDiscardFile": "", "": ["If set, the discarded messages will go to this file (default: <none>)",
"The keyword _ID_, if used, is replaced by the device ID.",
"Discarded Debug messages don't go there."]
"The keyword _ID_, if used, is replaced by the device ID.",
"Discarded Debug messages don't go there."],
"filterRotateMaxBytes": "", "": "Maximum size of the discard file.",
"filterRotateMaxFiles": "", "": "Maximum number of discard files."
Copy link
Copy Markdown
Collaborator

@knopers8 knopers8 Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why there could be more than one?

Copy link
Copy Markdown
Collaborator Author

@Barthelemy Barthelemy Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't that how logrotate usually works ? when a file is full it creates a new one, within the limit of the number of files ?

Copy link
Copy Markdown
Collaborator

@knopers8 knopers8 Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, of course

},
"postprocessing": { "": "Configuration parameters for post-processing",
"periodSeconds": 10.0, "": "Sets the interval of checking all the triggers. One can put a very small value",
Expand Down