Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(logger): unset CORVA_LOGGER.propagate = False for OTel Log Sending (refs EE-31) #90

Merged

Conversation

smoke
Copy link
Member

@smoke smoke commented Nov 6, 2024

Rationale

refs https://github.com/corva-ai/otel/pull/37
refs https://corvaqa.atlassian.net/browse/EE-31

Unset the CORVA_LOGGER.propagate = False
so the OTel handler will be able to collect those logs as well

Here details on what I learned for Python Logging:

  1. There is a Root Logger singletone, that is configured using logging.basicConfig()
  2. using logging.getLogger() gets you the Root Logger
  3. There are Child Loggers singletones, indexed by "name" that you can get via logging.getLogger("name")
  4. Child loggers by default emit items to Parent, at the end to Root Logger
  5. Each logger can have Handlers, used to print / send / collect the log record where it is needed
  6. Each log record is just a struct with a bunch of data - message, level, and other custom fields, it is up to the handler and formatter to make the most of it
  7. There are now 2 common scenarios for apps development:
  8. You are interested to print / send / collect each and every record regardless of logger - you use the logging.basicConfig() (or the Root Logger singletone directly) with proper handlers and formatters
  9. You are interested to print / send / collect your code stuff then use logging.getLogger(name) and configure with proper handlers and formatters your own singletones
  10. any other scenario would lead to confusion and issues
  11. Avoid setting propagate = False unless for very isolated / local needs - this way whatever needs to work with logs centrally can continue doing so from the Root logger

Aside - here snippet to do experiments if you wish

import logging

fmt = "%(asctime)s %(levelname)s [%(name)s] %(message)s"
logging.basicConfig(format=("root handler: " + fmt))

logger = logging.getLogger("child_logger")
logger.setLevel(logging.DEBUG)
ch = logging.StreamHandler()
# TODO: Figure out how to establish format that works with Uptrace and has OTEL_PYTHON_LOG_CORRELATION support
ch.setFormatter(logging.Formatter(("child handler: " + fmt)))
logger.addHandler(ch)

logger.info("hi there")

This "wrongly" configures handlers for both Root and Child logger, and will print:

child handler: 2024-11-06 17:02:30,741 INFO [child_logger] hi there
root handler: 2024-11-06 17:02:30,741 INFO [child_logger] hi there

Changes

Unset the CORVA_LOGGER.propagate = False
so the OTel handler will be able to collect those logs as well

TODO

  • Update CHANGELOG.md

@smoke smoke force-pushed the fix/logger-unset-propagation-for-otel-handler-to-collect-logs branch from 6587699 to a0d0e07 Compare November 6, 2024 15:40
@smoke smoke merged commit fcc7dfb into master Nov 6, 2024
4 checks passed
@smoke smoke deleted the fix/logger-unset-propagation-for-otel-handler-to-collect-logs branch November 6, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant