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

Don't output multi line responses in INFO level log #420

Closed
robklg opened this issue Jun 22, 2022 · 0 comments · Fixed by #461
Closed

Don't output multi line responses in INFO level log #420

robklg opened this issue Jun 22, 2022 · 0 comments · Fixed by #461

Comments

@robklg
Copy link
Contributor

robklg commented Jun 22, 2022

We currently output the multi line messages (such as the output of STAT) to the logs when in INFO mode. We should handle this differently. Perhaps only do this in DEBUG mode? Along with output length restrictions.

INFO level log lines should still be there but not contain the contents.

robklg pushed a commit that referenced this issue May 28, 2023
Fixes #420 (omit multi line output in the logs)
Fixes part one of #81 (although was already handled mostly by Werner)

And lots of other things I came across:

Logging:
- Improved mappings from storage errors to FTP errors
- Moved many log messages from INFO to DEBUG level
- Introduced more user friendly log messages at INFO level
- Improved the log messages for all commands, making sure the log entries are clear and 'actionable'
- Introduced nice additional information on transfers (duration, size and transfer speed)
- Added some error logs when there were none

Metrics:
- Added a specific counter metric for data transfer commands, with a verdict on whether it's a server or client error
- Added in-flight count on transferred bytes (also labeled by command) so that we can calculate average transfer speed

Some bug fixes:
- LIST and NLST weren't returning replies correctly
- RETR was sending no Reply on the Control channel when there was no Data connection, causing the client to hang
robklg added a commit that referenced this issue May 28, 2023
Particularly the STAT <path> command can sometimes pollute the log.
We don't want to dump directory listings into the log stream.

In this commit I've totally stripped the multiline output (only indicating the number lines for information). I didn't
really see the need for outputting any of the other multilines either.
robklg added a commit that referenced this issue May 31, 2023
Particularly the STAT <path> command can sometimes pollute the log.
We don't want to dump directory listings into the log stream.

In this commit I've totally stripped the multiline output (only indicating the number lines for information). I didn't
really see the need for outputting any of the other multilines either.
robklg added a commit that referenced this issue May 31, 2023
Bug fixes:
- #420 (omit multi line output in the logs)
- Part one of #81 (was already handled mostly by Werner)
- LIST and NLST weren't returning replies correctly
- RETR was sending no Reply on the Control channel when there was no Data connection, causing the client to hang
- Bug with REST (restart) command not working correctly

Logging improvements:
- Improved mappings from storage errors to FTP errors
- Moved many log messages from INFO to DEBUG level
- Introduced more user friendly log messages at INFO level
- Improved the log messages for all commands, making sure the log
entries are clear and 'actionable'
- Introduced nice additional information on transfers (duration, size
and transfer speed)
- Added some error logs when there were none

Metrics additions:
- Added a specific counter metric for data transfers (stor, retr, list), recording success count and errors (with a label attributing it to a cause: server, client, client interruption, network, permissions), for better reliability insight
- Added in-flight count on transferred bytes (also labeled by command) so that we can calculate average transfer speed
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 a pull request may close this issue.

1 participant