From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Fri, 1 Apr 2011 14:39:24 +0000 (-0600)
Subject: Log all transactions including those with uncertain status or no sent response.
X-Git-Tag: take06~27^2~42
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6fd1086a5ef5d0dad88ed7ddf6e886ae2dca6104;p=thirdparty%2Fsquid.git

Log all transactions including those with uncertain status or no sent response.

Excluding those transactions from access log hides valuable information.

For example, it hides certain aborted client transactions which sometimes
indicate Squid bugs or configuration problems that the administrator can fix.
It also leads to wrong statistics reporting if the reporting tools are based
on access logs information.

The change just removes the logging guard and adds debugging. The logging
code itself is unchanged except for indenting.
---

diff --git a/src/client_side.cc b/src/client_side.cc
index 94d61be179..52cf389d8c 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -615,74 +615,75 @@ prepareLogWithRequestDetails(HttpRequest * request, AccessLogEntry * aLogEntry)
 void
 ClientHttpRequest::logRequest()
 {
-    if (out.size || logType) {
-        al.icp.opcode = ICP_INVALID;
-        al.url = log_uri;
-        debugs(33, 9, "clientLogRequest: al.url='" << al.url << "'");
-
-        if (al.reply) {
-            al.http.code = al.reply->sline.status;
-            al.http.content_type = al.reply->content_type.termedBuf();
-        } else if (loggingEntry() && loggingEntry()->mem_obj) {
-            al.http.code = loggingEntry()->mem_obj->getReply()->sline.status;
-            al.http.content_type = loggingEntry()->mem_obj->getReply()->content_type.termedBuf();
-        }
+    if (!out.size && !logType)
+        debugs(33, 5, HERE << "logging half-baked transaction: " << log_uri);
+
+    al.icp.opcode = ICP_INVALID;
+    al.url = log_uri;
+    debugs(33, 9, "clientLogRequest: al.url='" << al.url << "'");
+
+    if (al.reply) {
+        al.http.code = al.reply->sline.status;
+        al.http.content_type = al.reply->content_type.termedBuf();
+    } else if (loggingEntry() && loggingEntry()->mem_obj) {
+        al.http.code = loggingEntry()->mem_obj->getReply()->sline.status;
+        al.http.content_type = loggingEntry()->mem_obj->getReply()->content_type.termedBuf();
+    }
 
-        debugs(33, 9, "clientLogRequest: http.code='" << al.http.code << "'");
+    debugs(33, 9, "clientLogRequest: http.code='" << al.http.code << "'");
 
-        if (loggingEntry() && loggingEntry()->mem_obj)
-            al.cache.objectSize = loggingEntry()->contentLen();
+    if (loggingEntry() && loggingEntry()->mem_obj)
+        al.cache.objectSize = loggingEntry()->contentLen();
 
-        al.cache.caddr.SetNoAddr();
+    al.cache.caddr.SetNoAddr();
 
-        if (getConn() != NULL) al.cache.caddr = getConn()->log_addr;
+    if (getConn() != NULL) al.cache.caddr = getConn()->log_addr;
 
-        al.cache.requestSize = req_sz;
-        al.cache.requestHeadersSize = req_sz;
+    al.cache.requestSize = req_sz;
+    al.cache.requestHeadersSize = req_sz;
 
-        al.cache.replySize = out.size;
-        al.cache.replyHeadersSize = out.headers_sz;
+    al.cache.replySize = out.size;
+    al.cache.replyHeadersSize = out.headers_sz;
 
-        al.cache.highOffset = out.offset;
+    al.cache.highOffset = out.offset;
 
-        al.cache.code = logType;
+    al.cache.code = logType;
 
-        al.cache.msec = tvSubMsec(start_time, current_time);
+    al.cache.msec = tvSubMsec(start_time, current_time);
 
-        if (request)
-            prepareLogWithRequestDetails(request, &al);
+    if (request)
+        prepareLogWithRequestDetails(request, &al);
 
-        if (getConn() != NULL && getConn()->rfc931[0])
-            al.cache.rfc931 = getConn()->rfc931;
+    if (getConn() != NULL && getConn()->rfc931[0])
+        al.cache.rfc931 = getConn()->rfc931;
 
 #if USE_SSL && 0
 
-        /* This is broken. Fails if the connection has been closed. Needs
-         * to snarf the ssl details some place earlier..
-         */
-        if (getConn() != NULL)
-            al.cache.ssluser = sslGetUserEmail(fd_table[getConn()->fd].ssl);
+    /* This is broken. Fails if the connection has been closed. Needs
+     * to snarf the ssl details some place earlier..
+     */
+    if (getConn() != NULL)
+        al.cache.ssluser = sslGetUserEmail(fd_table[getConn()->fd].ssl);
 
 #endif
 
-        ACLFilledChecklist *checklist = clientAclChecklistCreate(Config.accessList.log, this);
+    ACLFilledChecklist *checklist = clientAclChecklistCreate(Config.accessList.log, this);
 
-        if (al.reply)
-            checklist->reply = HTTPMSGLOCK(al.reply);
+    if (al.reply)
+        checklist->reply = HTTPMSGLOCK(al.reply);
 
-        if (!Config.accessList.log || checklist->fastCheck()) {
-            if (request)
-                al.adapted_request = HTTPMSGLOCK(request);
-            accessLogLog(&al, checklist);
-            updateCounters();
-
-            if (getConn() != NULL)
-                clientdbUpdate(getConn()->peer, logType, AnyP::PROTO_HTTP, out.size);
-        }
+    if (!Config.accessList.log || checklist->fastCheck()) {
+        if (request)
+            al.adapted_request = HTTPMSGLOCK(request);
+        accessLogLog(&al, checklist);
+        updateCounters();
 
-        delete checklist;
+        if (getConn() != NULL)
+            clientdbUpdate(getConn()->peer, logType, AnyP::PROTO_HTTP, out.size);
     }
 
+    delete checklist;
+
     accessLogFreeMemory(&al);
 }