Logging policy #856
Logging policy #856
Conversation
|
|
||
| -- Returns the value for ngx.var.enable_acess_logs | ||
| local function enable_access_log_value(enabled) | ||
| return (enabled and '1') or '0' |
There was a problem hiding this comment.
Better to use dispatch table for this. Conditionals are not JITable.
There was a problem hiding this comment.
In this case, it doesn't matter much, because it's only called from new().
But I changed it because as a general rule we should do this, and I tend to forget about this 😅
gateway/conf/nginx.conf.liquid
Outdated
| @@ -120,7 +120,8 @@ http { | |||
|
|
|||
| server { | |||
|
|
|||
| access_log {{ access_log_file | default: "/dev/stdout" }} time; | |||
| set $enable_access_logs '1'; | |||
There was a problem hiding this comment.
I'd rather name it access_logs_enabled so we can later have access_logs_format etc.
davidor
force-pushed
the
logging-policy
branch
from
fc2781a to
2655c0e
Compare
davidor
force-pushed
the
logging-policy
branch
2 times, most recently
from
c3e0f49 to
3310f92
Compare
davidor
force-pushed
the
logging-policy
branch
from
3310f92 to
74b263d
Compare
| @@ -120,7 +120,8 @@ http { | |||
|
|
|||
| server { | |||
|
|
|||
| access_log {{ access_log_file | default: "/dev/stdout" }} time; | |||
| set $access_logs_enabled '1'; | |||
There was a problem hiding this comment.
Are you sure about the quotes?
There was a problem hiding this comment.
I think it's correct. From the docs:
The if parameter (1.7.0) enables conditional logging. A request will not be logged if the condition evaluates to “0” or an empty string.
There was a problem hiding this comment.
Yes. But what I meant is that set value should not be quoted. Now the value is '1', not 1.
It is a bit confusing (even though it will still work).
There was a problem hiding this comment.
Yes, I wanted it to be a string because the docs mention "0" as a string.
There was a problem hiding this comment.
So just to be clear, I inspected the value of the ngx.var and it's the string "1", not the string "'1'". Not sure if that's what you meant.
There was a problem hiding this comment.
Ok. There is some magic going on.
Both these are the same value (string):
set $access_log_enabled '1';
set $access_log_enabled_num 1;I thought we had a PR to strip '' from some of the values because it was generating quotes. My bad I guess.
Allows enabling/disabling access logs per service, so addresses #712 partially.