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

cth_log_redirect could support more logger configuration #8062

Open
eproxus opened this issue Jan 29, 2024 · 6 comments
Open

cth_log_redirect could support more logger configuration #8062

eproxus opened this issue Jan 29, 2024 · 6 comments
Assignees
Labels
enhancement priority:low team:PS Assigned to OTP team PS waiting waiting for changes/input from author

Comments

@eproxus
Copy link
Contributor

eproxus commented Jan 29, 2024

Is your feature request related to a problem? Please describe.
When using cth_log_redirect progress logs get enabled automatically (because of how logger works by default). In some cases, these can get very verbose and increase the size of test logs by megabytes.

The actual case is starting hackney with {ssl_options, tls_certificate_check:options(Host)} which will include all system certificates as binaries in the start arguments. This results in logging megabytes of binary data to the log every time Hackney starts.

Describe the solution you'd like
The module cth_log_redirect should copy additional logger configuration such as filters and filter_default so that one can use e.g. logger_filters:progress/2 as a filter for the test logs.

Describe alternatives you've considered
Completely replace our HTTP client implementation just to avoid blowing up the size of the test logs.

Additional context
Would most likely require minimal changes here:

{DefaultFormatter, DefaultLevel} =
case logger:get_handler_config(default) of
{ok, Default} ->
{maps:get(formatter, Default), maps:get(level, Default)};
_Else ->
{{?DEFAULT_FORMATTER,?DEFAULT_FORMAT_CONFIG},info}
end,
HandlerConfig = #{level => DefaultLevel, formatter => DefaultFormatter},

Would be happy to do a PR if there is agreement that this change is desirable.

@eproxus eproxus changed the title cth_log_redirect should suppor more logger configuration cth_log_redirect should support more logger configuration Jan 29, 2024
@eproxus eproxus changed the title cth_log_redirect should support more logger configuration cth_log_redirect could support more logger configuration Jan 29, 2024
@u3s u3s added the team:PS Assigned to OTP team PS label Jan 29, 2024
@u3s
Copy link
Contributor

u3s commented Jan 29, 2024

I think this sounds like good idea and matches to other initiatives aiming at taming CT output.
Maybe @garazdawi could also comment about preferred shape of such PR, since this is relates to both CT and logger ...

@u3s u3s self-assigned this Jan 29, 2024
@eproxus
Copy link
Contributor Author

eproxus commented Jan 29, 2024

I'd propose replacing the above snippet with the following change:

    HandlerConfig =
        case logger:get_handler_config(default) of
            {ok, Default} ->
                maps:with([level, formatter, filters, filter_default], Default);
            _Else ->
                #{
                    level => info,
                    formatter => {?DEFAULT_FORMATTER, ?DEFAULT_FORMAT_CONFIG}
                }
        end,

@u3s
Copy link
Contributor

u3s commented Jan 31, 2024

Please create a PR. I guess you intended to use maps:with/2 in your snippet ...

@eproxus
Copy link
Contributor Author

eproxus commented Jan 31, 2024

👍 Yes, you are correct (I've updated the suggestion snippet)

@u3s u3s added waiting waiting for changes/input from author priority:low labels Feb 12, 2024
@eproxus
Copy link
Contributor Author

eproxus commented Feb 29, 2024

Short status update: got stuck with the tests. I haven't figured out how they work exactly and how to add a new test for this case yet.

@u3s
Copy link
Contributor

u3s commented Feb 29, 2024

CT tests are sometimes tricky. Can you push PR without tests at this stage?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement priority:low team:PS Assigned to OTP team PS waiting waiting for changes/input from author
Projects
None yet
Development

No branches or pull requests

2 participants