From 67e96cde7f0a7f80a1d962ccd5ef356630a8f2eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Piotr=20Ma=C5=9Blanka?= <piotr.maslanka@henrietta.com.pl>
Date: Fri, 20 Dec 2024 15:32:46 +0100
Subject: [PATCH] fixes #76

---
 docs/conf.py                    |  10 +++
 docs/index.rst                  |   1 +
 docs/migration.rst              |  22 +++++++
 docs/usage.rst                  |  31 ++--------
 seqlog/__init__.py              |  35 +++++------
 seqlog/feature_flags.py         |  77 -----------------------
 seqlog/structured_logging.py    | 104 ++++++++++----------------------
 tests/test_configuration.py     |   4 +-
 tests/test_structured_logger.py |   3 +-
 9 files changed, 88 insertions(+), 199 deletions(-)
 create mode 100644 docs/migration.rst
 delete mode 100644 seqlog/feature_flags.py

diff --git a/docs/conf.py b/docs/conf.py
index 6473847..d203694 100755
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -54,6 +54,16 @@
 # The master toctree document.
 master_doc = 'index'
 
+autodoc_default_options = {
+    'members': True,
+}
+autodoc_default_flags = [
+    'show-inheritance'
+]
+autodoc_typehints = "description"
+autoclass_content = 'both'
+
+
 # General information about the project.
 project = u'SeqLog'
 copyright = u"2016, Adam Friedman"
diff --git a/docs/index.rst b/docs/index.rst
index 08a06b9..ca420ae 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -17,6 +17,7 @@ Contents:
 
    readme
    installation
+   migration
    usage
    usage-gunicorn
    Modules <api/modules>
diff --git a/docs/migration.rst b/docs/migration.rst
new file mode 100644
index 0000000..a0af02e
--- /dev/null
+++ b/docs/migration.rst
@@ -0,0 +1,22 @@
+===============================
+Migration guide from 0.4 to 0.5
+===============================
+
+First of all, the official way to configure Seq is via
+
+.. autofunction:: seqlog.configure_from_dict
+
+Alternatively you can call
+
+.. autofunction:: seqlog.configure_from_file
+
+
+.. warning:: DO NOT call :code:`logging.config.fromDict`
+
+Then, FeatureFlags were completely obliterated and moved to SeqLogHandler's constructor.
+
+Then, the SeqLogHandler accepts way more arguments that you can define in this dict:
+
+.. autoclass:: seqlog.structured_logging.SeqLogHandler
+    :members:
+
diff --git a/docs/usage.rst b/docs/usage.rst
index b709a42..c1c73ba 100644
--- a/docs/usage.rst
+++ b/docs/usage.rst
@@ -58,6 +58,8 @@ First, create your configuration file (e.g. ``/foo/bar/my_config.yml``):
 
     # Configure logging from scratch.
     disable_existing_loggers: True
+    override_root_logger: True
+    use_structured_logger: True
 
     # Configure the root logger to use Seq
     root:
@@ -98,7 +100,7 @@ First, create your configuration file (e.g. ``/foo/bar/my_config.yml``):
       seq:
         style: '{'
 
-Then, call ``seqlog.configure_from_file()``:
+Then, call :func:`seqlog.configure_from_file`:
 
 .. code-block:: python
 
@@ -115,11 +117,9 @@ Then, call ``seqlog.configure_from_file()``:
 Configuring logging from a dictionary
 -------------------------------------
 
-.. deprecated:: 0.5.0
-    Use logging.config.dictConfig() directly
-
 Seqlog can also use a dictionary to describe the desired logging configuration.
 This dictionary has the schema specified in Python's `logging.config <https://docs.python.org/3/library/logging.config.html#logging-config-dictschema>`_ module.
+With some extra options described in :func:`seqlog.configure_from_dict`.
 
 .. code-block:: python
 
@@ -137,29 +137,6 @@ This dictionary has the schema specified in Python's `logging.config <https://do
     another_logger = logging.getLogger('another_logger')
     another_logger.info('This is another logger.')
 
-Note that you can pass flags that were previously given to :func:`seqlog.configure_from_dict` directly in the dictionary, eg.
-
-.. autofunction:: seqlog.configure_from_dict
-
-.. code-block:: python
-
-    a['handlers']['console'] = {
-        'class': 'seqlog.structured_logging.ConsoleStructuredLogHandler',
-        'formatter': 'standard'
-        'override_root_logger': True
-        'use_structured_logging': True,
-        'use_clef': True        
-    }
-    logging.config.dictConfig(a)
-
-Basically all of the arguments in
-
-.. autoclass:: seqlog.structured_logging.BaseStructuredLogHandler
-
-can be put there.
-
-Note that only first arguments that were previously passed globally will be set. Argument configured once in one logger
-won't pass to another.
 
 Batching and auto-flush
 -----------------------
diff --git a/seqlog/__init__.py b/seqlog/__init__.py
index 2d1645e..a938ccf 100644
--- a/seqlog/__init__.py
+++ b/seqlog/__init__.py
@@ -7,7 +7,6 @@
 
 import yaml
 
-from seqlog.feature_flags import FeatureFlag, configure_feature
 from seqlog.structured_logging import StructuredLogger, StructuredRootLogger, _override_root_logger
 from seqlog.structured_logging import SeqLogHandler, ConsoleStructuredLogHandler
 from seqlog.structured_logging import get_global_log_properties as _get_global_log_properties
@@ -23,7 +22,8 @@
 
 def configure_from_file(file_name):
     """
-    Configure Seq logging using YAML-format configuration file.
+    Configure Seq logging using YAML-format configuration file. Essentially loads the YAML, and invokes
+    :func:`configure_from_dict`.
 
     Uses `logging.config.dictConfig()`.
     """
@@ -31,24 +31,25 @@ def configure_from_file(file_name):
     with open(file_name) as config_file:
         config = yaml.load(config_file, Loader=yaml.SafeLoader)
 
-    logging.config.dictConfig(config)
+    configure_from_dict(config)
 
 
 def configure_from_dict(config):
     """
-    Configure Seq logging using a dictionary.
-
-    Uses `logging.config.dictConfig()`.
+    Configure Seq logging using a dictionary. Use it instead of logging.config.dictConfig().
 
-    .. deprecated: 0.5.0
-        Use logging.config.dictConfig() directly.
+    Extra parameters you can specify (as dictionary keys).
 
-    Note that if you provide None to any of the default arguments, it just won't get changed (ie. it will stay the same).
+    * `use_structured_logger` - this will configure Python logging environment to use a StructuredLogger, ie. one that
+       understands keyword arguments
+    * `override_root_logger` - overrides root logger with a StructuredLogger
 
-    :param config: A dict containing the configuration.
-    :type config: dict
+    The rest will be passed onto logging.config.dictConfig()
     """
-    warnings.warn('This is deprecated, use logging.config.dictConfig() directly', DeprecationWarning)
+    if config.pop('use_structured_logger', False):
+        logging.setLoggerClass(StructuredLogger)
+    if config.pop('override_root_logger', False):
+        _override_root_logger()
     logging.config.dictConfig(config)
 
 
@@ -86,19 +87,15 @@ def log_to_seq(server_url, api_key=None, level=logging.WARNING,
     :return: The `SeqLogHandler` that sends events to Seq. Can be used to forcibly flush records to Seq.
     :rtype: SeqLogHandler
     """
-
-    configure_feature(FeatureFlag.EXTRA_PROPERTIES, support_extra_properties)
-    configure_feature(FeatureFlag.STACK_INFO, support_stack_info)
-    configure_feature(FeatureFlag.IGNORE_SEQ_SUBMISSION_ERRORS, ignore_seq_submission_errors)
-    configure_feature(FeatureFlag.USE_CLEF, use_clef)
-
     logging.setLoggerClass(StructuredLogger)
 
     if override_root_logger:
         _override_root_logger()
 
     log_handlers = [
-        SeqLogHandler(server_url, api_key, batch_size, auto_flush_timeout, json_encoder_class)
+        SeqLogHandler(server_url, api_key, batch_size, auto_flush_timeout, json_encoder_class,
+                      support_extra_properties=support_extra_properties, support_stack_info=support_stack_info,
+                      use_clef=use_clef, ignore_seq_submission_errors=ignore_seq_submission_errors)
     ]
 
     if additional_handlers:
diff --git a/seqlog/feature_flags.py b/seqlog/feature_flags.py
deleted file mode 100644
index aa8526f..0000000
--- a/seqlog/feature_flags.py
+++ /dev/null
@@ -1,77 +0,0 @@
-# -*- coding: utf-8 -*-
-import typing as tp
-from enum import Enum
-
-
-class FeatureFlag(Enum):
-    """
-    Well-known feature flags.
-    """
-
-    EXTRA_PROPERTIES = 1  #: Support passing of additional properties to log via the `extra` argument?
-
-    STACK_INFO = 2  #: Support attaching of stack-trace information (if available) to log records?
-
-    IGNORE_SEQ_SUBMISSION_ERRORS = 3   #: Ignore errors encountered while sending log records to Seq?
-
-    USE_CLEF = 4    #: Use more modern API to submit log entries
-
-
-# Here None (despite for if purposes being False) carries additional meaning - that this entry was not yet configured
-_features = {
-    FeatureFlag.EXTRA_PROPERTIES: None,
-    FeatureFlag.STACK_INFO: None,
-    FeatureFlag.IGNORE_SEQ_SUBMISSION_ERRORS: None,
-    FeatureFlag.USE_CLEF: None
-}
-
-
-def is_feature_enabled(feature: FeatureFlag):
-    """
-    Is a feature enabled?
-
-    :param feature: A `FeatureFlag` value representing the feature.
-    :type feature: FeatureFlag
-    :return: `True`, if the feature is enabled; otherwise, `False`.
-    :rtype: bool
-    """
-
-    return _features.get(feature, False)
-
-
-def enable_feature(feature: FeatureFlag):
-    """
-    Enable a feature.
-
-    :param feature: A `FeatureFlag` value representing the feature to enable.
-    :type feature: FeatureFlag
-    """
-
-    configure_feature(feature, True)
-
-
-def disable_feature(feature: FeatureFlag):
-    """
-    Disable a feature.
-
-    :param feature: A `FeatureFlag` value representing the feature to disable.
-    :type feature: FeatureFlag
-    """
-
-    configure_feature(feature, False)
-
-
-def configure_feature(feature: FeatureFlag, enable: tp.Optional[bool], if_not_yet_configured: bool = False):
-    """
-    Enable or disable a feature.
-
-    :param feature: A `FeatureFlag` value representing the feature to configure. If you pass None, it won't get changed.
-    :type feature: FeatureFlag
-    :param enable: `True`, to enable the feature; `False` to disable it.
-    :param if_not_yet_configured: configure only if this has not yet been configured
-    """
-    if enable is None:
-        return
-    if not if_not_yet_configured and _features[feature] is None:
-        return
-    _features[feature] = enable
diff --git a/seqlog/structured_logging.py b/seqlog/structured_logging.py
index 4a2e779..23e3663 100644
--- a/seqlog/structured_logging.py
+++ b/seqlog/structured_logging.py
@@ -17,7 +17,6 @@
 import requests
 
 from seqlog.consumer import QueueConsumer
-from seqlog.feature_flags import FeatureFlag, is_feature_enabled, configure_feature
 
 # Well-known keyword arguments used by the logging system.
 _well_known_logger_kwargs = {"extra", "exc_info", "func", "sinfo"}
@@ -165,49 +164,19 @@ def getMessage(self):
             return self.msg
 
 
-_root_logger_overrided = False
-
-
 def _override_root_logger():
     """
     Override the root logger with a `StructuredRootLogger`.
     """
-    global _root_logger_overrided
-    if _root_logger_overrided:
-        return
     logging.root = StructuredRootLogger(logging.WARNING)
     logging.Logger.root = logging.root
     logging.Logger.manager = logging.Manager(logging.Logger.root)
-    _root_logger_overrided = True
 
 
 class BaseStructuredLogHandler(logging.Handler):
-    """
-    Base structured logger to set up all of the arguments previously required.
-
-    :param override_root_logger: whether to override the root logger, default is True
-    :param support_extra_properties: logger will support named arguments instead of passing them via extra
-    :param stack_info: attach stack info
-    :param ignore_seq_submission_errors: whether to ignore submission errors
-    :param use_clef: whether to use CLEF
-    """
-    def __init__(self, *args, level=logging.NOTSET, override_root_logger=True, **kwargs):
-        logging.Handler.__init__(self, level)
-
-        if override_root_logger:
-            _override_root_logger()
-
-        if kwargs.pop('use_structured_logger', None):
-            logging.setLoggerClass(StructuredLogger)
-
-        if kwargs.pop('support_extra_properties', None):
-            configure_feature(FeatureFlag.EXTRA_PROPERTIES, True, if_not_yet_configured=True)
-        if kwargs.pop('stack_info', None):
-            configure_feature(FeatureFlag.STACK_INFO, True, if_not_yet_configured=True)
-        if kwargs.pop('ignore_seq_submission_errors', None):
-            configure_feature(FeatureFlag.IGNORE_SEQ_SUBMISSION_ERRORS, True, if_not_yet_configured=True)
-        if kwargs.pop('use_clef', None):
-            configure_feature(FeatureFlag.USE_CLEF, True, if_not_yet_configured=True)
+    def __init__(self, level=logging.NOTSET, *args, support_extra_properties=False, **kwargs):
+        logging.Handler.__init__(self, level=level)
+        self.support_extra_properties = support_extra_properties
 
 
 class StructuredLogger(logging.Logger):
@@ -215,10 +184,6 @@ class StructuredLogger(logging.Logger):
     Custom (dummy) logger that understands named log arguments.
     """
 
-    @property
-    def _support_extra_properties(self):
-        return is_feature_enabled(FeatureFlag.EXTRA_PROPERTIES)
-
     def _log(self, level, msg, args, exc_info=None, extra=None, stack_info=False, **kwargs):
         """
         Called by public logger methods to generate a log entry.
@@ -226,7 +191,7 @@ def _log(self, level, msg, args, exc_info=None, extra=None, stack_info=False, **
         :param msg: The log message or message template.
         :param args: Ordinal arguments for the message format template.
         :param exc_info: Exception information to be included in the log entry.
-        :param extra: Extra information to be included in the log entry.
+        :param extra: Extra information to be included in the log entry. Note that this will take precedence over kwargs.
         :param stack_info: Include stack-trace information in the log entry?
         :param kwargs: Keyword arguments (if any) passed to the public logger method that called _log.
         """
@@ -247,7 +212,7 @@ def _log(self, level, msg, args, exc_info=None, extra=None, stack_info=False, **
 
             log_props[prop] = kwargs[prop]
 
-        if extra and self._support_extra_properties:
+        if extra:
             for extra_prop in extra.keys():
                 log_props['Extra_' + extra_prop] = extra[extra_prop]
 
@@ -388,7 +353,8 @@ class SeqLogHandler(BaseStructuredLogHandler):
     """
 
     def __init__(self, server_url, api_key=None, batch_size=10, auto_flush_timeout=None, json_encoder_class=None,
-                 **kwargs):
+                 use_clef: bool = False, ignore_seq_submission_errors: bool = False, support_stack_info: bool = False,
+                 support_extra_properties: bool = False, **kwargs):
         """
         Create a new `SeqLogHandler`.
 
@@ -398,14 +364,26 @@ def __init__(self, server_url, api_key=None, batch_size=10, auto_flush_timeout=N
         :param auto_flush_timeout: If specified, the time (in seconds) before
                                    the current batch is automatically flushed.
         :param json_encoder_class: The custom JSON encoder class (or fully-qualified class name), if any, to use.
+        :param use_clef: whether to use the CLEF format
+        :param ignore_seq_submission_errors: whether to ignore failures to send logs to Seq
+        :param support_stack_info: whether to recognize stack_info as an alternative to exc_info (with some limitations)
+        :poram support_extra_properties: whether to natively recognize kwargs (if True) or supply them as extra
+                                        (if False).
         """
 
-        super().__init__(**kwargs)
+        super().__init__(support_extra_properties=support_extra_properties, **kwargs)
+
+        server_url = server_url
+        if not server_url.endswith("/"):
+            server_url += "/"
 
-        self.base_server_url = server_url
-        if not self.base_server_url.endswith("/"):
-            self.base_server_url += "/"
+        if use_clef:
+            self.server_url = server_url + 'ingest/clef'
+        else:
+            self.server_url = server_url + 'api/events/raw'
 
+        self.support_stack_info = support_stack_info
+        self.ignore_seq_submission_errors = ignore_seq_submission_errors
         self.session = requests.Session()
 
         if api_key:
@@ -413,7 +391,7 @@ def __init__(self, server_url, api_key=None, batch_size=10, auto_flush_timeout=N
 
         json_encoder_class = json_encoder_class or json.encoder.JSONEncoder
         self.json_encoder_class = _ensure_class(json_encoder_class, compatible_class=json.encoder.JSONEncoder)
-
+        self.use_clef = use_clef
         self.log_queue = Queue()
         self.consumer = QueueConsumer(
             name="SeqLogHandler",
@@ -424,24 +402,6 @@ def __init__(self, server_url, api_key=None, batch_size=10, auto_flush_timeout=N
         )
         self.consumer.start()
 
-    @property
-    def server_url(self):
-        if self._use_clef:
-            return self.base_server_url + 'ingest/clef'
-        return self.base_server_url + 'api/events/raw'
-
-    @property
-    def _use_clef(self):
-        return is_feature_enabled(FeatureFlag.USE_CLEF)
-
-    @property
-    def _support_stack_info(self):
-        return is_feature_enabled(FeatureFlag.STACK_INFO)
-
-    @property
-    def _ignore_seq_submission_errors(self):
-        return is_feature_enabled(FeatureFlag.IGNORE_SEQ_SUBMISSION_ERRORS)
-
     def flush(self):
         try:
             self.consumer.flush()
@@ -496,7 +456,7 @@ def publish_log_batch(self, batch):     # type: (tp.List[StructuredLogRecord]) -
                 continue
             processed_records.append(resp)
 
-        if self._use_clef:
+        if self.use_clef:
             request_body_json = '\r\n'.join(processed_records)
         else:
             request_body_json = '{"Events": [%s]}' % (','.join(processed_records), )
@@ -507,12 +467,12 @@ def publish_log_batch(self, batch):     # type: (tp.List[StructuredLogRecord]) -
             response = self.session.post(
                 self.server_url,
                 data=request_body_json,
-                headers={'Content-Type': "application/vnd.serilog.clef" if self._use_clef else 'application/json'},
+                headers={'Content-Type': "application/vnd.serilog.clef" if self.use_clef else 'application/json'},
                 stream=True  # prevent '362'
             )
             response.raise_for_status()
         except requests.RequestException as requestFailed:
-            if not self._ignore_seq_submission_errors:
+            if not self.ignore_seq_submission_errors:
                 # Only notify for the first record in the batch, or we'll be generating too much noise.
                 self.handleError(batch[0])
 
@@ -544,7 +504,7 @@ def handleError(self, record: StructuredLogRecord):
             _callback_on_failure(exception)
 
     def _build_event_data(self, record):
-        if self._use_clef:
+        if self.use_clef:
             return self._build_event_data_clef(record)
         else:
             return self._build_event_data_ingest(record)
@@ -588,12 +548,12 @@ def _build_event_data_ingest(self, record):
         if record.exc_text:
             # Rendered exception has already been cached
             event_data["Exception"] = record.exc_text
-        elif self._support_stack_info and record.stack_info and not record.exc_info:
+        elif self.support_stack_info and record.stack_info and not record.exc_info:
             # Feature flag is set: fall back to stack_info (sinfo) if exc_info is not present
             event_data["Exception"] = record.stack_info
         elif isinstance(record.exc_info, tuple):
             # Exception info is present
-            if record.exc_info[0] is None and self._support_stack_info and record.stack_info:
+            if record.exc_info[0] is None and self.support_stack_info and record.stack_info:
                 event_data["Exception"] = "{0}--NoException\n{1}".format(logging.getLevelName(record.levelno), record.stack_info)
             else:
                 event_data["Exception"] = record.exc_text = self.formatter.formatException(record.exc_info)
@@ -651,12 +611,12 @@ def _build_event_data_clef(self, record):
         if record.exc_text:
             # Rendered exception has already been cached
             event_data["@x"] = record.exc_text
-        elif self._support_stack_info and record.stack_info and not record.exc_info:
+        elif self.support_stack_info and record.stack_info and not record.exc_info:
             # Feature flag is set: fall back to stack_info (sinfo) if exc_info is not present
             event_data["@x"] = record.stack_info
         elif isinstance(record.exc_info, tuple):
             # Exception info is present
-            if record.exc_info[0] is None and self._support_stack_info and record.stack_info:
+            if record.exc_info[0] is None and self.support_stack_info and record.stack_info:
                 event_data["@x"] = "{0}--NoException\n{1}".format(logging.getLevelName(record.levelno), record.stack_info)
             else:
                 event_data["@x"] = record.exc_text = self.formatter.formatException(record.exc_info)
diff --git a/tests/test_configuration.py b/tests/test_configuration.py
index 3291182..09ef81f 100644
--- a/tests/test_configuration.py
+++ b/tests/test_configuration.py
@@ -24,6 +24,8 @@
 disable_existing_loggers: True
 
 # Configure the root logger to use Seq
+override_root_logger: True
+use_structured_logger: True
 root:
   level: 'DEBUG'
   handlers:
@@ -32,8 +34,6 @@
   console:
     class: seqlog.structured_logging.ConsoleStructuredLogHandler
     formatter: standard
-    override_root_logger: True
-    use_structured_logging: True
 formatters:
   standard:
     format: '[%(levelname)s] %(asctime)s %(name)s: %(message)s'
diff --git a/tests/test_structured_logger.py b/tests/test_structured_logger.py
index 6843707..0fd9b00 100644
--- a/tests/test_structured_logger.py
+++ b/tests/test_structured_logger.py
@@ -12,8 +12,7 @@
 
 import tests.assertions as expect
 
-from seqlog import clear_global_log_properties
-from seqlog.structured_logging import StructuredLogger
+from seqlog.structured_logging import StructuredLogger, clear_global_log_properties
 from tests.stubs import StubStructuredLogHandler