-
Notifications
You must be signed in to change notification settings - Fork 90
Add structured logging support #2029
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
base: develop
Are you sure you want to change the base?
Changes from 2 commits
bd5aee4
9339049
29a61c8
a219a2f
a09d81b
4a1a6fa
987ee28
9436834
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,16 +3,161 @@ | |||||
| import copy | ||||||
| import getpass | ||||||
| import logging | ||||||
| import queue | ||||||
| from logging.handlers import QueueHandler, QueueListener | ||||||
| import os | ||||||
| import platform | ||||||
| import requests | ||||||
| import requests.adapters | ||||||
| import socket | ||||||
| import sys | ||||||
| import time | ||||||
| import threading | ||||||
| import warnings | ||||||
|
|
||||||
| import structlog | ||||||
|
|
||||||
| from . import Terminal | ||||||
|
|
||||||
| VECTOR_LOG_URL = os.getenv("AYON_VECTOR_LOG_URL", None) | ||||||
|
|
||||||
|
|
||||||
| class _RawQueueHandler(QueueHandler): | ||||||
| """QueueHandler that does not pre-format/stringify the record. | ||||||
|
|
||||||
| The stdlib's default 'prepare' stringifies 'record.msg', which | ||||||
| destroys the structlog event dict before it reaches the listener's | ||||||
| handlers. | ||||||
| """ | ||||||
|
|
||||||
| def prepare(self, record): | ||||||
| return record | ||||||
|
|
||||||
|
|
||||||
| class VectorHTTPHandler(logging.Handler): | ||||||
| """Forward formatted log records to a Vector HTTP source.""" | ||||||
|
|
||||||
| def __init__(self, url): | ||||||
| super().__init__() | ||||||
| self._url = url | ||||||
| # Reuse a single session so repeated POSTs reuse pooled | ||||||
| # connections instead of opening a new one per log record. | ||||||
| self._session = requests.Session() | ||||||
| adapter = requests.adapters.HTTPAdapter( | ||||||
| pool_connections=1, pool_maxsize=10 | ||||||
| ) | ||||||
| self._session.mount("http://", adapter) | ||||||
| self._session.mount("https://", adapter) | ||||||
|
|
||||||
| def emit(self, record): | ||||||
| try: | ||||||
| self._session.post( | ||||||
| self._url, | ||||||
| data=self.format(record), | ||||||
| headers={"Content-Type": "application/json"}, | ||||||
| timeout=1, | ||||||
| ) | ||||||
| except Exception: | ||||||
| self.handleError(record) | ||||||
|
|
||||||
| def close(self): | ||||||
| self._session.close() | ||||||
| super().close() | ||||||
|
|
||||||
|
|
||||||
| def configure_logger() -> None: | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to call this explicitly?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you need to configure the logger somewhere. It needs some entrypoint. The goal is to find out all the entrypoints and add this there. |
||||||
| """Configure logging for the application. | ||||||
|
|
||||||
| Including structlog and handlers for console and Vector HTTP. | ||||||
|
|
||||||
| Safe to call multiple times, and safe even if another package (e.g. | ||||||
| 'ayon_common' in ayon-launcher) configures logging first - only the | ||||||
| first call in the process has any effect, to avoid attaching | ||||||
| duplicate handlers. | ||||||
|
|
||||||
| """ | ||||||
| # 'structlog.is_configured()' is process-wide, so it also guards | ||||||
| # against other packages configuring logging first. | ||||||
| if structlog.is_configured(): | ||||||
| return | ||||||
|
|
||||||
| def _add_site_id(logger, method_name, event_dict): | ||||||
| event_dict.setdefault( | ||||||
| "site_id", os.environ.get("AYON_SITE_ID", "unknown") | ||||||
| ) | ||||||
| return event_dict | ||||||
|
|
||||||
| def _drop_site_id(logger, method_name, event_dict): | ||||||
| # Keep 'site_id' in JSON sent to Vector but not in console output | ||||||
| event_dict.pop("site_id", None) | ||||||
| return event_dict | ||||||
|
|
||||||
| shared_processors = [ | ||||||
| structlog.processors.add_log_level, | ||||||
| structlog.stdlib.add_logger_name, | ||||||
| structlog.processors.TimeStamper(fmt="iso"), | ||||||
| structlog.processors.StackInfoRenderer(), | ||||||
| _add_site_id, | ||||||
| ] | ||||||
|
|
||||||
| structlog.configure( | ||||||
| processors=shared_processors + [ | ||||||
| # Prepares details if sent to standard logging | ||||||
| structlog.stdlib.ProcessorFormatter.wrap_for_formatter, | ||||||
| ], | ||||||
| logger_factory=structlog.stdlib.LoggerFactory(), | ||||||
| wrapper_class=structlog.stdlib.BoundLogger, | ||||||
| cache_logger_on_first_use=True, | ||||||
| ) | ||||||
|
|
||||||
| console_formatter = structlog.stdlib.ProcessorFormatter( | ||||||
| foreign_pre_chain=shared_processors + [ | ||||||
| structlog.stdlib.PositionalArgumentsFormatter(), | ||||||
| ], | ||||||
| processors=[ | ||||||
| structlog.stdlib.ProcessorFormatter.remove_processors_meta, | ||||||
| _drop_site_id, | ||||||
| structlog.dev.ConsoleRenderer(), | ||||||
| ], | ||||||
| ) | ||||||
| json_formatter = structlog.stdlib.ProcessorFormatter( | ||||||
| foreign_pre_chain=shared_processors, | ||||||
| processors=[ | ||||||
| structlog.stdlib.ProcessorFormatter.remove_processors_meta, | ||||||
| _drop_site_id, | ||||||
| structlog.processors.JSONRenderer(), | ||||||
| ], | ||||||
| ) | ||||||
|
antirotor marked this conversation as resolved.
|
||||||
|
|
||||||
| handler = logging.StreamHandler(sys.stdout) | ||||||
| handler.setFormatter(console_formatter) | ||||||
|
|
||||||
| if VECTOR_LOG_URL: | ||||||
| # Send logs to Vector asynchronously so HTTP calls don't block the app. | ||||||
| vector_handler = VectorHTTPHandler(VECTOR_LOG_URL) | ||||||
| vector_handler.setFormatter(json_formatter) | ||||||
| log_queue = queue.Queue(-1) | ||||||
| queue_handler = _RawQueueHandler(log_queue) | ||||||
| queue_listener = QueueListener( | ||||||
| log_queue, vector_handler, respect_handler_level=True | ||||||
| ) | ||||||
| queue_listener.start() | ||||||
|
|
||||||
| root_logger = logging.getLogger() | ||||||
| root_logger.addHandler(handler) | ||||||
| if VECTOR_LOG_URL: | ||||||
| root_logger.addHandler(queue_handler) | ||||||
| root_logger.setLevel( | ||||||
| logging.INFO if os.getenv("AYON_DEBUG") != "1" else logging.DEBUG) | ||||||
|
|
||||||
| # 'Logger' (ayon_core.lib.log) may have attached its own fallback | ||||||
| # console handler to the "AYON" logger before structlog was configured. | ||||||
| # Drop it and let records propagate to the root logger instead, which | ||||||
| # now owns the shared handlers - avoids logging each record twice. | ||||||
| ayon_logger = Logger.get_root_logger() | ||||||
| for old_handler in list(ayon_logger.handlers): | ||||||
| ayon_logger.removeHandler(old_handler) | ||||||
|
|
||||||
|
|
||||||
| class LogStreamHandler(logging.StreamHandler): | ||||||
| """StreamHandler class. | ||||||
|
|
@@ -138,10 +283,15 @@ class Logger: | |||||
|
|
||||||
| @classmethod | ||||||
| @_deprecated_getter | ||||||
| def get_logger(cls, name: str) -> logging.Logger: | ||||||
| def get_logger(cls, name: str) -> structlog.BoundLogger | logging.Logger: | ||||||
| if not cls.initialized: | ||||||
| cls.initialize() | ||||||
|
|
||||||
| # Delegate to structlog when configured so records share the same | ||||||
| # processors (e.g. 'site_id', timestamps) as the rest of the app. | ||||||
| if structlog.is_configured(): | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does completelly skip the existing AYON logic. I just don't understand?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, it use structlog configured logger as root logger. Eventually we can completely remove |
||||||
| return structlog.get_logger(name or "__main__") | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this followed the same logic already in place but even there |
||||||
|
|
||||||
|
antirotor marked this conversation as resolved.
|
||||||
| logger = logging.getLogger(name or "__main__") | ||||||
| logger.setLevel(cls.log_level) | ||||||
| logger.parent = cls._root_logger | ||||||
|
|
@@ -191,9 +341,12 @@ def _initialize(cls): | |||||
| log_level = 20 | ||||||
| cls.log_level = int(log_level) | ||||||
| root_logger = logging.getLogger("AYON") | ||||||
| root_logger.propagate = False | ||||||
| # root_logger.propagate = False | ||||||
| root_logger.setLevel(cls.log_level) | ||||||
| root_logger.addHandler(cls._get_console_handler()) | ||||||
| # Skip own handler when structlog already owns the output pipeline | ||||||
| # to avoid double-formatting/handling the same records. | ||||||
| if not structlog.is_configured(): | ||||||
| root_logger.addHandler(cls._get_console_handler()) | ||||||
| cls._root_logger = root_logger | ||||||
|
|
||||||
| # Mark as initialized | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,6 @@ | |
| import pyblish.logic | ||
| import pyblish.plugin | ||
|
|
||
| from ayon_core.lib import Logger | ||
| from ayon_core.settings import get_project_settings | ||
| from ayon_core.pipeline.plugin_discover import DiscoverResult | ||
|
|
||
|
|
@@ -1018,7 +1017,6 @@ def _inner_publish_iter(self) -> PublishIterGen: | |
| @contextmanager | ||
| def _log_manager(self, plugin: PluginType): | ||
| root = logging.getLogger() | ||
| ayon_root = Logger.get_root_logger() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this removed? We added this so we can see logs in publish report viewer.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch, this is misfire. I tried to figure out double-logging. In any way - if we start using structlog, we could rewrite the publisher loging as another sink with it's own processors. I'll revert the changes. |
||
| plugin_log_has_handler = False | ||
| orig_propagate = plugin.log.propagate | ||
| if not self._log_to_console: | ||
|
|
@@ -1027,8 +1025,9 @@ def _log_manager(self, plugin: PluginType): | |
| if not plugin.log.propagate: | ||
| plugin_log_has_handler = True | ||
| plugin.log.addHandler(self._log_handler) | ||
| # "AYON" logger propagates into root by default, so attaching | ||
| # the handler here alone is enough to capture both trees. | ||
| root.addHandler(self._log_handler) | ||
| ayon_root.addHandler(self._log_handler) | ||
|
|
||
| try: | ||
| yield self._log_handler | ||
|
|
@@ -1038,7 +1037,6 @@ def _log_manager(self, plugin: PluginType): | |
| plugin.log.removeHandler(self._log_handler) | ||
| plugin.log.propagate = orig_propagate | ||
| root.removeHandler(self._log_handler) | ||
| ayon_root.removeHandler(self._log_handler) | ||
| self._log_handler.clear_records() | ||
|
|
||
| def _process_plugin( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,10 @@ dependencies = [ | |
| "numpy >=2.4.3", | ||
| "qtpy >=2.4.3", | ||
| "pyside6 >=6.8.3", | ||
| "Pillow ==9.5.0" | ||
| "Pillow ==9.5.0", | ||
| "structlog>=26.1.0", | ||
| "rich>=15.0.0", | ||
| "requests>=2.32.5", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Requests is defined by launcher, how usefull will this be?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when you run tests for example, they work with the |
||
| ] | ||
|
antirotor marked this conversation as resolved.
|
||
|
|
||
| [project.optional-dependencies] | ||
|
|
@@ -68,6 +71,7 @@ log_cli = true | |
| log_cli_level = "INFO" | ||
| addopts = "-ra -q" | ||
| testpaths = [ | ||
| "tests/ayon_core", | ||
| "client/ayon_core/tests", | ||
| "tests/client/ayon_core/ui", | ||
| ] | ||
|
antirotor marked this conversation as resolved.
|
||
|
|
@@ -83,3 +87,9 @@ markers = [ | |
| "slow: Slow tests", | ||
| "server: Tests that require a running AYON server", | ||
| ] | ||
|
|
||
| [tool.ty.environment] | ||
| extra-paths = [ | ||
| "client", | ||
| "tests/client/ayon_core/ui", | ||
| ] | ||
Uh oh!
There was an error while loading. Please reload this page.