Clean up the mess related to configure_from_file and configure_from_dict

[piotrmaslanka]

• PySeq version: 0.4.0
• Python version: 3.11
• Operating System: Linux

Description

Those functions introduce more mess than they're worth. For example configure_from_file doesn't pass even half of it's kwargs to configure_from_dict , and they don't even respect the parameters put in the YAML file, instead of preferring to add their own.

This might require a version bump

What do you think, @tintoy ? Because I long abandoned any hope of using these functions and configure my Seq via log_to_seq

I can do it in my spare time, so if you want me to, just assign me to it.

I postulate to re-read the docs on how to implement custom logging and rewrite it.

[tintoy]

Were you thinking of removing the need for those functions entirely?

I’m ok with changing the way we configure Seqlog, but those functions are the result of several footgun experiences (remembering to replace the default logger class, remembering to choose correct feature flags) etc - Seqlog does a couple of nonstandard things to the logging library and I’m mindful of not breaking compatibility for existing consumers (where practical at least.

Having said, if you are willing to do the work then I’m happy to work with you to find a solution that satisfies both our needs 🙂

[piotrmaslanka]

Were you thinking of removing the need for those functions entirely?

I’m ok with changing the way we configure Seqlog, but those functions are the result of several footgun experiences (remembering to replace the default logger class, remembering to choose correct feature flags) etc - Seqlog does a couple of nonstandard things to the logging library and I’m mindful of not breaking compatibility for existing consumers (where practical at least.

Having said, if you are willing to do the work then I’m happy to work with you to find a solution that satisfies both our needs 🙂

Remember that you're still in a beta version, and according to semver your API might change twice a day. Once you however release 1.0.0, you're toast.

Besides, my software is totally happy with being configured like this, running off my patch:

with open(filename, 'r', encoding='utf-8') as f_in:
      data = yaml.load(f_in, Loader=yaml.SafeLoader)

  logging.config.dictConfig(data)

  kwargs = {'service': Config.service_name,
            'slot': Config.slot,
            'trace_id': add_trace_id,
            'span_id': add_span_id}

  if preforking:
      kwargs['pid'] = add_pid

seqlog.structured_logging.set_global_log_properties(**kwargs)

And the YAML file (later transformed is):

# This is the Python logging schema version (currently, only the value 1 is supported here).
version: 1

# Configure logging from scratch.
disable_existing_loggers: True

# Configure the root logger to use Seq
root:
  level: {'DEBUG' if Config.debug else 'INFO'}
  handlers:
    - seq
    - file
    - console

# You can also configure non-root loggers.
loggers:
  werkzeug:
    propagate: False
    level: WARNING
    handlers:
      - seq
      - file
      - console
  urllib3:
    propagate: False
    level: WARNING
    handlers:
      - seq
      - file
      - console
  sai5.basic.channels:
    propagate: False
    level: {'DEBUG' if Config.debug_grpc else 'WARNING'}
    handlers:
      - seq
      - console
      - file
  cassandra:
    propagate: False
    level: {'DEBUG' if Config.debug_cassandra else 'WARNING'}
    handlers:
      - seq
      - file
      - console
  coolamqp:
    propagate: False
    level: {'DEBUG' if Config.debug_coolamqp else 'WARNING'}
    handlers:
      - seq
      - console
      - file

handlers:

  # Log to Seq
  seq:
    class: seqlog.structured_logging.SeqLogHandler
    formatter: seq

    # Seq-specific settings (add any others you need, they're just kwargs for SeqLogHandler's constructor).
    server_url: '{Config.seq.url}'
    api_key: '{Config.seq.api_key}'
    batch_size: 10
    auto_flush_timeout: 5
    override_root_logger: true
    use_clef: false
    # Use a custom JSON encoder, if you need to.
    json_encoder_class: satella.json.JSONEncoder
    support_stack_info: true

  # Log to STDOUT
  console:
    class: seqlog.structured_logging.ConsoleStructuredLogHandler
    formatter: standard
    use_stdout: false

  file:
    class: logging.handlers.TimedRotatingFileHandler
    filename: '/smok5/logs/{Config.service_name}-{Config.slot}.txt'
    formatter: precise
    when: midnight
    delay: True

formatters:
  standard:
    format: '[%(levelname)s] %(asctime)s %(name)s: %(message)s'
  seq:
    style: '{'
  precise:
    format: '[%(levelname)s] %(asctime)s %(pathname)s %(lineno)s %(msg)s %(exc_info)s'

[tintoy]

I know technically we’re pre-release (according to semver) but tbh I usually take that to mean “haven’t had time to review and decide if what’s there makes sense given existing consumers” (and I’m not sure who existing consumers are at the moment).

But we can probably try it out and see what happens 🙂

[piotrmaslanka]

I know technically we’re pre-release (according to semver) but tbh I usually take that to mean “haven’t had time to review and decide if what’s there makes sense given existing consumers” (and I’m not sure who existing consumers are at the moment).

But we can probably try it out and see what happens 🙂

Well, for once I am, and I'm running it in commercial production, so I feel confident enough to let it loose.

[piotrmaslanka]

Ok, @tintoy, I made the API a bit more backwards compatible with more warnings against bugs that are there. Feel free to do #78 at any time.