Willy,

attached is a first attempt at a patch that adds logging (without any rate
limiting). I have a few questions regarding the whole counters and logging
infrastructure:

1. I noticed that there is ha_warning(...) and send_log(p, LOG_WARNING, ...),
   usually both are used in the same place. What's the difference between
   them? Are the log lines in my patch going into the correct place?

2. How do you prefer the rate limiting of the log lines?
   - Is there an existing "flag" for rate limited logs?
   - Where should the information about the rate limited logging be stored
     (in a static variable?)?
   - Log only every Xth failed call to http_header_add_tail2?
   - By time? This would require an additional call to gettimeofday, no?
   - Should the code have separate rate limiting per header (I guess no,
     because the only meaningful thing a administrator can do is increase
     tune.maxrewrite and that fixes all headers at once)?

3. Regarding the counters I noticed frontends don't really seem to use
   error counters yet. Do I need to add a counter to both
   struct fe_counters as well as struct be_counters and increment depending
   on whether the (add|set)-header line is in a backend or frontend?

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.
-- >8 --

This patch adds a warning if an http-(request|reponse) (add|set)-header
rewrite fails to change the respective header in a request or response.

This usually happens when tune.maxrewrite is not sufficient to hold all
the headers that should be added.
---
 src/proto_http.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/proto_http.c b/src/proto_http.c
index 3adb54f2..9739fc7a 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -2631,7 +2631,11 @@ resume_execution:
                                }
                        }
 
-                       http_header_add_tail2(&txn->req, &txn->hdr_idx, 
replace->str, replace->len);
+                       if (http_header_add_tail2(&txn->req, &txn->hdr_idx, 
replace->str, replace->len) < 0) {
+                               replace->str[rule->arg.hdr_add.name_len] = 0;
+                               ha_warning("Proxy %s failed to add or set the 
request header '%s'. You might need to increase tune.maxrewrite.\n", px->id, 
replace->str);
+                               send_log(px, LOG_WARNING, "Proxy %s failed to 
add or set the request header '%s'. You might need to increase 
tune.maxrewrite.", px->id, replace->str);
+                       }
 
                        free_trash_chunk(replace);
                        break;
@@ -2931,7 +2935,12 @@ resume_execution:
                                        http_remove_header2(&txn->rsp, 
&txn->hdr_idx, &ctx);
                                }
                        }
-                       http_header_add_tail2(&txn->rsp, &txn->hdr_idx, 
replace->str, replace->len);
+
+                       if (http_header_add_tail2(&txn->rsp, &txn->hdr_idx, 
replace->str, replace->len) < 0) {
+                               replace->str[rule->arg.hdr_add.name_len] = 0;
+                               ha_warning("Proxy %s failed to add or set the 
response header '%s'. You might need to increase tune.maxrewrite.\n", px->id, 
replace->str);
+                               send_log(px, LOG_WARNING, "Proxy %s failed to 
add or set the response header '%s'. You might need to increase 
tune.maxrewrite.", px->id, replace->str);
+                       }
 
                        free_trash_chunk(replace);
                        break;
-- 
2.17.0


Reply via email to