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

Make logging more customization friendly #2320

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

Conversation

cmarqu
Copy link
Contributor

@cmarqu cmarqu commented Jan 3, 2021

The added overhead for using function calls to format the single
log line elements is compensated by introducing LRU caching.

Add a custom logger test that adds visual brackets to sim timestamps.

@cmarqu cmarqu added type:feature new or enhanced functionality status:needs-newsfragment Needs a towncrier newsfragment for the changelog labels Jan 3, 2021
@codecov
Copy link

codecov bot commented Jan 3, 2021

Codecov Report

Merging #2320 (a8dd531) into master (3d690d3) will increase coverage by 0.17%.
The diff coverage is 94.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2320      +/-   ##
==========================================
+ Coverage   73.98%   74.15%   +0.17%     
==========================================
  Files          36       36              
  Lines        6492     6505      +13     
  Branches      557      556       -1     
==========================================
+ Hits         4803     4824      +21     
+ Misses       1501     1495       -6     
+ Partials      188      186       -2     
Impacted Files Coverage Δ
cocotb/log.py 88.70% <94.87%> (+8.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d690d3...a8dd531. Read the comment docs.

@cmarqu cmarqu force-pushed the log-improvements branch 13 times, most recently from 1a4e0b9 to e10d728 Compare January 3, 2021 21:05
@cmarqu cmarqu removed the status:needs-newsfragment Needs a towncrier newsfragment for the changelog label Jan 3, 2021
@cmarqu cmarqu force-pushed the log-improvements branch 3 times, most recently from c74be19 to e70da93 Compare January 3, 2021 21:33
@themperek
Copy link
Contributor

I am not sure if this is the right place to comment. But out default logging, especially (multiple lines per log message) makes it harder to handle in any kind of parsing/post processing.

@cmarqu
Copy link
Contributor Author

cmarqu commented Jan 5, 2021

Note that this PR doesn't change the default log format at all.

This PR would actually make it easier to set up a (additional) logger that is parser-friendly.

@cmarqu
Copy link
Contributor Author

cmarqu commented Jan 18, 2021

I suppose it could be said that the new functions for sub-elements of the log line should be marked private so that we do not commit to them being stable until formally deprecated.

if len(string) > chars:
return ".." + string[(chars - 2) * -1:]
return "".join(["..", string[(chars - 2) * -1:]])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The truncation should be more configurable, for instance, we could use the ellipsis … to save one character.

in the log module, introduce new functions
format_sim_time(), format_recordname(), format_filename(),
format_funcname(), format_exc_msg(), format_build_line().

The added overhead for using these function calls to format the single
log line elements is (over-)compensated by introducing LRU caching.

Add a custom_logger example that shows how to add visual "brackets" to
simulator timestamps.
Copy link
Member

@ktbarrett ktbarrett left a comment

Choose a reason for hiding this comment

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

The point of this seems to be adding a couple hooks to tweak the existing log formatter, but is incapable of adding or deleting information, or changing order of the elements of a log line. cocotb's code is MIT licensed, any user could copy-paste the formatter and change it in more ways that this change provides. I don't see the value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature new or enhanced functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants