Issue 3102 - Introduce log_format "data" for storing POST messages

modules/nginx/templates/nginx.conf.erb

 user <%= scope.lookupvar('nginx::params::user') %> <%= scope.lookupvar('nginx::params::group') %>;
 worker_processes <%= worker_processes %>;
 
 error_log /var/log/nginx/error.log notice;
 pid /var/run/nginx.pid;
 
 events {
   worker_connections <%= worker_connections %>;
   <% if scope.lookupvar('nginx::params::multi_accept') == 'on' %>multi_accept on;<% end %>
 }
 worker_rlimit_nofile <%= scope.lookupvar('nginx::params::worker_rlimit_nofile') %>;
 
 http {
   include /etc/nginx/mime.types;
   default_type application/octet-stream;
 
   log_format main '$remote_addr - $remote_user [$time_local] "$request" '
                   '$status $bytes_sent "$http_referer" '
                   '"$http_user_agent" "$http_x_forwarded_for" $scheme '
                   '"$http_accept_language" "$http_host" "$http_x_client_id"';
 
-  log_format data '[$time_local] "$request" $status "$http_user_agent" '

[Felix Dahlke, 2015/09/21 18:30:45]

Why not have the same fields as with "main" here, and just add one for the data?

[Felix Dahlke, 2015/09/21 18:30:45]

Do we have more than one use case for this? Will filter hit statistics use the
same approach? If not - I would lean towards defining this in the relevant
module, not globally (as we're doing it for the notification servers).

[mathias, 2015/09/21 18:38:53]

Yes the filter hit statistics are meant to use the same logic (see my e-mail
update from today). In fact there are even a few more upcoming projects that
could use this exact approach. And from what we know so far, it looks like all
of them would be served with this solution quite well. Note that the particular
choice of fields logged has been made up with that in mind.

[mathias, 2015/09/21 18:38:53]

Because the data is meant to be anonymous. There is no need to track the IP or
any of the other meta-data associated with the request, and the selected fields
have been chosen together with our data scientists.

[Felix Dahlke, 2015/09/21 20:15:51]

While it's not relevant for the Adblock Browser data backend, for something like
filter hit statistics we need the IP for debugging purposes IMHO, anyone seeing
this will have a very easy time fooling us with false data. Since we won't store
these access logs for longer than 30 days, I see no reason why we shouldn't just
have the same set of fields, or well, at least the IP.

[mathias, 2015/09/21 21:38:41]

In fact most of the fields currently matching those in the access log have been
introduced for that exact purpose (so we could at least determine a matching IP
based on the assumption that other requests in the same time are valid), but I
see your point. Especially the fact that this approach would be way easier to
handle in aggregation.

To be honest I do not like combining system log content with application log
stuff (like done with the notifications, for example), which is the sole reason
for the separation. But I can agree to just having two versions. I'll update the
patch-set accordingly.

+  log_format data '$remote_addr - $remote_user [$time_local] "$request" '
+                  '$status $bytes_sent "$http_referer" '
+                  '"$http_user_agent" "$http_x_forwarded_for" $scheme '
+                  '"$http_accept_language" "$http_host" "$http_x_client_id" '
                   '$http_content_length "$http_content_type" $request_body';
 
   access_log /var/log/nginx/access.log main;
   server_tokens off;
 
   sendfile <%= scope.lookupvar('nginx::params::sendfile')%>;
 
   <% if scope.lookupvar('nginx::params::tcp_nopush') == 'on' %>
   tcp_nopush on;
   <% end %>
@@ skipping 25 matching lines @@
   <% if @geoip_country %>
   geoip_country <%= @geoip_country %>;
   <% end %>
   <% if @geoip_city %>
   geoip_city <%= @geoip_city %>;
   <% end %>
 
   include /etc/nginx/conf.d/*.conf;
   include /etc/nginx/sites-enabled/*;
 }