Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

Log originating request ID #324

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

apuntamb
Copy link
Contributor

As a beginner in yupana, created this small PR to fix logging of request_id for better traceability while debugging issues. Please review and feel free to correct in case something is missing.

Copy link

@ShimShtein ShimShtein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think better practice would be using LoggerAdapter to wrap log calls that are already emitted by the system with request_id.

Especially it's relevant for processors: abstract, report and slice. I see that the request_id is available through report_or_slice variable in pre_delegate. I suppose we can wrap the logger instance with LoggerAdapter inside the pre_delegate and then reuse that instance all the way along the class.

@apuntamb
Copy link
Contributor Author

apuntamb commented Oct 6, 2020

I think better practice would be using LoggerAdapter to wrap log calls that are already emitted by the system with request_id.

You mean something like logging.LoggerAdapter.log("Originating Request ID: %s" % request_id) inside the report_consumer or inside abstract, report and slice processors?

Especially it's relevant for processors: abstract, report and slice. I see that the request_id is available through report_or_slice variable in pre_delegate. I suppose we can wrap the logger instance with LoggerAdapter inside the pre_delegate and then reuse that instance all the way along the class.

In here https://github.com/quipucords/yupana/blob/master/yupana/processor/report_slice_processor.py#L98 ? By reusing it all the way do you mean wrapping it like above and using it in every method call?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants