From b38d1e1ec2336f47ec5d83f3878275e86343ddbc Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Thu, 21 Jun 2012 11:57:45 -0700 Subject: [PATCH 1/2] Reworking log writer API to make it easier to pass additional information to a writer's initialization method. However, for now the information provided is still the same. --- src/RemoteSerializer.cc | 18 ++++++------- src/RemoteSerializer.h | 5 ++-- src/logging/Manager.cc | 23 +++++++++------- src/logging/Manager.h | 5 ++-- src/logging/WriterBackend.cc | 22 ++++++++++++---- src/logging/WriterBackend.h | 44 ++++++++++++++++++++++--------- src/logging/WriterFrontend.cc | 23 ++++++++-------- src/logging/WriterFrontend.h | 12 ++++----- src/logging/writers/Ascii.cc | 8 +++--- src/logging/writers/Ascii.h | 2 +- src/logging/writers/DataSeries.cc | 12 ++++----- src/logging/writers/DataSeries.h | 2 +- src/logging/writers/None.cc | 4 +-- src/logging/writers/None.h | 2 +- 14 files changed, 112 insertions(+), 70 deletions(-) diff --git a/src/RemoteSerializer.cc b/src/RemoteSerializer.cc index 838bafb0d6..6a73bae553 100644 --- a/src/RemoteSerializer.cc +++ b/src/RemoteSerializer.cc @@ -2503,17 +2503,17 @@ bool RemoteSerializer::ProcessRemotePrint() return true; } -bool RemoteSerializer::SendLogCreateWriter(EnumVal* id, EnumVal* writer, string path, int num_fields, const threading::Field* const * fields) +bool RemoteSerializer::SendLogCreateWriter(EnumVal* id, EnumVal* writer, const logging::WriterBackend::WriterInfo& info, int num_fields, const threading::Field* const * fields) { loop_over_list(peers, i) { - SendLogCreateWriter(peers[i]->id, id, writer, path, num_fields, fields); + SendLogCreateWriter(peers[i]->id, id, writer, info, num_fields, fields); } return true; } -bool RemoteSerializer::SendLogCreateWriter(PeerID peer_id, EnumVal* id, EnumVal* writer, string path, int num_fields, const threading::Field* const * fields) +bool RemoteSerializer::SendLogCreateWriter(PeerID peer_id, EnumVal* id, EnumVal* writer, const logging::WriterBackend::WriterInfo& info, int num_fields, const threading::Field* const * fields) { SetErrorDescr("logging"); @@ -2535,8 +2535,8 @@ bool RemoteSerializer::SendLogCreateWriter(PeerID peer_id, EnumVal* id, EnumVal* bool success = fmt.Write(id->AsEnum(), "id") && fmt.Write(writer->AsEnum(), "writer") && - fmt.Write(path, "path") && - fmt.Write(num_fields, "num_fields"); + fmt.Write(num_fields, "num_fields") && + info.Write(&fmt); if ( ! success ) goto error; @@ -2691,13 +2691,13 @@ bool RemoteSerializer::ProcessLogCreateWriter() fmt.StartRead(current_args->data, current_args->len); int id, writer; - string path; int num_fields; + logging::WriterBackend::WriterInfo info; bool success = fmt.Read(&id, "id") && fmt.Read(&writer, "writer") && - fmt.Read(&path, "path") && - fmt.Read(&num_fields, "num_fields"); + fmt.Read(&num_fields, "num_fields") && + info.Read(&fmt); if ( ! success ) goto error; @@ -2716,7 +2716,7 @@ bool RemoteSerializer::ProcessLogCreateWriter() id_val = new EnumVal(id, BifType::Enum::Log::ID); writer_val = new EnumVal(writer, BifType::Enum::Log::Writer); - if ( ! log_mgr->CreateWriter(id_val, writer_val, path, num_fields, fields, true, false) ) + if ( ! log_mgr->CreateWriter(id_val, writer_val, info, num_fields, fields, true, false) ) goto error; Unref(id_val); diff --git a/src/RemoteSerializer.h b/src/RemoteSerializer.h index 4ebf15e68d..1d7feef585 100644 --- a/src/RemoteSerializer.h +++ b/src/RemoteSerializer.h @@ -9,6 +9,7 @@ #include "IOSource.h" #include "Stats.h" #include "File.h" +#include "logging/WriterBackend.h" #include #include @@ -104,10 +105,10 @@ public: bool SendPrintHookEvent(BroFile* f, const char* txt, size_t len); // Send a request to create a writer on a remote side. - bool SendLogCreateWriter(PeerID peer, EnumVal* id, EnumVal* writer, string path, int num_fields, const threading::Field* const * fields); + bool SendLogCreateWriter(PeerID peer, EnumVal* id, EnumVal* writer, const logging::WriterBackend::WriterInfo& info, int num_fields, const threading::Field* const * fields); // Broadcasts a request to create a writer. - bool SendLogCreateWriter(EnumVal* id, EnumVal* writer, string path, int num_fields, const threading::Field* const * fields); + bool SendLogCreateWriter(EnumVal* id, EnumVal* writer, const logging::WriterBackend::WriterInfo& info, int num_fields, const threading::Field* const * fields); // Broadcast a log entry to everybody interested. bool SendLogWrite(EnumVal* id, EnumVal* writer, string path, int num_fields, const threading::Value* const * vals); diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index f0b5cc1748..b30ee26534 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -74,6 +74,7 @@ struct Manager::WriterInfo { double interval; Func* postprocessor; WriterFrontend* writer; + WriterBackend::WriterInfo info; }; struct Manager::Stream { @@ -764,8 +765,11 @@ bool Manager::Write(EnumVal* id, RecordVal* columns) for ( int j = 0; j < filter->num_fields; ++j ) arg_fields[j] = new threading::Field(*filter->fields[j]); + WriterBackend::WriterInfo info; + info.path = path; + writer = CreateWriter(stream->id, filter->writer, - path, filter->num_fields, + info, filter->num_fields, arg_fields, filter->local, filter->remote); if ( ! writer ) @@ -953,7 +957,7 @@ threading::Value** Manager::RecordToFilterVals(Stream* stream, Filter* filter, return vals; } -WriterFrontend* Manager::CreateWriter(EnumVal* id, EnumVal* writer, string path, +WriterFrontend* Manager::CreateWriter(EnumVal* id, EnumVal* writer, const WriterBackend::WriterInfo& info, int num_fields, const threading::Field* const* fields, bool local, bool remote) { Stream* stream = FindStream(id); @@ -963,7 +967,7 @@ WriterFrontend* Manager::CreateWriter(EnumVal* id, EnumVal* writer, string path, return false; Stream::WriterMap::iterator w = - stream->writers.find(Stream::WriterPathPair(writer->AsEnum(), path)); + stream->writers.find(Stream::WriterPathPair(writer->AsEnum(), info.path)); if ( w != stream->writers.end() ) // If we already have a writer for this. That's fine, we just @@ -973,7 +977,7 @@ WriterFrontend* Manager::CreateWriter(EnumVal* id, EnumVal* writer, string path, WriterFrontend* writer_obj = new WriterFrontend(id, writer, local, remote); assert(writer_obj); - writer_obj->Init(path, num_fields, fields); + writer_obj->Init(info, num_fields, fields); WriterInfo* winfo = new WriterInfo; winfo->type = writer->Ref()->AsEnumVal(); @@ -982,6 +986,7 @@ WriterFrontend* Manager::CreateWriter(EnumVal* id, EnumVal* writer, string path, winfo->rotation_timer = 0; winfo->interval = 0; winfo->postprocessor = 0; + winfo->info = info; // Search for a corresponding filter for the writer/path pair and use its // rotation settings. If no matching filter is found, fall back on @@ -993,7 +998,7 @@ WriterFrontend* Manager::CreateWriter(EnumVal* id, EnumVal* writer, string path, { Filter* f = *it; if ( f->writer->AsEnum() == writer->AsEnum() && - f->path == winfo->writer->Path() ) + f->path == winfo->writer->info.path ) { found_filter_match = true; winfo->interval = f->interval; @@ -1012,7 +1017,7 @@ WriterFrontend* Manager::CreateWriter(EnumVal* id, EnumVal* writer, string path, InstallRotationTimer(winfo); stream->writers.insert( - Stream::WriterMap::value_type(Stream::WriterPathPair(writer->AsEnum(), path), + Stream::WriterMap::value_type(Stream::WriterPathPair(writer->AsEnum(), info.path), winfo)); return writer_obj; @@ -1093,7 +1098,7 @@ void Manager::SendAllWritersTo(RemoteSerializer::PeerID peer) EnumVal writer_val(i->first.first, BifType::Enum::Log::Writer); remote_serializer->SendLogCreateWriter(peer, (*s)->id, &writer_val, - i->first.second, + i->second->info, writer->NumFields(), writer->Fields()); } @@ -1246,7 +1251,7 @@ void Manager::Rotate(WriterInfo* winfo) localtime_r(&teatime, &tm); strftime(buf, sizeof(buf), date_fmt, &tm); - string tmp = string(fmt("%s-%s", winfo->writer->Path().c_str(), buf)); + string tmp = string(fmt("%s-%s", winfo->writer->Info().path.c_str(), buf)); // Trigger the rotation. winfo->writer->Rotate(tmp, winfo->open_time, network_time, terminating); @@ -1274,7 +1279,7 @@ bool Manager::FinishedRotation(WriterFrontend* writer, string new_name, string o RecordVal* info = new RecordVal(BifType::Record::Log::RotationInfo); info->Assign(0, winfo->type->Ref()); info->Assign(1, new StringVal(new_name.c_str())); - info->Assign(2, new StringVal(winfo->writer->Path().c_str())); + info->Assign(2, new StringVal(winfo->writer->Info().path.c_str())); info->Assign(3, new Val(open, TYPE_TIME)); info->Assign(4, new Val(close, TYPE_TIME)); info->Assign(5, new Val(terminating, TYPE_BOOL)); diff --git a/src/logging/Manager.h b/src/logging/Manager.h index f5e62b0683..38dd9258b3 100644 --- a/src/logging/Manager.h +++ b/src/logging/Manager.h @@ -9,13 +9,14 @@ #include "../EventHandler.h" #include "../RemoteSerializer.h" +#include "WriterBackend.h" + class SerializationFormat; class RemoteSerializer; class RotationTimer; namespace logging { -class WriterBackend; class WriterFrontend; class RotationFinishedMessage; @@ -162,7 +163,7 @@ protected: //// Function also used by the RemoteSerializer. // Takes ownership of fields. - WriterFrontend* CreateWriter(EnumVal* id, EnumVal* writer, string path, + WriterFrontend* CreateWriter(EnumVal* id, EnumVal* writer, const WriterBackend::WriterInfo& info, int num_fields, const threading::Field* const* fields, bool local, bool remote); diff --git a/src/logging/WriterBackend.cc b/src/logging/WriterBackend.cc index 23a95279d7..35bb27d27b 100644 --- a/src/logging/WriterBackend.cc +++ b/src/logging/WriterBackend.cc @@ -4,6 +4,7 @@ #include "bro_inet_ntop.h" #include "threading/SerialTypes.h" +#include "Manager.h" #include "WriterBackend.h" #include "WriterFrontend.h" @@ -60,14 +61,25 @@ public: using namespace logging; +bool WriterBackend::WriterInfo::Read(SerializationFormat* fmt) + { + return fmt->Read(&path, "path"); + } + +bool WriterBackend::WriterInfo::Write(SerializationFormat* fmt) const + { + return fmt->Write(path, "path"); + } + WriterBackend::WriterBackend(WriterFrontend* arg_frontend) : MsgThread() { - path = ""; num_fields = 0; fields = 0; buffering = true; frontend = arg_frontend; + info.path = ""; + SetName(frontend->Name()); } @@ -108,17 +120,17 @@ void WriterBackend::DisableFrontend() SendOut(new DisableMessage(frontend)); } -bool WriterBackend::Init(string arg_path, int arg_num_fields, const Field* const* arg_fields) +bool WriterBackend::Init(const WriterInfo& arg_info, int arg_num_fields, const Field* const* arg_fields) { - path = arg_path; + info = arg_info; num_fields = arg_num_fields; fields = arg_fields; - string name = Fmt("%s/%s", path.c_str(), frontend->Name().c_str()); + string name = Fmt("%s/%s", info.path.c_str(), frontend->Name().c_str()); SetName(name); - if ( ! DoInit(arg_path, arg_num_fields, arg_fields) ) + if ( ! DoInit(arg_info, arg_num_fields, arg_fields) ) { DisableFrontend(); return false; diff --git a/src/logging/WriterBackend.h b/src/logging/WriterBackend.h index 1269976aee..30e1995430 100644 --- a/src/logging/WriterBackend.h +++ b/src/logging/WriterBackend.h @@ -5,12 +5,14 @@ #ifndef LOGGING_WRITERBACKEND_H #define LOGGING_WRITERBACKEND_H -#include "Manager.h" - #include "threading/MsgThread.h" +class RemoteSerializer; + namespace logging { +class WriterFrontend; + /** * Base class for writer implementation. When the logging::Manager creates a * new logging filter, it instantiates a WriterFrontend. That then in turn @@ -41,21 +43,39 @@ public: */ virtual ~WriterBackend(); + /** + * A struct passing information to the writer at initialization time. + */ + struct WriterInfo + { + /** + * A string left to the interpretation of the writer + * implementation; it corresponds to the value configured on + * the script-level for the logging filter. + */ + string path; + + private: + friend class ::RemoteSerializer; + + // Note, these need to be adapted when changing the struct's + // fields. They serialize/deserialize the struct. + bool Read(SerializationFormat* fmt); + bool Write(SerializationFormat* fmt) const; + }; + /** * One-time initialization of the writer to define the logged fields. * - * @param path A string left to the interpretation of the writer - * implementation; it corresponds to the value configured on the - * script-level for the logging filter. - * - * @param num_fields The number of log fields for the stream. + * @param info Meta information for the writer. + * @param num_fields * * @param fields An array of size \a num_fields with the log fields. * The methods takes ownership of the array. * * @return False if an error occured. */ - bool Init(string path, int num_fields, const threading::Field* const* fields); + bool Init(const WriterInfo& info, int num_fields, const threading::Field* const* fields); /** * Writes one log entry. @@ -108,9 +128,9 @@ public: void DisableFrontend(); /** - * Returns the log path as passed into the constructor. + * Returns the additional writer information into the constructor. */ - const string Path() const { return path; } + const WriterInfo& Info() const { return info; } /** * Returns the number of log fields as passed into the constructor. @@ -185,7 +205,7 @@ protected: * disabled and eventually deleted. When returning false, an * implementation should also call Error() to indicate what happened. */ - virtual bool DoInit(string path, int num_fields, + virtual bool DoInit(const WriterInfo& info, int num_fields, const threading::Field* const* fields) = 0; /** @@ -299,7 +319,7 @@ private: // this class, it's running in a different thread! WriterFrontend* frontend; - string path; // Log path. + WriterInfo info; // Meta information as passed to Init(). int num_fields; // Number of log fields. const threading::Field* const* fields; // Log fields. bool buffering; // True if buffering is enabled. diff --git a/src/logging/WriterFrontend.cc b/src/logging/WriterFrontend.cc index 33c9c04c63..6ad40757d6 100644 --- a/src/logging/WriterFrontend.cc +++ b/src/logging/WriterFrontend.cc @@ -2,6 +2,7 @@ #include "Net.h" #include "threading/SerialTypes.h" +#include "Manager.h" #include "WriterFrontend.h" #include "WriterBackend.h" @@ -15,14 +16,14 @@ namespace logging { class InitMessage : public threading::InputMessage { public: - InitMessage(WriterBackend* backend, const string path, const int num_fields, const Field* const* fields) + InitMessage(WriterBackend* backend, const WriterBackend::WriterInfo& info, const int num_fields, const Field* const* fields) : threading::InputMessage("Init", backend), - path(path), num_fields(num_fields), fields(fields) { } + info(info), num_fields(num_fields), fields(fields) { } - virtual bool Process() { return Object()->Init(path, num_fields, fields); } + virtual bool Process() { return Object()->Init(info, num_fields, fields); } private: - const string path; + WriterBackend::WriterInfo info; const int num_fields; const Field * const* fields; }; @@ -134,10 +135,10 @@ WriterFrontend::~WriterFrontend() string WriterFrontend::Name() const { - if ( path.size() ) + if ( info.path.size() ) return ty_name; - return ty_name + "/" + path; + return ty_name + "/" + info.path; } void WriterFrontend::Stop() @@ -149,7 +150,7 @@ void WriterFrontend::Stop() backend->Stop(); } -void WriterFrontend::Init(string arg_path, int arg_num_fields, const Field* const * arg_fields) +void WriterFrontend::Init(const WriterBackend::WriterInfo& arg_info, int arg_num_fields, const Field* const * arg_fields) { if ( disabled ) return; @@ -157,19 +158,19 @@ void WriterFrontend::Init(string arg_path, int arg_num_fields, const Field* cons if ( initialized ) reporter->InternalError("writer initialize twice"); - path = arg_path; + info = arg_info; num_fields = arg_num_fields; fields = arg_fields; initialized = true; if ( backend ) - backend->SendIn(new InitMessage(backend, arg_path, arg_num_fields, arg_fields)); + backend->SendIn(new InitMessage(backend, arg_info, arg_num_fields, arg_fields)); if ( remote ) remote_serializer->SendLogCreateWriter(stream, writer, - arg_path, + arg_info, arg_num_fields, arg_fields); @@ -183,7 +184,7 @@ void WriterFrontend::Write(int num_fields, Value** vals) if ( remote ) remote_serializer->SendLogWrite(stream, writer, - path, + info.path, num_fields, vals); diff --git a/src/logging/WriterFrontend.h b/src/logging/WriterFrontend.h index b83250a5b8..8a0dce4645 100644 --- a/src/logging/WriterFrontend.h +++ b/src/logging/WriterFrontend.h @@ -3,13 +3,13 @@ #ifndef LOGGING_WRITERFRONTEND_H #define LOGGING_WRITERFRONTEND_H -#include "Manager.h" +#include "WriterBackend.h" #include "threading/MsgThread.h" namespace logging { -class WriterBackend; +class Manager; /** * Bridge class between the logging::Manager and backend writer threads. The @@ -68,7 +68,7 @@ public: * * This method must only be called from the main thread. */ - void Init(string path, int num_fields, const threading::Field* const* fields); + void Init(const WriterBackend::WriterInfo& info, int num_fields, const threading::Field* const* fields); /** * Write out a record. @@ -169,9 +169,9 @@ public: bool Disabled() { return disabled; } /** - * Returns the log path as passed into the constructor. + * Returns the additional writer information as passed into the constructor. */ - const string Path() const { return path; } + const WriterBackend::WriterInfo& Info() const { return info; } /** * Returns the number of log fields as passed into the constructor. @@ -207,7 +207,7 @@ protected: bool remote; // True if loggin remotely. string ty_name; // Name of the backend type. Set by the manager. - string path; // The log path. + WriterBackend::WriterInfo info; // The writer information. int num_fields; // The number of log fields. const threading::Field* const* fields; // The log fields. diff --git a/src/logging/writers/Ascii.cc b/src/logging/writers/Ascii.cc index 1e7a55c34c..6e5ceef678 100644 --- a/src/logging/writers/Ascii.cc +++ b/src/logging/writers/Ascii.cc @@ -69,8 +69,10 @@ bool Ascii::WriteHeaderField(const string& key, const string& val) return (fwrite(str.c_str(), str.length(), 1, file) == 1); } -bool Ascii::DoInit(string path, int num_fields, const Field* const * fields) +bool Ascii::DoInit(const WriterInfo& info, int num_fields, const Field* const * fields) { + string path = info.path; + if ( output_to_stdout ) path = "/dev/stdout"; @@ -290,7 +292,7 @@ bool Ascii::DoWrite(int num_fields, const Field* const * fields, Value** vals) { if ( ! file ) - DoInit(Path(), NumFields(), Fields()); + DoInit(Info(), NumFields(), Fields()); desc.Clear(); @@ -320,7 +322,7 @@ bool Ascii::DoWrite(int num_fields, const Field* const * fields, bool Ascii::DoRotate(string rotated_path, double open, double close, bool terminating) { // Don't rotate special files or if there's not one currently open. - if ( ! file || IsSpecial(Path()) ) + if ( ! file || IsSpecial(Info().path) ) return true; fclose(file); diff --git a/src/logging/writers/Ascii.h b/src/logging/writers/Ascii.h index 6f507aff01..a95e644d49 100644 --- a/src/logging/writers/Ascii.h +++ b/src/logging/writers/Ascii.h @@ -19,7 +19,7 @@ public: static string LogExt(); protected: - virtual bool DoInit(string path, int num_fields, + virtual bool DoInit(const WriterInfo& info, int num_fields, const threading::Field* const* fields); virtual bool DoWrite(int num_fields, const threading::Field* const* fields, threading::Value** vals); diff --git a/src/logging/writers/DataSeries.cc b/src/logging/writers/DataSeries.cc index 1d5a6ea4da..b34ea3412a 100644 --- a/src/logging/writers/DataSeries.cc +++ b/src/logging/writers/DataSeries.cc @@ -263,7 +263,7 @@ bool DataSeries::OpenLog(string path) return true; } -bool DataSeries::DoInit(string path, int num_fields, const threading::Field* const * fields) +bool DataSeries::DoInit(const WriterInfo& info, int num_fields, const threading::Field* const * fields) { // We first construct an XML schema thing (and, if ds_dump_schema is // set, dump it to path + ".ds.xml"). Assuming that goes well, we @@ -298,11 +298,11 @@ bool DataSeries::DoInit(string path, int num_fields, const threading::Field* con schema_list.push_back(val); } - string schema = BuildDSSchemaFromFieldTypes(schema_list, path); + string schema = BuildDSSchemaFromFieldTypes(schema_list, info.path); if( ds_dump_schema ) { - FILE* pFile = fopen ( string(path + ".ds.xml").c_str() , "wb" ); + FILE* pFile = fopen ( string(info.path + ".ds.xml").c_str() , "wb" ); if( pFile ) { @@ -340,7 +340,7 @@ bool DataSeries::DoInit(string path, int num_fields, const threading::Field* con log_type = log_types.registerTypePtr(schema); log_series.setType(log_type); - return OpenLog(path); + return OpenLog(info.path); } bool DataSeries::DoFlush() @@ -401,7 +401,7 @@ bool DataSeries::DoRotate(string rotated_path, double open, double close, bool t // size will be (much) larger. CloseLog(); - string dsname = Path() + ".ds"; + string dsname = Info().path + ".ds"; string nname = rotated_path + ".ds"; rename(dsname.c_str(), nname.c_str()); @@ -411,7 +411,7 @@ bool DataSeries::DoRotate(string rotated_path, double open, double close, bool t return false; } - return OpenLog(Path()); + return OpenLog(Info().path); } bool DataSeries::DoSetBuf(bool enabled) diff --git a/src/logging/writers/DataSeries.h b/src/logging/writers/DataSeries.h index 0d9ab67e95..0ae3572b76 100644 --- a/src/logging/writers/DataSeries.h +++ b/src/logging/writers/DataSeries.h @@ -26,7 +26,7 @@ public: protected: // Overidden from WriterBackend. - virtual bool DoInit(string path, int num_fields, + virtual bool DoInit(const WriterInfo& info, int num_fields, const threading::Field* const * fields); virtual bool DoWrite(int num_fields, const threading::Field* const* fields, diff --git a/src/logging/writers/None.cc b/src/logging/writers/None.cc index a9a7872f85..e133394722 100644 --- a/src/logging/writers/None.cc +++ b/src/logging/writers/None.cc @@ -6,9 +6,9 @@ using namespace writer; bool None::DoRotate(string rotated_path, double open, double close, bool terminating) { - if ( ! FinishedRotation(string("/dev/null"), Path(), open, close, terminating)) + if ( ! FinishedRotation(string("/dev/null"), Info().path, open, close, terminating)) { - Error(Fmt("error rotating %s", Path().c_str())); + Error(Fmt("error rotating %s", Info().path.c_str())); return false; } diff --git a/src/logging/writers/None.h b/src/logging/writers/None.h index cce48953d1..89ba690e09 100644 --- a/src/logging/writers/None.h +++ b/src/logging/writers/None.h @@ -18,7 +18,7 @@ public: { return new None(frontend); } protected: - virtual bool DoInit(string path, int num_fields, + virtual bool DoInit(const WriterInfo& info, int num_fields, const threading::Field* const * fields) { return true; } virtual bool DoWrite(int num_fields, const threading::Field* const* fields, From 19eea409c38fdefae7cd9113b23f9d7c6fc46285 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Thu, 21 Jun 2012 17:42:33 -0700 Subject: [PATCH 2/2] Extending the log writer DoInit() API. We now pass in a Info struct that contains: - the path name (as before) - the rotation interval - the log_rotate_base_time in seconds - a table of key/value pairs with further configuration options. To fill the table, log filters have a new field "config: table[string] of strings". This gives a way to pass arbitrary values from script-land to writers. Interpretation is left up to the writer. Also splits calc_next_rotate() into two functions, one of which is thread-safe and can be used with the log_rotate_base_time value from DoInit(). Includes also updates to the None writer: - It gets its own script writers/none.bro. - New bool option LogNone::debug to enable debug output. It then prints out all the values passed to DoInit(). That's used by a btest test to ensure the new DoInit() values are right. - Fixed a bug that prevented Bro from terminating.. (scripts.base.frameworks.logging.rotate-custom currently fails. Haven't yet investigated why.) --- scripts/base/frameworks/logging/__load__.bro | 1 + scripts/base/frameworks/logging/main.bro | 6 +++ .../base/frameworks/logging/writers/none.bro | 17 ++++++++ src/File.cc | 3 +- src/Val.cc | 1 + src/bro.bif | 4 +- src/logging.bif | 6 +++ src/logging/Manager.cc | 36 +++++++++++++++-- src/logging/WriterBackend.cc | 40 ++++++++++++++++++- src/logging/WriterBackend.h | 18 +++++++++ src/logging/writers/None.cc | 27 +++++++++++++ src/logging/writers/None.h | 4 +- src/util.cc | 25 +++++++----- src/util.h | 17 +++++++- .../output | 12 ++++++ .../base/frameworks/logging/none-debug.bro | 37 +++++++++++++++++ 16 files changed, 231 insertions(+), 23 deletions(-) create mode 100644 scripts/base/frameworks/logging/writers/none.bro create mode 100644 testing/btest/Baseline/scripts.base.frameworks.logging.none-debug/output create mode 100644 testing/btest/scripts/base/frameworks/logging/none-debug.bro diff --git a/scripts/base/frameworks/logging/__load__.bro b/scripts/base/frameworks/logging/__load__.bro index 17e03e2ef7..be44a7e34f 100644 --- a/scripts/base/frameworks/logging/__load__.bro +++ b/scripts/base/frameworks/logging/__load__.bro @@ -2,3 +2,4 @@ @load ./postprocessors @load ./writers/ascii @load ./writers/dataseries +@load ./writers/none diff --git a/scripts/base/frameworks/logging/main.bro b/scripts/base/frameworks/logging/main.bro index bec5f31dc6..9936ae44b1 100644 --- a/scripts/base/frameworks/logging/main.bro +++ b/scripts/base/frameworks/logging/main.bro @@ -138,6 +138,10 @@ export { ## Callback function to trigger for rotated files. If not set, the ## default comes out of :bro:id:`Log::default_rotation_postprocessors`. postprocessor: function(info: RotationInfo) : bool &optional; + + ## A key/value table that will be passed on the writer. + ## Interpretation of the values is left to the writer. + config: table[string] of string &default=table(); }; ## Sentinel value for indicating that a filter was not found when looked up. @@ -327,6 +331,8 @@ function __default_rotation_postprocessor(info: RotationInfo) : bool { if ( info$writer in default_rotation_postprocessors ) return default_rotation_postprocessors[info$writer](info); + + return F; } function default_path_func(id: ID, path: string, rec: any) : string diff --git a/scripts/base/frameworks/logging/writers/none.bro b/scripts/base/frameworks/logging/writers/none.bro new file mode 100644 index 0000000000..22d83bd8ec --- /dev/null +++ b/scripts/base/frameworks/logging/writers/none.bro @@ -0,0 +1,17 @@ +##! Interface for the None log writer. Thiis writer is mainly for debugging. + +module LogNone; + +export { + ## If true, output some debugging output that can be useful for unit + ##testing the logging framework. + const debug = F &redef; +} + +function default_rotation_postprocessor_func(info: Log::RotationInfo) : bool + { + return T; + } + +redef Log::default_rotation_postprocessors += { [Log::WRITER_NONE] = default_rotation_postprocessor_func }; + diff --git a/src/File.cc b/src/File.cc index 8b432f4428..20e845c09f 100644 --- a/src/File.cc +++ b/src/File.cc @@ -572,8 +572,9 @@ void BroFile::InstallRotateTimer() const char* base_time = log_rotate_base_time ? log_rotate_base_time->AsString()->CheckString() : 0; + double base = parse_rotate_base_time(base_time); double delta_t = - calc_next_rotate(rotate_interval, base_time); + calc_next_rotate(network_time, rotate_interval, base); rotate_timer = new RotateTimer(network_time + delta_t, this, true); } diff --git a/src/Val.cc b/src/Val.cc index 32a3c367bb..8a8c2b18c0 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -1651,6 +1651,7 @@ int TableVal::RemoveFrom(Val* val) const while ( (v = tbl->NextEntry(k, c)) ) { Val* index = RecoverIndex(k); + Unref(index); Unref(t->Delete(k)); delete k; diff --git a/src/bro.bif b/src/bro.bif index 1feccb8639..b1f33c9c46 100644 --- a/src/bro.bif +++ b/src/bro.bif @@ -4814,7 +4814,9 @@ function calc_next_rotate%(i: interval%) : interval %{ const char* base_time = log_rotate_base_time ? log_rotate_base_time->AsString()->CheckString() : 0; - return new Val(calc_next_rotate(i, base_time), TYPE_INTERVAL); + + double base = parse_rotate_base_time(base_time); + return new Val(calc_next_rotate(network_time, i, base), TYPE_INTERVAL); %} ## Returns the size of a given file. diff --git a/src/logging.bif b/src/logging.bif index efc6ed0b4b..d25e89c33c 100644 --- a/src/logging.bif +++ b/src/logging.bif @@ -81,3 +81,9 @@ const extent_size: count; const dump_schema: bool; const use_integer_for_time: bool; const num_threads: count; + +# Options for the None writer. + +module LogNone; + +const debug: bool; diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index b30ee26534..23b6f070a1 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -51,6 +51,7 @@ struct Manager::Filter { string path; Val* path_val; EnumVal* writer; + TableVal* config; bool local; bool remote; double interval; @@ -519,6 +520,7 @@ bool Manager::AddFilter(EnumVal* id, RecordVal* fval) Val* log_remote = fval->LookupWithDefault(rtype->FieldOffset("log_remote")); Val* interv = fval->LookupWithDefault(rtype->FieldOffset("interv")); Val* postprocessor = fval->LookupWithDefault(rtype->FieldOffset("postprocessor")); + Val* config = fval->LookupWithDefault(rtype->FieldOffset("config")); Filter* filter = new Filter; filter->name = name->AsString()->CheckString(); @@ -530,6 +532,7 @@ bool Manager::AddFilter(EnumVal* id, RecordVal* fval) filter->remote = log_remote->AsBool(); filter->interval = interv->AsInterval(); filter->postprocessor = postprocessor ? postprocessor->AsFunc() : 0; + filter->config = config->Ref()->AsTableVal(); Unref(name); Unref(pred); @@ -538,6 +541,7 @@ bool Manager::AddFilter(EnumVal* id, RecordVal* fval) Unref(log_remote); Unref(interv); Unref(postprocessor); + Unref(config); // Build the list of fields that the filter wants included, including // potentially rolling out fields. @@ -768,6 +772,22 @@ bool Manager::Write(EnumVal* id, RecordVal* columns) WriterBackend::WriterInfo info; info.path = path; + HashKey* k; + IterCookie* c = filter->config->AsTable()->InitForIteration(); + + TableEntryVal* v; + while ( (v = filter->config->AsTable()->NextEntry(k, c)) ) + { + ListVal* index = filter->config->RecoverIndex(k); + string key = index->Index(0)->AsString()->CheckString(); + string value = v->Value()->AsString()->CheckString(); + info.config.insert(std::make_pair(key, value)); + Unref(index); + delete k; + } + + // CreateWriter() will set the other fields in info. + writer = CreateWriter(stream->id, filter->writer, info, filter->num_fields, arg_fields, filter->local, filter->remote); @@ -777,7 +797,6 @@ bool Manager::Write(EnumVal* id, RecordVal* columns) Unref(columns); return false; } - } // Alright, can do the write now. @@ -977,8 +996,6 @@ WriterFrontend* Manager::CreateWriter(EnumVal* id, EnumVal* writer, const Writer WriterFrontend* writer_obj = new WriterFrontend(id, writer, local, remote); assert(writer_obj); - writer_obj->Init(info, num_fields, fields); - WriterInfo* winfo = new WriterInfo; winfo->type = writer->Ref()->AsEnumVal(); winfo->writer = writer_obj; @@ -1020,6 +1037,16 @@ WriterFrontend* Manager::CreateWriter(EnumVal* id, EnumVal* writer, const Writer Stream::WriterMap::value_type(Stream::WriterPathPair(writer->AsEnum(), info.path), winfo)); + // Still need to set the WriterInfo's rotation parameters, which we + // computed above. + const char* base_time = log_rotate_base_time ? + log_rotate_base_time->AsString()->CheckString() : 0; + + winfo->info.rotation_interval = winfo->interval; + winfo->info.rotation_base = parse_rotate_base_time(base_time); + + writer_obj->Init(winfo->info, num_fields, fields); + return writer_obj; } @@ -1223,8 +1250,9 @@ void Manager::InstallRotationTimer(WriterInfo* winfo) const char* base_time = log_rotate_base_time ? log_rotate_base_time->AsString()->CheckString() : 0; + double base = parse_rotate_base_time(base_time); double delta_t = - calc_next_rotate(rotation_interval, base_time); + calc_next_rotate(network_time, rotation_interval, base); winfo->rotation_timer = new RotationTimer(network_time + delta_t, winfo, true); diff --git a/src/logging/WriterBackend.cc b/src/logging/WriterBackend.cc index 35bb27d27b..a31b0ebc0f 100644 --- a/src/logging/WriterBackend.cc +++ b/src/logging/WriterBackend.cc @@ -63,12 +63,48 @@ using namespace logging; bool WriterBackend::WriterInfo::Read(SerializationFormat* fmt) { - return fmt->Read(&path, "path"); + int size; + + if ( ! (fmt->Read(&path, "path") && + fmt->Read(&rotation_base, "rotation_base") && + fmt->Read(&rotation_interval, "rotation_interval") && + fmt->Read(&size, "config_size")) ) + return false; + + config.clear(); + + while ( size ) + { + string value; + string key; + + if ( ! (fmt->Read(&value, "config-value") && fmt->Read(&value, "config-key")) ) + return false; + + config.insert(std::make_pair(value, key)); + } + + return true; } + bool WriterBackend::WriterInfo::Write(SerializationFormat* fmt) const { - return fmt->Write(path, "path"); + int size = config.size(); + + if ( ! (fmt->Write(path, "path") && + fmt->Write(rotation_base, "rotation_base") && + fmt->Write(rotation_interval, "rotation_interval") && + fmt->Write(size, "config_size")) ) + return false; + + for ( config_map::const_iterator i = config.begin(); i != config.end(); ++i ) + { + if ( ! (fmt->Write(i->first, "config-value") && fmt->Write(i->second, "config-key")) ) + return false; + } + + return true; } WriterBackend::WriterBackend(WriterFrontend* arg_frontend) : MsgThread() diff --git a/src/logging/WriterBackend.h b/src/logging/WriterBackend.h index 30e1995430..84c43818a6 100644 --- a/src/logging/WriterBackend.h +++ b/src/logging/WriterBackend.h @@ -48,6 +48,8 @@ public: */ struct WriterInfo { + typedef std::map config_map; + /** * A string left to the interpretation of the writer * implementation; it corresponds to the value configured on @@ -55,6 +57,22 @@ public: */ string path; + /** + * The rotation interval as configured for this writer. + */ + double rotation_interval; + + /** + * The parsed value of log_rotate_base_time in seconds. + */ + double rotation_base; + + /** + * A map of key/value pairs corresponding to the relevant + * filter's "config" table. + */ + std::map config; + private: friend class ::RemoteSerializer; diff --git a/src/logging/writers/None.cc b/src/logging/writers/None.cc index e133394722..acf9355cf7 100644 --- a/src/logging/writers/None.cc +++ b/src/logging/writers/None.cc @@ -1,9 +1,36 @@ #include "None.h" +#include "NetVar.h" using namespace logging; using namespace writer; +bool None::DoInit(const WriterInfo& info, int num_fields, + const threading::Field* const * fields) + { + if ( BifConst::LogNone::debug ) + { + std::cout << "[logging::writer::None]" << std::endl; + std::cout << " path=" << info.path << std::endl; + std::cout << " rotation_interval=" << info.rotation_interval << std::endl; + std::cout << " rotation_base=" << info.rotation_base << std::endl; + + for ( std::map::const_iterator i = info.config.begin(); i != info.config.end(); i++ ) + std::cout << " config[" << i->first << "] = " << i->second << std::endl; + + for ( int i = 0; i < num_fields; i++ ) + { + const threading::Field* field = fields[i]; + std::cout << " field " << field->name << ": " + << type_name(field->type) << std::endl; + } + + std::cout << std::endl; + } + + return true; + } + bool None::DoRotate(string rotated_path, double open, double close, bool terminating) { if ( ! FinishedRotation(string("/dev/null"), Info().path, open, close, terminating)) diff --git a/src/logging/writers/None.h b/src/logging/writers/None.h index 89ba690e09..7e2e4ef4eb 100644 --- a/src/logging/writers/None.h +++ b/src/logging/writers/None.h @@ -19,7 +19,7 @@ public: protected: virtual bool DoInit(const WriterInfo& info, int num_fields, - const threading::Field* const * fields) { return true; } + const threading::Field* const * fields); virtual bool DoWrite(int num_fields, const threading::Field* const* fields, threading::Value** vals) { return true; } @@ -27,7 +27,7 @@ protected: virtual bool DoRotate(string rotated_path, double open, double close, bool terminating); virtual bool DoFlush() { return true; } - virtual bool DoFinish() { return true; } + virtual bool DoFinish() { WriterBackend::DoFinish(); return true; } }; } diff --git a/src/util.cc b/src/util.cc index 798be400d1..16df52b987 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1082,18 +1082,8 @@ const char* log_file_name(const char* tag) return fmt("%s.%s", tag, (env ? env : "log")); } -double calc_next_rotate(double interval, const char* rotate_base_time) +double parse_rotate_base_time(const char* rotate_base_time) { - double current = network_time; - - // Calculate start of day. - time_t teatime = time_t(current); - - struct tm t; - t = *localtime(&teatime); - t.tm_hour = t.tm_min = t.tm_sec = 0; - double startofday = mktime(&t); - double base = -1; if ( rotate_base_time && rotate_base_time[0] != '\0' ) @@ -1105,6 +1095,19 @@ double calc_next_rotate(double interval, const char* rotate_base_time) base = t.tm_min * 60 + t.tm_hour * 60 * 60; } + return base; + } + +double calc_next_rotate(double current, double interval, double base) + { + // Calculate start of day. + time_t teatime = time_t(current); + + struct tm t; + t = *localtime_r(&teatime, &t); + t.tm_hour = t.tm_min = t.tm_sec = 0; + double startofday = mktime(&t); + if ( base < 0 ) // No base time given. To get nice timestamps, we round // the time up to the next multiple of the rotation interval. diff --git a/src/util.h b/src/util.h index 6b237edfd8..6ca584900c 100644 --- a/src/util.h +++ b/src/util.h @@ -193,9 +193,22 @@ extern FILE* rotate_file(const char* name, RecordVal* rotate_info); // This mimics the script-level function with the same name. const char* log_file_name(const char* tag); +// Parse a time string of the form "HH:MM" (as used for the rotation base +// time) into a double representing the number of seconds. Returns -1 if the +// string cannot be parsed. The function's result is intended to be used with +// calc_next_rotate(). +// +// This function is not thread-safe. +double parse_rotate_base_time(const char* rotate_base_time); + // Calculate the duration until the next time a file is to be rotated, based -// on the given rotate_interval and rotate_base_time. -double calc_next_rotate(double rotate_interval, const char* rotate_base_time); +// on the given rotate_interval and rotate_base_time. 'current' the the +// current time to be used as base, 'rotate_interval' the rotation interval, +// and 'base' the value returned by parse_rotate_base_time(). For the latter, +// if the function returned -1, that's fine, calc_next_rotate() handles that. +// +// This function is thread-safe. +double calc_next_rotate(double current, double rotate_interval, double base); // Terminates processing gracefully, similar to pressing CTRL-C. void terminate_processing(); diff --git a/testing/btest/Baseline/scripts.base.frameworks.logging.none-debug/output b/testing/btest/Baseline/scripts.base.frameworks.logging.none-debug/output new file mode 100644 index 0000000000..b2a8921c38 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.logging.none-debug/output @@ -0,0 +1,12 @@ +[logging::writer::None] + path=ssh + rotation_interval=3600 + rotation_base=300 + config[foo] = bar + config[foo2] = bar2 + field id.orig_p: port + field id.resp_h: addr + field id.resp_p: port + field status: string + field country: string + diff --git a/testing/btest/scripts/base/frameworks/logging/none-debug.bro b/testing/btest/scripts/base/frameworks/logging/none-debug.bro new file mode 100644 index 0000000000..5d2e98323a --- /dev/null +++ b/testing/btest/scripts/base/frameworks/logging/none-debug.bro @@ -0,0 +1,37 @@ +# +# @TEST-EXEC: bro -b %INPUT >output +# @TEST-EXEC: btest-diff output + +redef Log::default_writer = Log::WRITER_NONE; +redef LogNone::debug = T; +redef Log::default_rotation_interval= 1hr; +redef log_rotate_base_time = "00:05"; + +module SSH; + +export { + redef enum Log::ID += { LOG }; + + type Log: record { + t: time; + id: conn_id; # Will be rolled out into individual columns. + status: string &optional; + country: string &default="unknown"; + } &log; +} + +event bro_init() +{ + local config: table[string] of string; + config["foo"]="bar"; + config["foo2"]="bar2"; + + local cid = [$orig_h=1.2.3.4, $orig_p=1234/tcp, $resp_h=2.3.4.5, $resp_p=80/tcp]; + + Log::create_stream(SSH::LOG, [$columns=Log]); + + Log::remove_default_filter(SSH::LOG); + Log::add_filter(SSH::LOG, [$name="f1", $exclude=set("t", "id.orig_h"), $config=config]); + Log::write(SSH::LOG, [$t=network_time(), $id=cid, $status="success"]); +} +