Contextual logging #413
Contextual logging #413
Conversation
| @@ -46,6 +44,8 @@ def __call__( # type: ignore | |||
| control_url=control_url, | |||
| events_url=events_url, | |||
| initial_params=json.dumps(params), | |||
| request_id=request_id_var.get(), | |||
There was a problem hiding this comment.
@mjh1 i think instead of passing request_id everywhere, you can just add a configure_logging function in app.log, which you can just call inside the piplinestreamer class init method, that way it'll be a bit clean and all the logging params setting is localized to a single place, easier to add and remove things later on
There was a problem hiding this comment.
@varshith15 That would be great, I'm unsure of how to do that though. I won't be able to set the requestID value in a class init method right but do you just mean setting up the logger? Are you suggesting using a single logging setup and removing all these different logging configs we have?
| @@ -160,12 +162,12 @@ def _setup_logging(self): | |||
|
|
|||
| level = logging.DEBUG if os.environ.get('VERBOSE_LOGGING') == '1' else logging.INFO | |||
| logging.basicConfig( | |||
| format='%(asctime)s %(levelname)-8s %(message)s', | |||
| format=f"%(asctime)s %(levelname)-8s request_id={self.request_id} stream_id={self.stream_id} %(message)s", | |||
| level=level, | |||
| datefmt='%Y-%m-%d %H:%M:%S') | |||
|
|
|||
| queue_handler = LogQueueHandler(self) | |||
There was a problem hiding this comment.
maybe adding the main queue config_logging to LogQueueHandler init should remove all this extra param passing
There was a problem hiding this comment.
I tried that in this commit but it can't even import it 😕 can you see what the issue is?
[infer.py] Traceback (most recent call last):
[infer.py] File "/app/app/live/infer.py", line 12, in <module>
[infer.py] from streamer import PipelineStreamer
[infer.py] File "/app/app/live/streamer/__init__.py", line 1, in <module>
[infer.py] from .streamer import PipelineStreamer
[infer.py] File "/app/app/live/streamer/streamer.py", line 13, in <module>
[infer.py] from .process import PipelineProcess
[infer.py] File "/app/app/live/streamer/process.py", line 16, in <module>
[infer.py] from app.log import config_logging
[infer.py] ModuleNotFoundError: No module named 'app.log'
There was a problem hiding this comment.
Is everything under app/live a totally separate app?
| def config_logging(): | ||
| formatter = logging.Formatter( | ||
| "%(asctime)s - %(name)s - %(levelname)s - request_id=%(request_id)s stream_id=%(stream_id)s %(message)s", | ||
| defaults={"request_id": "", "stream_id": ""}, |
There was a problem hiding this comment.
maybe this is not needed? contextfilter is gonna add them anyway
To make debugging easier this change implements tagging of logs with some identifiers,
request_idwhich is generated by the gateway is specific to a single session andstream_id. The runner receives the IDs from the orchestrator via request headers and configures the various loggers with them. With this change we can simply search our logs with one of the IDs and view all relevant logs (across the different apps G, O and runner).Example:
My python experience is pretty limited so please let me know what I could do better. I'd particularly like to avoid the switching from
loggertologginginrunner/app/pipelines/live_video_to_video.py, whatever I tried it seemed like thelogging.getLogger(__name__)logger wouldn't have theContextFilterfilter added to it.