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

DM-30977: Add python log handler to store LogRecord as they come in #546

Merged
merged 27 commits into from Jul 26, 2021

Conversation

timj
Copy link
Member

@timj timj commented Jul 7, 2021

These records can then be serialized in butler datastore. This is assuming that lsst/log#55 is merged which will also need changes in daf_butler for initializing logging such that we no longer forward everything to lsst.log. Will need changes to ctrl_mpexec as well.

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes

@andy-slac
Copy link
Contributor

DM-30996 will most likely add MDC attribute to LogRecord which will be a dict-like object (or maybe SimpleNamespace but it should support dict methods too). I think we want to save its contents too in some portable form.

@timj
Copy link
Member Author

timj commented Jul 8, 2021

The new ButlerLogRecord can be as flexible as we want it to be. I can definitely make it scan the logging.LogRecord for additional dict-like objects and add them in to ButlerLogRecord.extra or add them into the expanded message.

Copy link
Contributor

@andy-slac andy-slac left a comment

Choose a reason for hiding this comment

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

Looks good, few minor comments.

python/lsst/daf/butler/cli/cliLog.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/cli/opt/options.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/core/logging.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/core/logging.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/core/logging.py Show resolved Hide resolved
python/lsst/daf/butler/formatters/logs.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/formatters/logs.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/formatters/logs.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/formatters/logs.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/formatters/logs.py Outdated Show resolved Hide resolved
These records can then be serialized in butler datastore.
timj added 11 commits July 23, 2021 08:52
There is a question as to whether this should be attached to
the main message, kept as a separate string, or stored
separately as a list of strings.

For now it is stored as a separate single string.
This can be used to directly stream LogRecords in JSON format.
Returning a new ButlerLogRecords instance.
One record per line rather than a single JSON model per file.
This makes it available to test code and to other users without
having to limit reading to butler formatter.
Also unify the format detection code rather than repeating it
in stream and byte handling.
@timj timj force-pushed the tickets/DM-30977 branch 3 times, most recently from d198762 to 772330e Compare July 24, 2021 01:27
When splitting a potentially enormous string we want to avoid
creating a list with potentially millions of elements.
@timj timj force-pushed the tickets/DM-30977 branch 2 times, most recently from f45f25f to 877e3f9 Compare July 25, 2021 00:56
timj added 4 commits July 25, 2021 18:24
Only make ButlerLogRecords public by default since that is
the type stored in datastore.

Also rename JsonFormatter to JsonLogFormatter to avoid
confusion with the datastore formatter of the same name.
This temporarily changes the MDC for logging.
@timj timj merged commit 95d096a into master Jul 26, 2021
@timj timj deleted the tickets/DM-30977 branch July 26, 2021 22:40
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.

None yet

2 participants