From 4d109f622c31b8285fc3c9b97dc0ba13c7d8b650 Mon Sep 17 00:00:00 2001 From: Nathan Klapstein Date: Wed, 20 Mar 2019 14:57:16 -0600 Subject: [PATCH 01/11] making `facility` GELF field an additional field --- graypy/handler.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/graypy/handler.py b/graypy/handler.py index 1ca930dc4..7f29bb229 100644 --- a/graypy/handler.py +++ b/graypy/handler.py @@ -79,7 +79,7 @@ def __init__(self, chunk_size=WAN_CHUNK, debugging_fields=True, ``host`` GELF field. :type localname: str or None - :param facility: If specified, replace the ``facility`` GELF field + :param facility: If specified, replace the ``_facility`` GELF field with the specified value. Additionally, the LogRecord.name will used populate the ``_logger`` GELF field. :type facility: str @@ -141,7 +141,7 @@ def _make_gelf_dict(self, record): 'short_message': self.formatter.format(record) if self.formatter else record.getMessage(), 'timestamp': record.created, 'level': SYSLOG_LEVELS.get(record.levelno, record.levelno), - 'facility': self.facility or record.name, + '_facility': self.facility or record.name, } # add in specified optional extras @@ -188,7 +188,7 @@ def _set_custom_facility(gelf_dict, facility_value, record): field. :type record: logging.LogRecord """ - gelf_dict.update({"facility": facility_value, '_logger': record.name}) + gelf_dict.update({"_facility": facility_value, '_logger': record.name}) @staticmethod def _add_full_message(gelf_dict, record): From c9d9407f18aabe1754b5df17a4ae5bf23facf85f Mon Sep 17 00:00:00 2001 From: Nathan Klapstein Date: Wed, 20 Mar 2019 14:58:18 -0600 Subject: [PATCH 02/11] making `line` GELF field an additional field `_line` --- graypy/handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graypy/handler.py b/graypy/handler.py index 7f29bb229..14c39d8a4 100644 --- a/graypy/handler.py +++ b/graypy/handler.py @@ -248,7 +248,7 @@ def _add_debugging_fields(gelf_dict, record): """ gelf_dict.update({ 'file': record.pathname, - 'line': record.lineno, + '_line': record.lineno, '_function': record.funcName, '_pid': record.process, '_thread_name': record.threadName, From f72d33ff7582dea219411417d078f12fa15ad71c Mon Sep 17 00:00:00 2001 From: Nathan Klapstein Date: Wed, 20 Mar 2019 15:00:44 -0600 Subject: [PATCH 03/11] making `file` GELF field an additional field `_file` --- graypy/handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graypy/handler.py b/graypy/handler.py index 14c39d8a4..9bd43526c 100644 --- a/graypy/handler.py +++ b/graypy/handler.py @@ -247,7 +247,7 @@ def _add_debugging_fields(gelf_dict, record): :type record: logging.LogRecord """ gelf_dict.update({ - 'file': record.pathname, + '_file': record.pathname, '_line': record.lineno, '_function': record.funcName, '_pid': record.process, From 1372654d3f1f5897640062d69fc14c431d9b3426 Mon Sep 17 00:00:00 2001 From: Nathan Klapstein Date: Wed, 20 Mar 2019 15:01:42 -0600 Subject: [PATCH 04/11] updating docs on `_facility` GELF field --- graypy/handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graypy/handler.py b/graypy/handler.py index 9bd43526c..b5d342227 100644 --- a/graypy/handler.py +++ b/graypy/handler.py @@ -173,14 +173,14 @@ def _add_level_names(gelf_dict, record): @staticmethod def _set_custom_facility(gelf_dict, facility_value, record): - """Set the ``gelf_dict``'s ``facility`` field to the specified value + """Set the ``gelf_dict``'s ``_facility`` field to the specified value also add the the extra ``_logger`` field containing the LogRecord.name :param gelf_dict: dictionary representation of a GELF log. :type gelf_dict: dict :param facility_value: Value to set as the ``gelf_dict``'s - ``facility`` field. + ``_facility`` field. :type facility_value: str :param record: :class:`logging.LogRecord` to extract it's record From e664caf6fa023b7961768586eec7aaf5b923e720 Mon Sep 17 00:00:00 2001 From: Nathan Klapstein Date: Wed, 20 Mar 2019 15:02:49 -0600 Subject: [PATCH 05/11] fixing test assertion for `_facility` field --- tests/unit/test_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_handler.py b/tests/unit/test_handler.py index 95edfc903..cffa01646 100644 --- a/tests/unit/test_handler.py +++ b/tests/unit/test_handler.py @@ -183,7 +183,7 @@ def test_set_custom_facility(): facility = "test facility" BaseGELFHandler._set_custom_facility(gelf_dict, facility, MOCK_LOG_RECORD) assert MOCK_LOG_RECORD_NAME == gelf_dict["_logger"] - assert "test facility" == gelf_dict["facility"] + assert "test facility" == gelf_dict["_facility"] def test_formatted_logger(formatted_logger, mock_send): From cc8659d3f602df98d6d3219db537ac98ccb7e609 Mon Sep 17 00:00:00 2001 From: Nathan Klapstein Date: Wed, 20 Mar 2019 15:04:09 -0600 Subject: [PATCH 06/11] bumping to GELF version 1.1 --- graypy/handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graypy/handler.py b/graypy/handler.py index b5d342227..3db0320b6 100644 --- a/graypy/handler.py +++ b/graypy/handler.py @@ -136,7 +136,7 @@ def _make_gelf_dict(self, record): """ # construct the base GELF format gelf_dict = { - 'version': "1.0", + 'version': "1.1", 'host': BaseGELFHandler._resolve_host(self.fqdn, self.localname), 'short_message': self.formatter.format(record) if self.formatter else record.getMessage(), 'timestamp': record.created, From 5a36d267bad20c93ae599f86669a3038389081d1 Mon Sep 17 00:00:00 2001 From: Nathan Klapstein Date: Wed, 20 Mar 2019 15:28:03 -0600 Subject: [PATCH 07/11] making GELF field `level_name` a additional field `_level_name` --- graypy/handler.py | 11 +++++------ tests/unit/test_handler.py | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/graypy/handler.py b/graypy/handler.py index 3db0320b6..eca511936 100644 --- a/graypy/handler.py +++ b/graypy/handler.py @@ -84,8 +84,8 @@ def __init__(self, chunk_size=WAN_CHUNK, debugging_fields=True, used populate the ``_logger`` GELF field. :type facility: str - :param level_names: If :obj:`True` use string error level names - instead of numerical values (:obj:`False` by default). + :param level_names: If :obj:`True` add a ``_level_name`` GELF field + noting the logs error level as a string name. :type level_names: bool :param compress: If :obj:`True` compress the GELF message before @@ -158,9 +158,8 @@ def _make_gelf_dict(self, record): @staticmethod def _add_level_names(gelf_dict, record): - """Add the ``level_name`` field to the ``gelf_dict`` which notes - the logging level via the string error level names instead of - numerical values + """Add the ``_level_name`` field to the ``gelf_dict`` noting the logs + error level as a string name :param gelf_dict: dictionary representation of a GELF log. :type gelf_dict: dict @@ -169,7 +168,7 @@ def _add_level_names(gelf_dict, record): level from to insert into the given ``gelf_dict``. :type record: logging.LogRecord """ - gelf_dict['level_name'] = logging.getLevelName(record.levelno) + gelf_dict['_level_name'] = logging.getLevelName(record.levelno) @staticmethod def _set_custom_facility(gelf_dict, facility_value, record): diff --git a/tests/unit/test_handler.py b/tests/unit/test_handler.py index cffa01646..2ad9e8ac8 100644 --- a/tests/unit/test_handler.py +++ b/tests/unit/test_handler.py @@ -166,7 +166,7 @@ def test_status_field_issue(logger, mock_send): def test_add_level_name(): gelf_dict = dict() BaseGELFHandler._add_level_names(gelf_dict, MOCK_LOG_RECORD) - assert "INFO" == gelf_dict["level_name"] + assert "INFO" == gelf_dict["_level_name"] def test_resolve_host(): From 8819836bd387a6b20b92630c4cf45ebe3c75e763 Mon Sep 17 00:00:00 2001 From: Nathan Klapstein Date: Wed, 20 Mar 2019 16:09:47 -0600 Subject: [PATCH 08/11] adding check on format of additional GELF fields --- graypy/handler.py | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/graypy/handler.py b/graypy/handler.py index eca511936..ca025c23d 100644 --- a/graypy/handler.py +++ b/graypy/handler.py @@ -9,6 +9,7 @@ import logging import math import random +import re import socket import ssl import struct @@ -289,8 +290,24 @@ def _add_extra_fields(gelf_dict, record): 'processName', 'relativeCreated', 'thread', 'threadName') for key, value in record.__dict__.items(): - if key not in skip_list and not key.startswith('_'): - gelf_dict['_%s' % key] = value + if key not in skip_list: + additional_field_name = "_{}".format(key) + BaseGELFHandler.validate_gelf_additional_field_name(additional_field_name) + gelf_dict[additional_field_name] = value + + @staticmethod + def validate_gelf_additional_field_name(additional_field_name): + """Validate a GELF additional field name + + :param additional_field_name: GELF additional field name to validate + :type additional_field_name: str + + :raises ValueError: if the given GELF additional field name is invalid + """ + if re.match(r"^[\w.\-]*$", additional_field_name): + if additional_field_name != "_id": + return + raise ValueError("Invalid GELF additional field name") @staticmethod def _pack_gelf_dict(gelf_dict): From a08fc640691c600d020671d48cdec01ffe69e894 Mon Sep 17 00:00:00 2001 From: Nathan Klapstein Date: Wed, 20 Mar 2019 16:18:07 -0600 Subject: [PATCH 09/11] implementing checks for additional GELF fields --- graypy/handler.py | 5 ++--- tests/unit/test_handler.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/graypy/handler.py b/graypy/handler.py index ca025c23d..f31b865a6 100644 --- a/graypy/handler.py +++ b/graypy/handler.py @@ -291,9 +291,8 @@ def _add_extra_fields(gelf_dict, record): for key, value in record.__dict__.items(): if key not in skip_list: - additional_field_name = "_{}".format(key) - BaseGELFHandler.validate_gelf_additional_field_name(additional_field_name) - gelf_dict[additional_field_name] = value + BaseGELFHandler.validate_gelf_additional_field_name(key) + gelf_dict["_{}".format(key)] = value @staticmethod def validate_gelf_additional_field_name(additional_field_name): diff --git a/tests/unit/test_handler.py b/tests/unit/test_handler.py index 2ad9e8ac8..b4adcafb2 100644 --- a/tests/unit/test_handler.py +++ b/tests/unit/test_handler.py @@ -240,3 +240,34 @@ def test_glef_chunking(): assert expected_index == chunk[10:11] assert expected_chunks_count == chunk[11:12] assert expected_chunk == chunk[12:] + + +@pytest.mark.parametrize( + "field_name", + [ + "foo", + "bar", + "_bar", + "foo.bar", + "foo-bar", + "foo1" + ] +) +def test_validate_additional_field_name_valid(field_name): + BaseGELFHandler.validate_gelf_additional_field_name(field_name) + pass + + +@pytest.mark.parametrize( + "field_name", + [ + " ", + " foo", + "foo#", + "@", + "_id" + ] +) +def test_validate_additional_field_name_invalid(field_name): + with pytest.raises(ValueError): + BaseGELFHandler.validate_gelf_additional_field_name(field_name) From 43539fdfa78048d610c4ec17259134698e03fab9 Mon Sep 17 00:00:00 2001 From: Nathan Klapstein Date: Mon, 30 Sep 2019 18:38:52 -0400 Subject: [PATCH 10/11] using `_facility` in `simplified_gelf_dict` --- graypy/handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graypy/handler.py b/graypy/handler.py index 5950efd9c..f76237923 100644 --- a/graypy/handler.py +++ b/graypy/handler.py @@ -536,7 +536,7 @@ def gen_chunk_overflow_gelf_log(self, raw_message): "short_message": "", "timestamp": gelf_dict["timestamp"], "level": SYSLOG_LEVELS.get(logging.ERROR, logging.ERROR), - "facility": gelf_dict["facility"], + "_facility": gelf_dict["_facility"], "_chunk_overflow": True, } From 0b74b7e936a4542bbd8147478b1a8eb81b336395 Mon Sep 17 00:00:00 2001 From: Nathan Klapstein Date: Mon, 30 Sep 2019 18:49:22 -0400 Subject: [PATCH 11/11] using black formatting --- graypy/handler.py | 34 +++++++++++++++++++--------------- tests/unit/test_handler.py | 23 +++-------------------- 2 files changed, 22 insertions(+), 35 deletions(-) diff --git a/graypy/handler.py b/graypy/handler.py index f76237923..d9579af2d 100644 --- a/graypy/handler.py +++ b/graypy/handler.py @@ -136,12 +136,14 @@ def _make_gelf_dict(self, record): """ # construct the base GELF format gelf_dict = { - 'version': "1.1", - 'host': self._resolve_host(self.fqdn, self.localname), - 'short_message': self.formatter.format(record) if self.formatter else record.getMessage(), - 'timestamp': record.created, - 'level': SYSLOG_LEVELS.get(record.levelno, record.levelno), - '_facility': self.facility or record.name, + "version": "1.1", + "host": self._resolve_host(self.fqdn, self.localname), + "short_message": self.formatter.format(record) + if self.formatter + else record.getMessage(), + "timestamp": record.created, + "level": SYSLOG_LEVELS.get(record.levelno, record.levelno), + "_facility": self.facility or record.name, } # add in specified optional extras @@ -168,7 +170,7 @@ def _add_level_names(gelf_dict, record): level from to insert into the given ``gelf_dict``. :type record: logging.LogRecord """ - gelf_dict['_level_name'] = logging.getLevelName(record.levelno) + gelf_dict["_level_name"] = logging.getLevelName(record.levelno) @staticmethod def _set_custom_facility(gelf_dict, facility_value, record): @@ -189,7 +191,7 @@ def _set_custom_facility(gelf_dict, facility_value, record): field. :type record: logging.LogRecord """ - gelf_dict.update({"_facility": facility_value, '_logger': record.name}) + gelf_dict.update({"_facility": facility_value, "_logger": record.name}) @staticmethod def _add_full_message(gelf_dict, record): @@ -246,13 +248,15 @@ def _add_debugging_fields(gelf_dict, record): fields from to insert into the given ``gelf_dict``. :type record: logging.LogRecord """ - gelf_dict.update({ - '_file': record.pathname, - '_line': record.lineno, - '_function': record.funcName, - '_pid': record.process, - '_thread_name': record.threadName, - }) + gelf_dict.update( + { + "_file": record.pathname, + "_line": record.lineno, + "_function": record.funcName, + "_pid": record.process, + "_thread_name": record.threadName, + } + ) # record.processName was added in Python 2.6.2 pn = getattr(record, "processName", None) diff --git a/tests/unit/test_handler.py b/tests/unit/test_handler.py index 94fb0dbd9..bc87387a3 100644 --- a/tests/unit/test_handler.py +++ b/tests/unit/test_handler.py @@ -229,33 +229,16 @@ def test_invalid_client_certs(): # missing client cert GELFTLSHandler("127.0.0.1", keyfile="/dev/null") - + @pytest.mark.parametrize( - "field_name", - [ - "foo", - "bar", - "_bar", - "foo.bar", - "foo-bar", - "foo1" - ] + "field_name", ["foo", "bar", "_bar", "foo.bar", "foo-bar", "foo1"] ) def test_validate_additional_field_name_valid(field_name): BaseGELFHandler.validate_gelf_additional_field_name(field_name) pass -@pytest.mark.parametrize( - "field_name", - [ - " ", - " foo", - "foo#", - "@", - "_id" - ] -) +@pytest.mark.parametrize("field_name", [" ", " foo", "foo#", "@", "_id"]) def test_validate_additional_field_name_invalid(field_name): with pytest.raises(ValueError): BaseGELFHandler.validate_gelf_additional_field_name(field_name)