From: Christos Tsantilas Date: Tue, 18 Jun 2013 06:22:13 +0000 (+0300) Subject: Deprecate log_icap and log_access configuration directives X-Git-Tag: SQUID_3_4_0_1~49 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8ebad7803f7bb84df970e1d12990490ccd9f3661;p=thirdparty%2Fsquid.git 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 2d2a24b257..3e71d0815e 100644 --- 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 41fc63404a..f353e7b1b1 100644 --- 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 ea74ea11a2..a3dfe922ed 100644 --- 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 f11de68a9e..6fda5b7b04 100644 --- 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 b61aaa01f9..0ca84867cb 100644 --- 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 43a837e7f8..5fdbf615c5 100644 --- 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 9f344e364d..a332617ceb 100644 --- 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