Opened 6 years ago

Closed 6 years ago

#596 closed defect (fixed)

Условие при логировании в syslog

Reported by: Oleksandr Typlyns'kyi Owned by:
Priority: minor Milestone:
Component: nginx-module Version: 1.7.x
Keywords: ngx_http_log_module Cc:
uname -a:
nginx -V: nginx version: nginx/1.7.3
configure arguments: --prefix=/usr/local/etc/nginx --with-cc-opt='-I /usr/local/include' --with-ld-opt='-L /usr/local/lib' --conf-path=/usr/local/etc/nginx/nginx.conf --sbin-path=/usr/local/sbin/nginx --pid-path=/var/run/nginx.pid --error-log-path=/var/log/nginx-error.log --user=www --group=www --with-file-aio --http-client-body-temp-path=/var/tmp/nginx/client_body_temp --http-fastcgi-temp-path=/var/tmp/nginx/fastcgi_temp --http-proxy-temp-path=/var/tmp/nginx/proxy_temp --http-scgi-temp-path=/var/tmp/nginx/scgi_temp --http-uwsgi-temp-path=/var/tmp/nginx/uwsgi_temp --http-log-path=/var/log/nginx-access.log --with-http_dav_module --add-module=/usr/ports/www/nginx-devel/work/arut-nginx-dav-ext-module-0e07a3e --with-http_flv_module --with-http_mp4_module --with-http_random_index_module --with-http_realip_module --with-http_secure_link_module --with-http_stub_status_module --with-http_sub_module --with-pcre

Description

В документации access_log для syslog заявлена возможность использовать условие.
Указываю в конфиге:
access_log syslog:server=unix:/var/run/log,facility=local5 main if=$loggable;
Получаю ошибку:
nginx: [emerg] parameter "if=$loggable" is not supported by syslog

Насколько я понял по коду ngx_http_log_handler, конфликтов у filter с syslog быть не должно.
В качестве быстрого патча сделал исключение для "if=" и ещё один goto:

--- src/http/modules/ngx_http_log_module.c.orig 2014-07-08 16:22:39.000000000 +0300
+++ src/http/modules/ngx_http_log_module.c      2014-07-28 15:09:29.621933289 +0300
@@ -1256,6 +1256,23 @@ process_formats:

     if (log->syslog_peer != NULL) {
         if (cf->args->nelts > 3) {
+            if (cf->args->nelts == 4) {
+                if (ngx_strncmp(value[3].data, "if=", 3) == 0) {
+                    filter.len = value[3].len - 3;
+                    filter.data = value[3].data + 3;
+                    goto process_filter;
+               } else {
+                    ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+                                       "parameter \"%V\" is not supported by syslog",
+                                       &value[3]);
+                    return NGX_CONF_ERROR;
+                }
+            } else {
+                ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+                                   "parameter \"%V\" is not supported by syslog",
+                                   &value[4]);
+                return NGX_CONF_ERROR;
+            }
             ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
                                "parameter \"%V\" is not supported by syslog",
                                &value[3]);
@@ -1410,6 +1427,8 @@ process_formats:
         log->file->data = buffer;
     }

+process_filter:
+
     if (filter.len) {
         log->filter = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t));
         if (log->filter == NULL) {

Change History (7)

comment:1 by Sergey Kandaurov, 6 years ago

Status: newaccepted

comment:2 by Sergey Kandaurov, 6 years ago

Мне больше нравится нижеследующий вариант как более простой.
Вы можете его проверить?

# HG changeset patch
# User Sergey Kandaurov <pluknet@nginx.com>
# Date 1406738641 -14400
#      Wed Jul 30 20:44:01 2014 +0400
# Node ID 643b86472b6f87522898a86dea06009dfef060c7
# Parent  4d092aa2f4637ce50284d2accd99a8e91aae2b4c
Access log: syslog support for certain parameters (ticket #596).

diff -r 4d092aa2f463 -r 643b86472b6f src/http/modules/ngx_http_log_module.c
--- a/src/http/modules/ngx_http_log_module.c	Mon Jul 28 12:27:57 2014 -0700
+++ b/src/http/modules/ngx_http_log_module.c	Wed Jul 30 20:44:01 2014 +0400
@@ -1254,17 +1254,6 @@ process_formats:
         return NGX_CONF_ERROR;
     }
 
-    if (log->syslog_peer != NULL) {
-        if (cf->args->nelts > 3) {
-            ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
-                               "parameter \"%V\" is not supported by syslog",
-                               &value[3]);
-            return NGX_CONF_ERROR;
-        }
-
-        return NGX_CONF_OK;
-    }
-
     size = 0;
     flush = 0;
     gzip = 0;
@@ -1272,7 +1261,9 @@ process_formats:
 
     for (i = 3; i < cf->args->nelts; i++) {
 
-        if (ngx_strncmp(value[i].data, "buffer=", 7) == 0) {
+        if (ngx_strncmp(value[i].data, "buffer=", 7) == 0
+            && log->syslog_peer == NULL)
+        {
             s.len = value[i].len - 7;
             s.data = value[i].data + 7;
 
@@ -1287,7 +1278,9 @@ process_formats:
             continue;
         }
 
-        if (ngx_strncmp(value[i].data, "flush=", 6) == 0) {
+        if (ngx_strncmp(value[i].data, "flush=", 6) == 0
+            && log->syslog_peer == NULL)
+        {
             s.len = value[i].len - 6;
             s.data = value[i].data + 6;
 
@@ -1303,7 +1296,8 @@ process_formats:
         }
 
         if (ngx_strncmp(value[i].data, "gzip", 4) == 0
-            && (value[i].len == 4 || value[i].data[4] == '='))
+            && (value[i].len == 4 || value[i].data[4] == '=')
+            && log->syslog_peer == NULL)
         {
 #if (NGX_ZLIB)
             if (size == 0) {

comment:3 by Oleksandr Typlyns'kyi, 6 years ago

Без goto и дублирования кода - однозначно лучше.
Можно ещё информативность сообщения об ошибке сохранить:

--- src/http/modules/ngx_http_log_module.c.orig 2014-07-08 16:22:39.000000000 +0300
+++ src/http/modules/ngx_http_log_module.c      2014-07-31 21:12:20.917622118 +0300
@@ -1254,17 +1254,6 @@ process_formats:
         return NGX_CONF_ERROR;
     }

-    if (log->syslog_peer != NULL) {
-        if (cf->args->nelts > 3) {
-            ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
-                               "parameter \"%V\" is not supported by syslog",
-                               &value[3]);
-            return NGX_CONF_ERROR;
-        }
-
-        return NGX_CONF_OK;
-    }
-
     size = 0;
     flush = 0;
     gzip = 0;
@@ -1272,7 +1261,9 @@ process_formats:

     for (i = 3; i < cf->args->nelts; i++) {

-        if (ngx_strncmp(value[i].data, "buffer=", 7) == 0) {
+        if (ngx_strncmp(value[i].data, "buffer=", 7) == 0
+            && log->syslog_peer == NULL)
+        {
             s.len = value[i].len - 7;
             s.data = value[i].data + 7;

@@ -1287,7 +1278,9 @@ process_formats:
             continue;
         }

-        if (ngx_strncmp(value[i].data, "flush=", 6) == 0) {
+        if (ngx_strncmp(value[i].data, "flush=", 6) == 0
+            && log->syslog_peer == NULL)
+        {
             s.len = value[i].len - 6;
             s.data = value[i].data + 6;

@@ -1303,7 +1296,8 @@ process_formats:
         }

         if (ngx_strncmp(value[i].data, "gzip", 4) == 0
-            && (value[i].len == 4 || value[i].data[4] == '='))
+            && (value[i].len == 4 || value[i].data[4] == '=')
+            && log->syslog_peer == NULL)
         {
 #if (NGX_ZLIB)
             if (size == 0) {
@@ -1341,6 +1335,13 @@ process_formats:
             continue;
         }

+        if (log->syslog_peer != NULL) {
+           ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+                              "parameter \"%V\" is not supported by syslog",
+                              &value[i]);
+            return NGX_CONF_ERROR;
+       }
+
         ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
                            "invalid parameter \"%V\"", &value[i]);
         return NGX_CONF_ERROR;

comment:4 by Maxim Dounin, 6 years ago

Господа, а не проще как-то так:

--- a/src/http/modules/ngx_http_log_module.c
+++ b/src/http/modules/ngx_http_log_module.c
@@ -1254,17 +1254,6 @@ process_formats:
         return NGX_CONF_ERROR;
     }
 
-    if (log->syslog_peer != NULL) {
-        if (cf->args->nelts > 3) {
-            ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
-                               "parameter \"%V\" is not supported by syslog",
-                               &value[3]);
-            return NGX_CONF_ERROR;
-        }
-
-        return NGX_CONF_OK;
-    }
-
     size = 0;
     flush = 0;
     gzip = 0;
@@ -1361,6 +1350,12 @@ process_formats:
             return NGX_CONF_ERROR;
         }
 
+        if (log->syslog_peer) {
+            ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+                               "logs to syslog cannot be buffered");
+            return NGX_CONF_ERROR;
+        }
+
         if (log->file->data) {
             buffer = log->file->data;
 

?

(Untested, но я не вижу причин почему не. Особенно с учётом того, что аналогичная проверка на использование переменных там уже стоит.)

comment:5 by Oleksandr Typlyns'kyi, 6 years ago

Да, и так работает.
Сообщение об ошибке при указании только gzip не совсем очевидно для читавших документацию плохо, но и указание его идёт в разрез с нею и логикой :)

comment:6 by Maxim Dounin <mdounin@…>, 6 years ago

In b1f8285297a710687fc4616ea1a26c2732b59ee7/nginx:

Access log: allowed logs to syslog with "if=" (ticket #596).

comment:7 by Maxim Dounin, 6 years ago

Resolution: fixed
Status: acceptedclosed

Fix committed.

Note: See TracTickets for help on using tickets.