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

log: consume TSS logging and feed into python logging module #164

Closed
wants to merge 1 commit into from

Conversation

joholl
Copy link
Contributor

@joholl joholl commented Jun 7, 2021

Remove the TSS logging to stderr. Instead, have the TSS log to a pipe. Read the pipe and feed the logging records into the python logging framework.

Example:

import logging

from test.TSS2_BaseTest import TpmSimulator

from tpm2_pytss import FAPI, FapiConfig
from tpm2_pytss.log import tss_loggers

# highlight using ANSI color codes
green = "\x1b[32m"
yellow = "\x1b[93m"
red = "\x1b[31"
blue = "\x1b[34m"
cyan = "\x1b[96m"
light_grey = "\x1b[37m"
reset = "\x1b[0m"

# setup logging
root_logger = logging.getLogger()
root_logger.setLevel(logging.NOTSET)
handler = logging.StreamHandler()
formatter = logging.Formatter(
    f"{light_grey}[%(levelname)s]{reset} {blue}%(pathname)s:%(lineno)d{reset} - {cyan}%(name)s {yellow}%(message)s{reset}",
    "%Y-%m-%d %H:%M:%S",
)
handler.setFormatter(formatter)
root_logger.addHandler(handler)

# set some TSS log levels as an example
for _module, logger in tss_loggers.items():
    logger.setLevel(logging.WARNING)
logging.getLogger("TSS.fapijson").setLevel(logging.DEBUG)

if __name__ == "__main__":
    tpm = TpmSimulator.getSimulator()
    tpm.start()

    with FapiConfig(
        temp_dirs=True, tcti=tpm.tcti_name_conf, ek_cert_less="yes"
    ) as fapi_config:
        with FAPI() as fapi:
            fapi.provision()

            fapi.create_key(path=f"/{fapi.config.profile_name}/HS/SRK/key_123")

            # provoke error logs
            fapi.create_key(
                path=f"/{fapi.config.profile_name}/HS/SRK/key_123", exists_ok=True
            )

    tpm.close()

Fixes #49.

@codecov
Copy link

codecov bot commented Jun 7, 2021

Codecov Report

Merging #164 (ec28137) into master (07a92e9) will decrease coverage by 0.76%.
The diff coverage is 53.24%.

❗ Current head ec28137 differs from pull request most recent head 9868644. Consider uploading reports for the commit 9868644 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #164      +/-   ##
==========================================
- Coverage   87.81%   87.04%   -0.77%     
==========================================
  Files          13       14       +1     
  Lines        3382     3459      +77     
  Branches      318      330      +12     
==========================================
+ Hits         2970     3011      +41     
- Misses        277      306      +29     
- Partials      135      142       +7     
Impacted Files Coverage Δ
tpm2_pytss/log.py 52.63% <52.63%> (ø)
tpm2_pytss/__init__.py 100.00% <100.00%> (ø)

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 07a92e9...9868644. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented Jun 7, 2021

This pull request introduces 1 alert when merging ec28137 into 07a92e9 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented Jun 7, 2021

This pull request introduces 1 alert when merging 9868644 into 07a92e9 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@williamcroberts
Copy link
Member

#49 was for logs needed within the Python bindings, I never really expected to grab the TSS log errors from stderr from the underlying C library.

It would be nicer if the C libraries had a setlog function where we could specify a callback into the Python code to decide what to do with the log over spinning up a Thread to consume stderr and filter.

@joholl
Copy link
Contributor Author

joholl commented Jun 7, 2021

Of course, why didn't I think about that? I agree, a C callback would be much more elegant. I'd really like to see that in the release!

Closing this for now. I'll work on this some more when I find the time.

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.

Feature Request: set TSS log level per logging module
2 participants