From d555755423c179d23ef02ab2e0b856fd2b04fa71 Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Tue, 16 Jul 2024 10:42:21 +0100 Subject: [PATCH] proxy_logging config: unit tests and doc pointers Add unit tests to verify the precedence of access_log_ and log_ prefixes to options. Add pointers from proxy_logging sections in other sample config files to the proxy-server.conf-sample file. Change-Id: Id18176d3790fd187e304f0e33e3f74a94dc5305c --- etc/container-reconciler.conf-sample | 1 + etc/internal-client.conf-sample | 1 + etc/object-expirer.conf-sample | 39 +------- etc/proxy-server.conf-sample | 11 +-- .../common/middleware/test_proxy_logging.py | 95 ++++++++++++++++++- 5 files changed, 102 insertions(+), 45 deletions(-) diff --git a/etc/container-reconciler.conf-sample b/etc/container-reconciler.conf-sample index 422a567200..6011a3c21e 100644 --- a/etc/container-reconciler.conf-sample +++ b/etc/container-reconciler.conf-sample @@ -89,6 +89,7 @@ use = egg:swift#memcache [filter:proxy-logging] use = egg:swift#proxy_logging +# See proxy-server.conf-sample for options [filter:catch_errors] use = egg:swift#catch_errors diff --git a/etc/internal-client.conf-sample b/etc/internal-client.conf-sample index d9ed5e24b2..cbeb401c03 100644 --- a/etc/internal-client.conf-sample +++ b/etc/internal-client.conf-sample @@ -44,6 +44,7 @@ use = egg:swift#memcache [filter:proxy-logging] use = egg:swift#proxy_logging +# See proxy-server.conf-sample for options [filter:catch_errors] use = egg:swift#catch_errors diff --git a/etc/object-expirer.conf-sample b/etc/object-expirer.conf-sample index 2b5740035f..1e8762d6bc 100644 --- a/etc/object-expirer.conf-sample +++ b/etc/object-expirer.conf-sample @@ -129,41 +129,4 @@ use = egg:swift#catch_errors [filter:proxy-logging] use = egg:swift#proxy_logging -# If not set, logging directives from [DEFAULT] without "access_" will be used -# access_log_name = swift -# access_log_facility = LOG_LOCAL0 -# access_log_level = INFO -# access_log_address = /dev/log -# -# If set, access_log_udp_host will override access_log_address -# access_log_udp_host = -# access_log_udp_port = 514 -# -# You can use log_statsd_* from [DEFAULT] or override them here: -# access_log_statsd_host = -# access_log_statsd_port = 8125 -# access_log_statsd_default_sample_rate = 1.0 -# access_log_statsd_sample_rate_factor = 1.0 -# access_log_statsd_metric_prefix = -# access_log_headers = false -# -# If access_log_headers is True and access_log_headers_only is set only -# these headers are logged. Multiple headers can be defined as comma separated -# list like this: access_log_headers_only = Host, X-Object-Meta-Mtime -# access_log_headers_only = -# -# By default, the X-Auth-Token is logged. To obscure the value, -# set reveal_sensitive_prefix to the number of characters to log. -# For example, if set to 12, only the first 12 characters of the -# token appear in the log. An unauthorized access of the log file -# won't allow unauthorized usage of the token. However, the first -# 12 or so characters is unique enough that you can trace/debug -# token usage. Set to 0 to suppress the token completely (replaced -# by '...' in the log). -# Note: reveal_sensitive_prefix will not affect the value -# logged with access_log_headers=True. -# reveal_sensitive_prefix = 16 -# -# What HTTP methods are allowed for StatsD logging (comma-sep); request methods -# not in this list will have "BAD_METHOD" for the portion of the metric. -# log_statsd_valid_http_methods = GET,HEAD,POST,PUT,DELETE,COPY,OPTIONS +# See proxy-server.conf-sample for options diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index 9edb08d76f..0a3de342cb 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -980,6 +980,11 @@ use = egg:swift#etag_quoter use = egg:swift#list_endpoints # list_endpoints_path = /endpoints/ +# Note: The double proxy-logging in the pipeline is not a mistake. The +# left-most proxy-logging is there to log requests that were handled in +# middleware and never made it through to the right-most middleware (and +# proxy server). Double logging is prevented for normal requests. See +# proxy-logging docs. [filter:proxy-logging] use = egg:swift#proxy_logging # If not set, logging directives from [DEFAULT] without "access_" will be used @@ -1026,12 +1031,6 @@ use = egg:swift#proxy_logging # not in this list will have "BAD_METHOD" for the portion of the metric. # log_statsd_valid_http_methods = GET,HEAD,POST,PUT,DELETE,COPY,OPTIONS # -# Note: The double proxy-logging in the pipeline is not a mistake. The -# left-most proxy-logging is there to log requests that were handled in -# middleware and never made it through to the right-most middleware (and -# proxy server). Double logging is prevented for normal requests. See -# proxy-logging docs. -# # Hashing algorithm for log anonymization. Must be one of algorithms supported # by Python's hashlib. # log_anonymization_method = MD5 diff --git a/test/unit/common/middleware/test_proxy_logging.py b/test/unit/common/middleware/test_proxy_logging.py index c16b35134d..9bdf6f76cf 100644 --- a/test/unit/common/middleware/test_proxy_logging.py +++ b/test/unit/common/middleware/test_proxy_logging.py @@ -12,6 +12,7 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. +import logging import mock import time @@ -30,7 +31,7 @@ from swift.common.registry import register_sensitive_header, \ from swift.common.swob import Request, Response, HTTPServiceUnavailable from swift.common import constraints, registry from swift.common.storage_policy import StoragePolicy -from test.debug_logger import debug_logger +from test.debug_logger import debug_logger, FakeStatsdClient from test.unit import patch_policies from test.unit.common.middleware.helpers import FakeAppThatExcepts, FakeSwift @@ -169,6 +170,98 @@ class TestProxyLogging(unittest.TestCase): ('host', 8125)), app.access_logger.statsd_client.sendto_calls) + def test_init_statsd_options_log_prefix(self): + conf = { + 'log_headers': 'no', + 'log_statsd_valid_http_methods': 'GET', + 'log_facility': 'LOG_LOCAL7', + 'log_name': 'bob', + 'log_level': 'DEBUG', + 'log_udp_host': 'example.com', + 'log_udp_port': '3456', + 'log_statsd_host': 'example.com', + 'log_statsd_port': '1234', + 'log_statsd_default_sample_rate': 10, + 'log_statsd_sample_rate_factor': .04, + 'log_statsd_metric_prefix': 'foo', + } + with mock.patch('swift.common.statsd_client.StatsdClient', + FakeStatsdClient): + app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), conf) + + self.assertFalse(app.log_hdrs) + self.assertEqual(['GET'], app.valid_methods) + + log_adapter = app.access_logger + self.assertEqual('proxy-access', log_adapter.name) + self.assertEqual('bob', app.access_logger.server) + self.assertEqual(logging.DEBUG, log_adapter.logger.level) + self.assertEqual(('example.com', 3456), + log_adapter.logger.handlers[0].address) + self.assertEqual(SysLogHandler.LOG_LOCAL7, + log_adapter.logger.handlers[0].facility) + + statsd_client = app.access_logger.logger.statsd_client + self.assertIsInstance(statsd_client, FakeStatsdClient) + with mock.patch.object(statsd_client, 'random', return_value=0): + statsd_client.increment('baz') + self.assertEqual( + [(b'foo.proxy-server.baz:1|c|@0.4', ('example.com', 1234))], + statsd_client.sendto_calls) + + def test_init_statsd_options_access_log_prefix(self): + # verify that access_log_ prefix has precedence over log_ + conf = { + 'access_log_route': 'my-proxy-access', + 'access_log_headers': 'yes', + 'access_log_statsd_valid_http_methods': 'GET, HEAD', + 'access_log_facility': 'LOG_LOCAL6', + 'access_log_name': 'alice', + 'access_log_level': 'WARN', + 'access_log_udp_host': 'access.com', + 'access_log_udp_port': '6789', + 'log_headers': 'no', + 'log_statsd_valid_http_methods': 'GET', + 'log_facility': 'LOG_LOCAL7', + 'log_name': 'bob', + 'log_level': 'DEBUG', + 'log_udp_host': 'example.com', + 'log_udp_port': '3456', + 'access_log_statsd_host': 'access.com', + 'access_log_statsd_port': '5678', + 'access_log_statsd_default_sample_rate': 20, + 'access_log_statsd_sample_rate_factor': .03, + 'access_log_statsd_metric_prefix': 'access_foo', + 'log_statsd_host': 'example.com', + 'log_statsd_port': '1234', + 'log_statsd_default_sample_rate': 10, + 'log_statsd_sample_rate_factor': .04, + 'log_statsd_metric_prefix': 'foo', + } + with mock.patch('swift.common.statsd_client.StatsdClient', + FakeStatsdClient): + app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), conf) + + self.assertTrue(app.log_hdrs) + self.assertEqual(['GET', 'HEAD'], app.valid_methods) + + log_adapter = app.access_logger + self.assertEqual('my-proxy-access', log_adapter.name) + self.assertEqual('alice', app.access_logger.server) + self.assertEqual(logging.WARN, log_adapter.logger.level) + self.assertEqual(('access.com', 6789), + log_adapter.logger.handlers[0].address) + self.assertEqual(SysLogHandler.LOG_LOCAL6, + log_adapter.logger.handlers[0].facility) + + statsd_client = app.access_logger.logger.statsd_client + self.assertIsInstance(statsd_client, FakeStatsdClient) + with mock.patch.object(statsd_client, 'random', return_value=0): + statsd_client.increment('baz') + self.assertEqual( + [(b'access_foo.proxy-server.baz:1|c|@0.6', ('access.com', 5678))], + statsd_client.sendto_calls) + def test_logger_statsd_prefix(self): app = proxy_logging.ProxyLoggingMiddleware( FakeApp(), {'log_statsd_host': 'example.com'})