Deprecate log_icap and log_access configuration directives

The log_icap and log_access are not really needed to control requests logging.
Someone can use acls  with access_log and icap_log configuration directives
for this purpose.
Also currently the requests denied for logging using the log_access access list
will not be accounted for in performance counters.

This patch:
 - removes log_icap and log_access options from configuration file.
 - adds the "stats_collection" access list to control performane counters
   accounting.

This is a Measurement Factory project

diff --git a/src/SquidConfig.h b/src/SquidConfig.h
index 2d2a24b25799d821d690601e2c6392205df2d75f..3e71d0815eec9902f061f8e6e8419a23bc2bce50 100644 (file)
--- a/src/SquidConfig.h
+++ b/src/SquidConfig.h
@@ -373,7 +373,7 @@ public:
         acl_access *AlwaysDirect;
         acl_access *ASlists;
         acl_access *noCache;
-        acl_access *log;
+        acl_access *stats_collection;
 #if SQUID_SNMP
 
         acl_access *snmp;
@@ -398,10 +398,6 @@ public:
         acl_access *followXFF;
 #endif /* FOLLOW_X_FORWARDED_FOR */
 
-#if ICAP_CLIENT
-        acl_access* icap;
-#endif
-
         /// spoof_client_ip squid.conf acl.
         /// nil unless configured
         acl_access* spoof_client_ip;

diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc
index 41fc63404a698f385381c3db1a61cfb332a19088..f353e7b1b18a2a0f97753f15baf93b1cd8e07816 100644 (file)
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@ -3,7 +3,6 @@
  */
 
 #include "squid.h"
-#include "acl/FilledChecklist.h"
 #include "adaptation/icap/Config.h"
 #include "adaptation/icap/Launcher.h"
 #include "adaptation/icap/Xaction.h"
@@ -16,7 +15,6 @@
 #include "err_detail_type.h"
 #include "fde.h"
 #include "FwdState.h"
-#include "globals.h"
 #include "HttpMsg.h"
 #include "HttpReply.h"
 #include "HttpRequest.h"
@@ -550,16 +548,8 @@ void Adaptation::Icap::Xaction::tellQueryAborted()
 void Adaptation::Icap::Xaction::maybeLog()
 {
     if (IcapLogfileStatus == LOG_ENABLE) {
-        ACLFilledChecklist *checklist = new ACLFilledChecklist(::Config.accessList.icap, al.request, dash_str);
-        if (al.reply) {
-            checklist->reply = al.reply;
-            HTTPMSGLOCK(checklist->reply);
-        }
-        if (!::Config.accessList.icap || checklist->fastCheck() == ACCESS_ALLOWED) {
-            finalizeLogInfo();
-            icapLogLog(alep, checklist);
-        }
-        delete checklist;
+        finalizeLogInfo();
+        icapLogLog(alep);
     }
 }
 

diff --git a/src/adaptation/icap/icap_log.cc b/src/adaptation/icap/icap_log.cc
index ea74ea11a2d8dbb815d82df200fcacc6c2253411..a3dfe922eddebf09ca80e0e126783c5257060472 100644 (file)
--- a/src/adaptation/icap/icap_log.cc
+++ b/src/adaptation/icap/icap_log.cc
@@ -1,6 +1,9 @@
 #include "squid.h"
 #include "icap_log.h"
 #include "AccessLogEntry.h"
+#include "acl/FilledChecklist.h"
+#include "HttpReply.h"
+#include "globals.h"
 #include "log/CustomLog.h"
 #include "log/File.h"
 #include "log/Formats.h"
@@ -46,8 +49,14 @@ icapLogRotate()
     }
 }
 
-void icapLogLog(AccessLogEntry::Pointer &al, ACLChecklist * checklist)
+void icapLogLog(AccessLogEntry::Pointer &al)
 {
-    if (IcapLogfileStatus == LOG_ENABLE)
-        accessLogLogTo(Config.Log.icaplogs, al, checklist);
+    if (IcapLogfileStatus == LOG_ENABLE) {
+        ACLFilledChecklist checklist(NULL, al->adapted_request, NULL);
+        if (al->reply) {
+            checklist.reply = al->reply;
+            HTTPMSGLOCK(checklist.reply);
+        }
+        accessLogLogTo(Config.Log.icaplogs, al, &checklist);
+    }
 }

diff --git a/src/adaptation/icap/icap_log.h b/src/adaptation/icap/icap_log.h
index f11de68a9eb7eb6e4276ecaede11283cea7f7b43..6fda5b7b049b596ad4d38d16564d89c9a8f8b3e1 100644 (file)
--- a/src/adaptation/icap/icap_log.h
+++ b/src/adaptation/icap/icap_log.h
@@ -11,7 +11,7 @@ class ACLChecklist;
 void icapLogClose();
 void icapLogOpen();
 void icapLogRotate();
-void icapLogLog(AccessLogEntryPointer &al, ACLChecklist * checklist);
+void icapLogLog(AccessLogEntryPointer &al);
 
 extern int IcapLogfileStatus;
 

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index b61aaa01f9712d2018ad7a694ca080b77fc79701..0ca84867cb9d5ca50db47d2b09ebc4f01ef4289e 100644 (file)
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -1011,6 +1011,12 @@ parse_obsolete(const char *name)
         debugs(3, DBG_CRITICAL, "WARNING: url_rewrite_concurrency upgrade overriding url_rewrite_children settings.");
         Config.redirectChildren.concurrency = cval;
     }
+
+    if (!strcmp(name, "log_access"))
+        self_destruct();
+
+    if (!strcmp(name, "log_icap"))
+        self_destruct();
 }
 
 /* Parse a time specification from the config file.  Store the

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 43a837e7f848cfce928be333096166646c693968..5fdbf615c5d5e9143d1027baa1acac18c3b0b7d6 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -4046,31 +4046,31 @@ DOC_START
 DOC_END
 
 NAME: log_access
+TYPE: obsolete
+DOC_START
+       Remove this line. Use acls with access_log directives to control access logging
+DOC_END
+
+NAME: log_icap
+TYPE: obsolete
+DOC_START
+       Remove this line. Use acls with icap_log directives to control icap logging
+DOC_END
+
+NAME: stats_collection
 TYPE: acl_access
-LOC: Config.accessList.log
+LOC: Config.accessList.stats_collection
 DEFAULT: none
 DEFAULT_DOC: Allow logging for all transactions.
 COMMENT: allow|deny acl acl...
 DOC_START
-       This options allows you to control which requests gets logged
-       to access.log (see access_log directive). Requests denied for
-       logging will also not be accounted for in performance counters.
+       This options allows you to control which requests gets accounted
+       in performance counters.
 
        This clause only supports fast acl types.
        See http://wiki.squid-cache.org/SquidFaq/SquidAcl for details.
 DOC_END
 
-NAME: log_icap
-TYPE: acl_access
-IFDEF: ICAP_CLIENT
-LOC: Config.accessList.icap
-DEFAULT: none
-DEFAULT_DOC: Allow logging for all ICAP transactions.
-DOC_START
-       This options allows you to control which requests get logged
-       to icap.log. See the icap_log directive for ICAP log details.
-DOC_END
-
 NAME: cache_store_log
 TYPE: string
 DEFAULT: none

diff --git a/src/client_side.cc b/src/client_side.cc
index 9f344e364df6d575068923403986d95df56454bb..a332617cebac463ddc61e7fc6d1e4dd14d932b21 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -698,27 +698,35 @@ ClientHttpRequest::logRequest()
         }
     }
 
-    ACLFilledChecklist *checklist = clientAclChecklistCreate(Config.accessList.log, this);
-
+    ACLFilledChecklist checklist(NULL, request, NULL);
     if (al->reply) {
-        checklist->reply = al->reply;
-        HTTPMSGLOCK(checklist->reply);
+        checklist.reply = al->reply;
+        HTTPMSGLOCK(checklist.reply);
+    }
+
+    if (request) {
+        al->adapted_request = request;
+        HTTPMSGLOCK(al->adapted_request);
     }
+    accessLogLog(al, &checklist);
 
-    if (!Config.accessList.log || checklist->fastCheck() == ACCESS_ALLOWED) {
-        if (request) {
-            al->adapted_request = request;
-            HTTPMSGLOCK(al->adapted_request);
+    bool updatePerformanceCounters = true;
+    if (Config.accessList.stats_collection) {
+        ACLFilledChecklist statsCheck(Config.accessList.stats_collection, request, NULL);
+        if (al->reply) {
+            statsCheck.reply = al->reply;
+            HTTPMSGLOCK(statsCheck.reply);
         }
-        accessLogLog(al, checklist);
+        updatePerformanceCounters = (statsCheck.fastCheck() == ACCESS_ALLOWED);
+    }
+
+    if (updatePerformanceCounters) {
         if (request)
             updateCounters();
 
         if (getConn() != NULL && getConn()->clientConnection != NULL)
             clientdbUpdate(getConn()->clientConnection->remote, logType, AnyP::PROTO_HTTP, out.size);
     }
-
-    delete checklist;
 }
 
 void