You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2013/10/31 16:18:34 UTC

[15/17] git commit: TS-2302: separate predefined formats from LogFormat

TS-2302: separate predefined formats from LogFormat

LogFormat now only knows about text and custom logs. LogConfig still
have some knowledge of custom formats, but it is largeley separated
and doesn't have a LogFormat dependency any more. Remove various
predefined log constants and apply addition const to pass format
strings around.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/b95d1e89
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/b95d1e89
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/b95d1e89

Branch: refs/heads/master
Commit: b95d1e89a83377f9a0e8b845e19e510153124deb
Parents: 1228c65
Author: James Peach <jp...@apache.org>
Authored: Thu Sep 26 21:59:28 2013 -0700
Committer: James Peach <jp...@apache.org>
Committed: Thu Oct 31 08:16:28 2013 -0700

----------------------------------------------------------------------
 proxy/logcat.cc                |  11 ++--
 proxy/logging/Log.cc           |   2 +-
 proxy/logging/LogBuffer.cc     |   7 ++-
 proxy/logging/LogBuffer.h      |   6 +--
 proxy/logging/LogConfig.cc     |  92 ++++++---------------------------
 proxy/logging/LogConfig.h      |  27 ++--------
 proxy/logging/LogFile.cc       |   4 +-
 proxy/logging/LogFile.h        |   5 +-
 proxy/logging/LogFormat.cc     | 100 +++++++-----------------------------
 proxy/logging/LogFormat.h      |  36 +++++++++----
 proxy/logging/LogFormatType.h  |  48 -----------------
 proxy/logging/LogObject.cc     |   2 +-
 proxy/logging/LogPredefined.cc |  30 ++++++-----
 proxy/logging/LogPredefined.h  |   9 +++-
 proxy/logging/LogUtils.cc      |   2 -
 proxy/logging/Makefile.am      |   1 -
 16 files changed, 104 insertions(+), 278 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logcat.cc
----------------------------------------------------------------------
diff --git a/proxy/logcat.cc b/proxy/logcat.cc
index ae851bd..e2faea8 100644
--- a/proxy/logcat.cc
+++ b/proxy/logcat.cc
@@ -43,6 +43,7 @@
 #include "LogBuffer.h"
 #include "LogUtils.h"
 #include "LogSock.h"
+#include "LogPredefined.h"
 #include "Log.h"
 
 // logcat-specific command-line flags
@@ -163,15 +164,15 @@ process_file(int in_fd, int out_fd)
     // see if there is an alternate format request from the command
     // line
     //
-    char *alt_format = NULL;
+    const char * alt_format = NULL;
     if (squid_flag)
-      alt_format = (char *) LogFormat::squid_format;
+      alt_format = PreDefinedFormatInfo::squid;
     if (clf_flag)
-      alt_format = (char *) LogFormat::common_format;
+      alt_format = PreDefinedFormatInfo::common;
     if (elf_flag)
-      alt_format = (char *) LogFormat::extended_format;
+      alt_format = PreDefinedFormatInfo::extended;
     if (elf2_flag)
-      alt_format = (char *) LogFormat::extended2_format;
+      alt_format = PreDefinedFormatInfo::extended2;
 
     // convert the buffer to ascii entries and place onto stdout
     //

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/Log.cc
----------------------------------------------------------------------
diff --git a/proxy/logging/Log.cc b/proxy/logging/Log.cc
index a5e5c22..a263944 100644
--- a/proxy/logging/Log.cc
+++ b/proxy/logging/Log.cc
@@ -1011,7 +1011,7 @@ Log::init_when_enabled()
     }
     // setup global scrap object
     //
-    global_scrap_format = NEW(new LogFormat(TEXT_LOG));
+    global_scrap_format = MakeTextLogFormat();
     global_scrap_object =
       NEW(new LogObject(global_scrap_format,
                         Log::config->logfile_dir,

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogBuffer.cc
----------------------------------------------------------------------
diff --git a/proxy/logging/LogBuffer.cc b/proxy/logging/LogBuffer.cc
index d1714ab..45452ee 100644
--- a/proxy/logging/LogBuffer.cc
+++ b/proxy/logging/LogBuffer.cc
@@ -43,7 +43,6 @@
 #include "LogAccess.h"
 #include "LogConfig.h"
 #include "LogBuffer.h"
-#include "LogFormatType.h"
 #include "Log.h"
 
 
@@ -626,11 +625,11 @@ LogBuffer::resolve_custom_entry(LogFieldList * fieldlist,
   -------------------------------------------------------------------------*/
 int
 LogBuffer::to_ascii(LogEntryHeader * entry, LogFormatType type,
-                    char *buf, int buf_len, char *symbol_str, char *printf_str,
-                    unsigned buffer_version, char *alt_format)
+                    char *buf, int buf_len, const char *symbol_str, char *printf_str,
+                    unsigned buffer_version, const char *alt_format)
 {
   ink_assert(entry != NULL);
-  ink_assert(type >= 0 && type < N_LOG_TYPES);
+  ink_assert(type == CUSTOM_LOG || type == TEXT_LOG);
   ink_assert(buf != NULL);
 
   char *read_from;              // keeps track of where we're reading from entry

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogBuffer.h
----------------------------------------------------------------------
diff --git a/proxy/logging/LogBuffer.h b/proxy/logging/LogBuffer.h
index acf67fe..08ff92e 100644
--- a/proxy/logging/LogBuffer.h
+++ b/proxy/logging/LogBuffer.h
@@ -26,7 +26,7 @@
 #define LOG_BUFFER_H
 
 #include "libts.h"
-#include "LogFormatType.h"
+#include "LogFormat.h"
 #include "LogLimits.h"
 #include "LogAccess.h"
 
@@ -181,8 +181,8 @@ public:
   // static functions
   static size_t max_entry_bytes();
   static int to_ascii(LogEntryHeader * entry, LogFormatType type,
-                      char *buf, int max_len, char *symbol_str, char *printf_str,
-                      unsigned buffer_version, char *alt_format = NULL);
+                      char *buf, int max_len, const char *symbol_str, char *printf_str,
+                      unsigned buffer_version, const char *alt_format = NULL);
   static int resolve_custom_entry(LogFieldList * fieldlist,
                                   char *printf_str, char *read_from, char *write_to,
                                   int write_to_len, long timestamp, long timestamp_us,

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogConfig.cc
----------------------------------------------------------------------
diff --git a/proxy/logging/LogConfig.cc b/proxy/logging/LogConfig.cc
index 616776a..02c254c 100644
--- a/proxy/logging/LogConfig.cc
+++ b/proxy/logging/LogConfig.cc
@@ -35,7 +35,6 @@
 #include "List.h"
 #include "InkXml.h"
 
-#include "LogFormatType.h"
 #include "LogField.h"
 #include "LogFilter.h"
 #include "LogFormat.h"
@@ -49,6 +48,7 @@
 #include "SimpleTokenizer.h"
 
 #include "LogCollationAccept.h"
+#include "LogPredefined.h"
 
 #define DISK_IS_CONFIG_FULL_MESSAGE \
     "Access logging to local log directory suspended - " \
@@ -782,63 +782,6 @@ LogConfig::display(FILE * fd)
   global_format_list.display(fd);
 }
 
-//-----------------------------------------------------------------------------
-// setup_pre_defined_info
-//
-// This function adds all the pre defined formats to the global_format_list
-// and gathers the information for the active formats in a single place
-// (an entry in a PreDefinedFormatInfo list)
-//
-void
-LogConfig::setup_pre_defined_info(PreDefinedFormatInfoList * preDefInfoList)
-{
-  LogFormat *fmt;
-  PreDefinedFormatInfo *pdfi;
-
-  Log::config->xuid_logging_enabled = xuid_logging_enabled;
-  fmt = NEW(new LogFormat(SQUID_LOG));
-
-  ink_assert(fmt != 0);
-  global_format_list.add(fmt, false);
-  Debug("log", "squid format added to the global format list");
-
-  if (squid_log_enabled) {
-    pdfi = NEW(new PreDefinedFormatInfo(fmt, squid_log_name, squid_log_is_ascii, squid_log_header));
-    preDefInfoList->enqueue(pdfi);
-  }
-
-  fmt = NEW(new LogFormat(COMMON_LOG));
-  ink_assert(fmt != 0);
-  global_format_list.add(fmt, false);
-  Debug("log", "common format added to the global format list");
-
-  if (common_log_enabled) {
-    pdfi = NEW(new PreDefinedFormatInfo(fmt, common_log_name, common_log_is_ascii, common_log_header));
-    preDefInfoList->enqueue(pdfi);
-  }
-
-  fmt = NEW(new LogFormat(EXTENDED_LOG));
-  ink_assert(fmt != 0);
-  global_format_list.add(fmt, false);
-  Debug("log", "extended format added to the global format list");
-
-  if (extended_log_enabled) {
-    pdfi = NEW(new PreDefinedFormatInfo(fmt, extended_log_name, extended_log_is_ascii, extended_log_header));
-    preDefInfoList->enqueue(pdfi);
-  }
-
-  fmt = NEW(new LogFormat(EXTENDED2_LOG));
-  ink_assert(fmt != 0);
-  global_format_list.add(fmt, false);
-  Debug("log", "extended2 format added to the global format list");
-
-  if (extended2_log_enabled) {
-    pdfi = NEW(new PreDefinedFormatInfo(fmt, extended2_log_name, extended2_log_is_ascii, extended2_log_header));
-    preDefInfoList->enqueue(pdfi);
-  }
-
-}
-
 /*                                                               */
 /* The user defined filters are added to the search_one          */
 /* log object. These filters are defined to filter the images    */
@@ -878,13 +821,13 @@ LogConfig::add_filters_to_search_log_object(const char *format_name)
 //
 
 void
-LogConfig::create_pre_defined_objects_with_filter(const PreDefinedFormatInfoList & pre_def_info_list, size_t num_filters,
+LogConfig::create_pre_defined_objects_with_filter(const PreDefinedFormatList & predef, size_t num_filters,
                                                   LogFilter ** filter, const char *filt_name, bool force_extension)
 {
   PreDefinedFormatInfo *pdi;
 
-  for (pdi = pre_def_info_list.head; pdi != NULL; pdi = (pdi->link).next) {
-    char *obj_fname;
+  for (pdi = predef.formats.head; pdi != NULL; pdi = (pdi->link).next) {
+    const char *obj_fname;
     char obj_filt_fname[PATH_NAME_MAX];
     if (filt_name) {
       ink_string_concatenate_strings_n(obj_filt_fname, PATH_NAME_MAX, pdi->filename, "-", filt_name, NULL);
@@ -943,7 +886,7 @@ LogConfig::create_pre_defined_objects_with_filter(const PreDefinedFormatInfoList
 // pre-defined formats.
 //
 LogFilter *
-LogConfig::split_by_protocol(const PreDefinedFormatInfoList & pre_def_info_list)
+LogConfig::split_by_protocol(const PreDefinedFormatList & predef)
 {
   if (!separate_icp_logs) {
     return NULL;
@@ -968,7 +911,7 @@ LogConfig::split_by_protocol(const PreDefinedFormatInfoList & pre_def_info_list)
   if (separate_icp_logs) {
     if (separate_icp_logs == 1) {
       filter[0] = NEW(new LogFilterInt(filter_name[icp], etype_field, LogFilter::ACCEPT, LogFilter::MATCH, value[icp]));
-      create_pre_defined_objects_with_filter(pre_def_info_list, 1, filter, name[icp]);
+      create_pre_defined_objects_with_filter(predef, 1, filter, name[icp]);
       delete filter[0];
     }
     filter_val[n++] = value[icp];
@@ -991,7 +934,7 @@ LogConfig::split_by_protocol(const PreDefinedFormatInfoList & pre_def_info_list)
 }
 
 size_t
-  LogConfig::split_by_hostname(const PreDefinedFormatInfoList & pre_def_info_list, LogFilter * reject_protocol_filter)
+  LogConfig::split_by_hostname(const PreDefinedFormatList & predef, LogFilter * reject_protocol_filter)
 {
   size_t n_hosts;
   char **host = read_log_hosts_file(&n_hosts);  // allocates memory for array
@@ -1017,7 +960,7 @@ size_t
         NEW(new LogFilterString(filter_name,
                                 shn_field, LogFilter::ACCEPT, LogFilter::CASE_INSENSITIVE_CONTAIN, host[i]));
 
-      create_pre_defined_objects_with_filter(pre_def_info_list, num_filt + 1, rp_ah, host[i], true);
+      create_pre_defined_objects_with_filter(predef, num_filt + 1, rp_ah, host[i], true);
       delete rp_ah[num_filt];
     }
 
@@ -1037,7 +980,7 @@ size_t
     // hosts other than those specified in the hosts file and for
     // those protocols that do not have their own file
     //
-    create_pre_defined_objects_with_filter(pre_def_info_list, num_filt + 1, rp_rh);
+    create_pre_defined_objects_with_filter(predef, num_filt + 1, rp_rh);
     delete rp_rh[num_filt];
 
     delete[]host;               // deallocate memory allocated by
@@ -1075,18 +1018,19 @@ LogConfig::setup_log_objects()
 
   // gather the config information for the pre-defined formats
   //
-  PreDefinedFormatInfoList pre_def_info_list;
-  setup_pre_defined_info(&pre_def_info_list);
+  PreDefinedFormatList predef;
+
+  predef.init(this);
 
   // do protocol splitting
   //
-  LogFilter *reject_protocol_filter = split_by_protocol(pre_def_info_list);
+  LogFilter *reject_protocol_filter = split_by_protocol(predef);
 
   // do host splitting
   //
   size_t num_hosts = 0;
   if (separate_host_logs) {
-    num_hosts = split_by_hostname(pre_def_info_list, reject_protocol_filter);
+    num_hosts = split_by_hostname(predef, reject_protocol_filter);
   }
 
   if (num_hosts == 0) {
@@ -1097,7 +1041,7 @@ LogConfig::setup_log_objects()
     //
     LogFilter *f[1];
     f[0] = reject_protocol_filter;
-    create_pre_defined_objects_with_filter(pre_def_info_list, 1, f);
+    create_pre_defined_objects_with_filter(predef, 1, f);
   }
 
   delete reject_protocol_filter;
@@ -1141,12 +1085,6 @@ LogConfig::setup_log_objects()
   if (is_debug_tag_set("log")) {
     log_object_manager.display();
   }
-
-  PreDefinedFormatInfo *pdfi;
-  while (!pre_def_info_list.empty()) {
-    pdfi = pre_def_info_list.pop();
-    delete pdfi;
-  }
 }
 
 /*-------------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogConfig.h
----------------------------------------------------------------------
diff --git a/proxy/logging/LogConfig.h b/proxy/logging/LogConfig.h
index 2dc5bc6..00e5dd5 100644
--- a/proxy/logging/LogConfig.h
+++ b/proxy/logging/LogConfig.h
@@ -75,8 +75,8 @@ enum
 extern RecRawStatBlock *log_rsb;
 
 struct dirent;
-
 struct LogCollationAccept;
+struct PreDefinedFormatList;
 
 /*-------------------------------------------------------------------------
   LogConfig
@@ -251,33 +251,14 @@ public:
 
 private:
 
-  // this struct collects all the necesary info to build a pre-defined object
-  //
-  struct PreDefinedFormatInfo
-  {
-    LogFormat *format;
-    char *filename;
-    int is_ascii;
-    char *header;
-    LINK(PreDefinedFormatInfo, link);
-
-    PreDefinedFormatInfo(LogFormat * fmt, char *fname, int ascii,
-                         char *hdr):format(fmt), filename(fname), is_ascii(ascii), header(hdr)
-    { }
-  };
-
-
-  typedef Queue<PreDefinedFormatInfo> PreDefinedFormatInfoList;
-
   void read_xml_log_config(int from_memory);
   char **read_log_hosts_file(size_t * nhosts);
 
   void setup_default_values();
   void setup_collation(LogConfig * prev_config);
-  void setup_pre_defined_info(PreDefinedFormatInfoList * pre_def_info_list);
-  LogFilter *split_by_protocol(const PreDefinedFormatInfoList & pre_def_info_list);
-  size_t split_by_hostname(const PreDefinedFormatInfoList & pre_def_info_list, LogFilter * reject_protocol);
-  void create_pre_defined_objects_with_filter(const PreDefinedFormatInfoList &pre_def_info_list, size_t num_filt,
+  LogFilter *split_by_protocol(const PreDefinedFormatList & pre_def_info_list);
+  size_t split_by_hostname(const PreDefinedFormatList & pre_def_info_list, LogFilter * reject_protocol);
+  void create_pre_defined_objects_with_filter(const PreDefinedFormatList &pre_def_info_list, size_t num_filt,
                                               LogFilter ** filter, const char *filt_name = 0, bool force_extension = false);
 
   void add_filters_to_search_log_object(const char *format_name);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogFile.cc
----------------------------------------------------------------------
diff --git a/proxy/logging/LogFile.cc b/proxy/logging/LogFile.cc
index dc564e6..13083a9 100644
--- a/proxy/logging/LogFile.cc
+++ b/proxy/logging/LogFile.cc
@@ -554,7 +554,7 @@ done:
   -------------------------------------------------------------------------*/
 
 int
-LogFile::write_ascii_logbuffer(LogBufferHeader * buffer_header, int fd, const char *path, char *alt_format)
+LogFile::write_ascii_logbuffer(LogBufferHeader * buffer_header, int fd, const char *path, const char *alt_format)
 {
   ink_assert(buffer_header != NULL);
   ink_assert(fd >= 0);
@@ -618,7 +618,7 @@ LogFile::write_ascii_logbuffer(LogBufferHeader * buffer_header, int fd, const ch
 }
 
 int
-LogFile::write_ascii_logbuffer3(LogBufferHeader * buffer_header, char *alt_format)
+LogFile::write_ascii_logbuffer3(LogBufferHeader * buffer_header, const char *alt_format)
 {
   Debug("log-file", "entering LogFile::write_ascii_logbuffer3 for %s " "(this=%p)", m_name, this);
   ink_assert(buffer_header != NULL);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogFile.h
----------------------------------------------------------------------
diff --git a/proxy/logging/LogFile.h b/proxy/logging/LogFile.h
index 144a2b5..68a334b 100644
--- a/proxy/logging/LogFile.h
+++ b/proxy/logging/LogFile.h
@@ -30,7 +30,6 @@
 #include <stdio.h>
 
 #include "libts.h"
-#include "LogFormatType.h"
 #include "LogBufferSink.h"
 
 class LogSock;
@@ -155,8 +154,8 @@ public:
     return (m_file_format == BINARY_LOG ? "binary" : (m_file_format == ASCII_PIPE ? "ascii_pipe" : "ascii"));
   }
 
-  static int write_ascii_logbuffer(LogBufferHeader * buffer_header, int fd, const char *path, char *alt_format = NULL);
-  int write_ascii_logbuffer3(LogBufferHeader * buffer_header, char *alt_format = NULL);
+  static int write_ascii_logbuffer(LogBufferHeader * buffer_header, int fd, const char *path, const char *alt_format = NULL);
+  int write_ascii_logbuffer3(LogBufferHeader * buffer_header, const char *alt_format = NULL);
   static bool rolled_logfile(char *file);
   static bool exists(const char *pathname);
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogFormat.cc
----------------------------------------------------------------------
diff --git a/proxy/logging/LogFormat.cc b/proxy/logging/LogFormat.cc
index cace523..eab34ee 100644
--- a/proxy/logging/LogFormat.cc
+++ b/proxy/logging/LogFormat.cc
@@ -52,31 +52,19 @@
 //
 bool LogFormat::m_tagging_on = false;
 
-// predefined formats
-//
-const char *const LogFormat::squid_format =
-  "%<cqtq> %<ttms> %<chi> %<crc>/%<pssc> %<psql> %<cqhm> %<cquc> %<caun> %<phr>/%<pqsn> %<psct> %<xid>";
-
-const char *const LogFormat::common_format = "%<chi> - %<caun> [%<cqtn>] \"%<cqtx>\" %<pssc> %<pscl>";
-
-const char *const LogFormat::extended_format =
-  "%<chi> - %<caun> [%<cqtn>] \"%<cqtx>\" %<pssc> %<pscl> "
-  "%<sssc> %<sscl> %<cqbl> %<pqbl> %<cqhl> %<pshl> %<pqhl> %<sshl> %<tts>";
-
-const char *const LogFormat::extended2_format = "%<chi> - %<caun> [%<cqtn>] \"%<cqtx>\" %<pssc> %<pscl> "
-  "%<sssc> %<sscl> %<cqbl> %<pqbl> %<cqhl> %<pshl> %<pqhl> %<sshl> %<tts> %<phr> %<cfsc> %<pfsc> %<crc>";
-
 /*-------------------------------------------------------------------------
   LogFormat::setup
   -------------------------------------------------------------------------*/
 
-void
+bool
 LogFormat::setup(const char *name, const char *format_str, unsigned interval_sec)
 {
-  if (name == NULL && format_str == NULL) {
-    Note("No name or field symbols for this format");
-    m_valid = false;
-  } else {
+  if (name == NULL) {
+    Note("missing log format name");
+    return false;
+  }
+
+  if (format_str) {
     const char *tag = " %<phn>";
     const size_t m_format_str_size = strlen(format_str) + (m_tagging_on ? strlen(tag) : 0) + 1;
     m_format_str = (char *)ats_malloc(m_format_str_size);
@@ -100,7 +88,14 @@ LogFormat::setup(const char *name, const char *format_str, unsigned interval_sec
 
     ats_free(fieldlist_str);
     ats_free(printf_str);
+
+    // We are only valid if init_variables() says we are.
+    return true;
   }
+
+  // We don't have a format string (ie. this will be a raw text log, so we are always valid.
+  m_valid = true;
+  return true;
 }
 
 /*-------------------------------------------------------------------------
@@ -176,57 +171,6 @@ LogFormat::init_variables(const char *name, const char *fieldlist_str, const cha
   }
 }
 
-
-/*-------------------------------------------------------------------------
-  LogFormat::LogFormat
-
-  This constructor builds a LogFormat object for one of the pre-defined
-  format types.  Only the type is needed since we know everything else we
-  need to about the pre-defined formats.
-  -------------------------------------------------------------------------*/
-
-LogFormat::LogFormat(LogFormatType type)
-  : m_interval_sec(0),
-    m_interval_next(0),
-    m_agg_marshal_space(NULL),
-    m_valid(false),
-    m_name_str(NULL),
-    m_name_id(0),
-    m_fieldlist_str(NULL),
-    m_fieldlist_id(0),
-    m_field_count(0),
-    m_printf_str(NULL),
-    m_aggregate(false),
-    m_format_str(NULL)
-{
-  switch (type) {
-  case SQUID_LOG:
-    setup("squid", (char *) squid_format);
-    break;
-  case COMMON_LOG:
-    setup("common", (char *) common_format);
-    break;
-  case EXTENDED_LOG:
-    setup("extended", (char *) extended_format);
-    break;
-  case EXTENDED2_LOG:
-    setup("extended2", (char *) extended2_format);
-    break;
-    // For text logs, there is no format string; we'll simply log the
-    // entire entry as a string without any field substitutions.  To
-    // indicate this, the format_str will be NULL
-    //
-  case TEXT_LOG:
-    m_name_str = ats_strdup("text");
-    m_valid = true;
-    break;
-  default:
-    Note("Invalid log format type %d", type);
-    m_valid = false;
-  }
-  m_format_type = type;
-}
-
 /*-------------------------------------------------------------------------
   LogFormat::LogFormat
 
@@ -251,7 +195,10 @@ LogFormat::LogFormat(const char *name, const char *format_str, unsigned interval
     m_format_str(NULL)
 {
   setup(name, format_str, interval_sec);
-  m_format_type = CUSTOM_LOG;
+
+  // A TEXT_LOG is a log without a format string, everything else is a CUSTOM_LOG. It's possible that we could get
+  // rid of log types altogether, but LogFile currently tests whether a format is a TEXT_LOG format ...
+  m_format_type = format_str ? CUSTOM_LOG : TEXT_LOG;
 }
 
 //-----------------------------------------------------------------------------
@@ -782,17 +729,6 @@ LogFormatList::find_by_name(const char *name) const
   return NULL;
 }
 
-LogFormat *
-LogFormatList::find_by_type(LogFormatType type, int32_t id) const
-{
-  for (LogFormat * f = first(); f; f = next(f)) {
-    if ((f->type() == type) && (f->name_id() == id)) {
-      return f;
-    }
-  }
-  return NULL;
-}
-
 unsigned
 LogFormatList::count()
 {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogFormat.h
----------------------------------------------------------------------
diff --git a/proxy/logging/LogFormat.h b/proxy/logging/LogFormat.h
index ee5f06b..b632b42 100644
--- a/proxy/logging/LogFormat.h
+++ b/proxy/logging/LogFormat.h
@@ -21,8 +21,6 @@
   limitations under the License.
  */
 
-
-
 #ifndef LOG_FORMAT_H
 #define LOG_FORMAT_H
 
@@ -30,9 +28,24 @@
 
 #include "libts.h"
 #include "LogField.h"
-#include "LogFormatType.h"
 #include "InkXml.h"
 
+enum LogFormatType
+{
+  // We start the numbering at 4 to compatibility with Traffic Server 4.x, which used
+  // to have the predefined log formats enumerated above ...
+  CUSTOM_LOG = 4,
+  TEXT_LOG = 5
+};
+
+enum LogFileFormat
+{
+  BINARY_LOG,
+  ASCII_LOG,
+  ASCII_PIPE,
+  N_LOGFILE_TYPES
+};
+
 /*-------------------------------------------------------------------------
   LogFormat
 
@@ -43,7 +56,6 @@
 class LogFormat
 {
 public:
-  LogFormat(LogFormatType type);
   LogFormat(const char *name, const char *format_str, unsigned interval_sec = 0);
   LogFormat(const char *name, const char *fieldlist_str, const char *printf_str, unsigned interval_sec = 0);
   LogFormat(const LogFormat & rhs);
@@ -77,7 +89,7 @@ public:
   static void turn_tagging_off() { m_tagging_on = false; }
 
 private:
-  void setup(const char *name, const char *format_str, unsigned interval_sec = 0);
+  bool setup(const char *name, const char *format_str, unsigned interval_sec = 0);
   void init_variables(const char *name, const char *fieldlist_str, const char *printf_str, unsigned interval_sec);
 
 public:
@@ -86,11 +98,6 @@ public:
   long m_interval_next;
   char *m_agg_marshal_space;
 
-  static const char *const squid_format;        // pre defined formats
-  static const char *const common_format;
-  static const char *const extended_format;
-  static const char *const extended2_format;
-
 private:
   static bool m_tagging_on;     // flag to control tagging, class
   // variable
@@ -114,6 +121,14 @@ private:
   LogFormat & operator=(LogFormat & rhs);
 };
 
+// For text logs, there is no format string; we'll simply log the
+// entire entry as a string without any field substitutions.  To
+// indicate this, the format_str will be NULL.
+static inline LogFormat *
+MakeTextLogFormat(const char * name = "text") {
+  return NEW(new LogFormat(name, NULL /* format_str */));
+}
+
 /*-------------------------------------------------------------------------
   LogFormatList
   -------------------------------------------------------------------------*/
@@ -126,7 +141,6 @@ public:
 
   void add(LogFormat * format, bool copy = true);
   LogFormat *find_by_name(const char *name) const;
-  LogFormat *find_by_type(LogFormatType type, int32_t id) const;
 
   LogFormat *first() const { return m_format_list.head; }
   LogFormat *next(LogFormat * here) const { return (here->link).next; }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogFormatType.h
----------------------------------------------------------------------
diff --git a/proxy/logging/LogFormatType.h b/proxy/logging/LogFormatType.h
deleted file mode 100644
index 3c68449..0000000
--- a/proxy/logging/LogFormatType.h
+++ /dev/null
@@ -1,48 +0,0 @@
-/** @file
-
-  A brief file description
-
-  @section license License
-
-  Licensed to the Apache Software Foundation (ASF) under one
-  or more contributor license agreements.  See the NOTICE file
-  distributed with this work for additional information
-  regarding copyright ownership.  The ASF licenses this file
-  to you under the Apache License, Version 2.0 (the
-  "License"); you may not use this file except in compliance
-  with the License.  You may obtain a copy of the License at
-
-      http://www.apache.org/licenses/LICENSE-2.0
-
-  Unless required by applicable law or agreed to in writing, software
-  distributed under the License is distributed on an "AS IS" BASIS,
-  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-  See the License for the specific language governing permissions and
-  limitations under the License.
- */
-
-
-
-#ifndef LOG_FORMAT_TYPE_H
-#define LOG_FORMAT_TYPE_H
-
-enum LogFormatType
-{
-  SQUID_LOG,
-  COMMON_LOG,
-  EXTENDED_LOG,
-  EXTENDED2_LOG,
-  CUSTOM_LOG,
-  TEXT_LOG,
-  N_LOG_TYPES
-};
-
-enum LogFileFormat
-{
-  BINARY_LOG,
-  ASCII_LOG,
-  ASCII_PIPE,
-  N_LOGFILE_TYPES
-};
-
-#endif

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogObject.cc
----------------------------------------------------------------------
diff --git a/proxy/logging/LogObject.cc b/proxy/logging/LogObject.cc
index 24e4d57..d2b09b1 100644
--- a/proxy/logging/LogObject.cc
+++ b/proxy/logging/LogObject.cc
@@ -831,7 +831,7 @@ TextLogObject::TextLogObject(const char *name, const char *log_dir,
                              int rolling_enabled, int flush_threads,
                              int rolling_interval_sec, int rolling_offset_hr,
                              int rolling_size_mb)
-  : LogObject(NEW(new LogFormat(TEXT_LOG)), log_dir, name, ASCII_LOG, header,
+  : LogObject(MakeTextLogFormat(), log_dir, name, ASCII_LOG, header,
               rolling_enabled, flush_threads, rolling_interval_sec,
               rolling_offset_hr, rolling_size_mb)
 {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogPredefined.cc
----------------------------------------------------------------------
diff --git a/proxy/logging/LogPredefined.cc b/proxy/logging/LogPredefined.cc
index 3e8569b..79e6d52 100644
--- a/proxy/logging/LogPredefined.cc
+++ b/proxy/logging/LogPredefined.cc
@@ -27,16 +27,18 @@
 #include "LogFormat.h"
 
 // predefined formats
-static const char squid_format[] =
+const char * const PreDefinedFormatInfo::squid =
   "%<cqtq> %<ttms> %<chi> %<crc>/%<pssc> %<psql> %<cqhm> %<cquc> %<caun> %<phr>/%<pqsn> %<psct> %<xid>";
 
-static const char common_format[] = "%<chi> - %<caun> [%<cqtn>] \"%<cqtx>\" %<pssc> %<pscl>";
+const char * const PreDefinedFormatInfo::common =
+  "%<chi> - %<caun> [%<cqtn>] \"%<cqtx>\" %<pssc> %<pscl>";
 
-static const char extended_format[] =
+const char * const PreDefinedFormatInfo::extended =
   "%<chi> - %<caun> [%<cqtn>] \"%<cqtx>\" %<pssc> %<pscl> "
   "%<sssc> %<sscl> %<cqbl> %<pqbl> %<cqhl> %<pshl> %<pqhl> %<sshl> %<tts>";
 
-static const char extended2_format[] = "%<chi> - %<caun> [%<cqtn>] \"%<cqtx>\" %<pssc> %<pscl> "
+const char * const PreDefinedFormatInfo::extended2 =
+  "%<chi> - %<caun> [%<cqtn>] \"%<cqtx>\" %<pssc> %<pscl> "
   "%<sssc> %<sscl> %<cqbl> %<pqbl> %<cqhl> %<pshl> %<pqhl> %<sshl> %<tts> %<phr> %<cfsc> %<pfsc> %<crc>";
 
 PreDefinedFormatList::PreDefinedFormatList()
@@ -46,8 +48,8 @@ PreDefinedFormatList::PreDefinedFormatList()
 PreDefinedFormatList::~PreDefinedFormatList()
 {
   PreDefinedFormatInfo * info;
-  while (!this->list.empty()) {
-    info = this->list.pop();
+  while (!this->formats.empty()) {
+    info = this->formats.pop();
     delete info;
   }
 }
@@ -57,36 +59,36 @@ PreDefinedFormatList::init(LogConfig * config)
 {
   LogFormat *fmt;
 
-  fmt = NEW(new LogFormat("squid", squid_format));
+  fmt = NEW(new LogFormat("squid", PreDefinedFormatInfo::squid));
   config->global_format_list.add(fmt, false);
   Debug("log", "squid format added to the global format list");
 
   if (config->squid_log_enabled) {
-    this->list.enqueue(NEW(new PreDefinedFormatInfo(fmt, config->squid_log_name, config->squid_log_is_ascii, config->squid_log_header)));
+    this->formats.enqueue(NEW(new PreDefinedFormatInfo(fmt, config->squid_log_name, config->squid_log_is_ascii, config->squid_log_header)));
   }
 
-  fmt = NEW(new LogFormat("common", common_format));
+  fmt = NEW(new LogFormat("common", PreDefinedFormatInfo::common));
   config->global_format_list.add(fmt, false);
   Debug("log", "common format added to the global format list");
 
   if (config->common_log_enabled) {
-    this->list.enqueue(NEW(new PreDefinedFormatInfo(fmt, config->common_log_name, config->common_log_is_ascii, config->common_log_header)));
+    this->formats.enqueue(NEW(new PreDefinedFormatInfo(fmt, config->common_log_name, config->common_log_is_ascii, config->common_log_header)));
   }
 
-  fmt = NEW(new LogFormat("extended", extended_format));
+  fmt = NEW(new LogFormat("extended", PreDefinedFormatInfo::extended));
   config->global_format_list.add(fmt, false);
   Debug("log", "extended format added to the global format list");
 
   if (config->extended_log_enabled) {
-    this->list.enqueue(NEW(new PreDefinedFormatInfo(fmt, config->extended_log_name, config->extended_log_is_ascii, config->extended_log_header)));
+    this->formats.enqueue(NEW(new PreDefinedFormatInfo(fmt, config->extended_log_name, config->extended_log_is_ascii, config->extended_log_header)));
   }
 
-  fmt = NEW(new LogFormat("extended2", extended2_format));
+  fmt = NEW(new LogFormat("extended2", PreDefinedFormatInfo::extended2));
   config->global_format_list.add(fmt, false);
   Debug("log", "extended2 format added to the global format list");
 
   if (config->extended2_log_enabled) {
-    this->list.enqueue(NEW(new PreDefinedFormatInfo(fmt, config->extended2_log_name, config->extended2_log_is_ascii, config->extended2_log_header)));
+    this->formats.enqueue(NEW(new PreDefinedFormatInfo(fmt, config->extended2_log_name, config->extended2_log_is_ascii, config->extended2_log_header)));
   }
 
 }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogPredefined.h
----------------------------------------------------------------------
diff --git a/proxy/logging/LogPredefined.h b/proxy/logging/LogPredefined.h
index 4a8f52e..59c1fa3 100644
--- a/proxy/logging/LogPredefined.h
+++ b/proxy/logging/LogPredefined.h
@@ -41,8 +41,15 @@ struct PreDefinedFormatInfo
   PreDefinedFormatInfo(LogFormat * fmt, const char * fname, bool ascii, const char * hdr)
     :format(fmt), filename(fname), is_ascii(ascii), header(hdr)
   { }
+
+  static const char * const squid;
+  static const char * const common;
+  static const char * const extended;
+  static const char * const extended2;
 };
 
+typedef Queue<PreDefinedFormatInfo> PreDefinedFormatInfoList;
+
 struct PreDefinedFormatList
 {
   PreDefinedFormatList();
@@ -52,7 +59,7 @@ struct PreDefinedFormatList
   // adding the predefined LogFormats to the LogConfig global_format_list.
   void init(LogConfig * config);
 
-  Queue<PreDefinedFormatInfo> list;
+  PreDefinedFormatInfoList formats;
 };
 
 #endif /* LOG_PREDEFINED_H */

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogUtils.cc
----------------------------------------------------------------------
diff --git a/proxy/logging/LogUtils.cc b/proxy/logging/LogUtils.cc
index c92fac9..8d42258 100644
--- a/proxy/logging/LogUtils.cc
+++ b/proxy/logging/LogUtils.cc
@@ -49,8 +49,6 @@
 
 #include "LogUtils.h"
 #include "LogLimits.h"
-#include "LogFormatType.h"
-
 
 
 /*-------------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/Makefile.am
----------------------------------------------------------------------
diff --git a/proxy/logging/Makefile.am b/proxy/logging/Makefile.am
index b56d38e..036cb3e 100644
--- a/proxy/logging/Makefile.am
+++ b/proxy/logging/Makefile.am
@@ -59,7 +59,6 @@ liblogging_a_SOURCES = \
   LogFilter.h \
   LogFormat.cc \
   LogFormat.h \
-  LogFormatType.h \
   LogHost.cc \
   LogHost.h \
   LogLimits.h \


Re: [15/17] git commit: TS-2302: separate predefined formats from LogFormat

Posted by Igor Galić <i....@brainsware.org>.
> >> ----------------------------------------------------------------------
> >> diff --git a/proxy/logging/LogConfig.h b/proxy/logging/LogConfig.h
> >> index 2dc5bc6..00e5dd5 100644
> >> --- a/proxy/logging/LogConfig.h
> >> +++ b/proxy/logging/LogConfig.h
> >> @@ -75,8 +75,8 @@ enum
> >> extern RecRawStatBlock *log_rsb;
> >> 
> >> struct dirent;
> > 
> > This declaration is really odd here.
> 
> Yup.

perhaps we should clean this up.
I reckon that in most *Config.(cc?|h) files we're duplicating
reading configuration files, so perhaps we can consolidate that.

> J

++ i
Igor Galić

Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 6880 4155 74BD FD7C B515  2EA5 4B1D 9E08 A097 C9AE


Re: [15/17] git commit: TS-2302: separate predefined formats from LogFormat

Posted by James Peach <jp...@apache.org>.
On Oct 31, 2013, at 3:40 PM, Igor Galić <i....@brainsware.org> wrote:

> 
> 
> ----- Original Message -----
>> TS-2302: separate predefined formats from LogFormat
>> 
>> LogFormat now only knows about text and custom logs. LogConfig still
>> have some knowledge of custom formats, but it is largeley separated
>> and doesn't have a LogFormat dependency any more. Remove various
>> predefined log constants and apply addition const to pass format
>> strings around.
> 
> How will this help us convert the configuration to Lua asap?
> (also, purge XML configs..)

I dunno. I guess that reducing code coupling help this, but that's not an explicit goal of this change.

>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/b95d1e89
>> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/b95d1e89
>> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/b95d1e89
>> 
>> Branch: refs/heads/master
>> Commit: b95d1e89a83377f9a0e8b845e19e510153124deb
> [snip]
>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogConfig.h
>> ----------------------------------------------------------------------
>> diff --git a/proxy/logging/LogConfig.h b/proxy/logging/LogConfig.h
>> index 2dc5bc6..00e5dd5 100644
>> --- a/proxy/logging/LogConfig.h
>> +++ b/proxy/logging/LogConfig.h
>> @@ -75,8 +75,8 @@ enum
>> extern RecRawStatBlock *log_rsb;
>> 
>> struct dirent;
> 
> This declaration is really odd here.

Yup.

J

Re: [15/17] git commit: TS-2302: separate predefined formats from LogFormat

Posted by Igor Galić <i....@brainsware.org>.

----- Original Message -----
> TS-2302: separate predefined formats from LogFormat
> 
> LogFormat now only knows about text and custom logs. LogConfig still
> have some knowledge of custom formats, but it is largeley separated
> and doesn't have a LogFormat dependency any more. Remove various
> predefined log constants and apply addition const to pass format
> strings around.

How will this help us convert the configuration to Lua asap?
(also, purge XML configs..)

> 
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/b95d1e89
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/b95d1e89
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/b95d1e89
> 
> Branch: refs/heads/master
> Commit: b95d1e89a83377f9a0e8b845e19e510153124deb
[snip]
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b95d1e89/proxy/logging/LogConfig.h
> ----------------------------------------------------------------------
> diff --git a/proxy/logging/LogConfig.h b/proxy/logging/LogConfig.h
> index 2dc5bc6..00e5dd5 100644
> --- a/proxy/logging/LogConfig.h
> +++ b/proxy/logging/LogConfig.h
> @@ -75,8 +75,8 @@ enum
>  extern RecRawStatBlock *log_rsb;
>  
>  struct dirent;

This declaration is really odd here.

-- 
Igor Galić

Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 6880 4155 74BD FD7C B515  2EA5 4B1D 9E08 A097 C9AE