Numerous improvements on logging and metrics #461
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I will break this up into multiple commits and do a thorough check myself, so no need to review yet. |
robklg
force-pushed
the
robby/logging-and-metrics
branch
from
f4f87dd
to
77a5770
Compare
robklg
force-pushed
the
robby/logging-and-metrics
branch
from
dc2e846
to
fae5e36
Compare
hannesdejager
approved these changes
May 31, 2023
When RETR can't find the data connection, it must Reply or the client will get stuck.
Part of effort to make INFO logging more useful. - Introduced new log lines, oriented to the end user - Include the path where appropriate - Added a command prefix to most log messages, to clarify the context
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.
When the client couldn't connect to GCS over HTTP, it would return a permanent error. We should let the client retry, so now we return a transient error instead.
We were not correctly mapping a bunch of errors. Most of these could lead to returning retryable LocalError's to the client, while they are not retryable.
Added/updated metrics: - Total session count (note: counter instead of the existing gauge) - Added total transfers (ftp_transferred_total), it also collapses all errors to a client-error, server-error or permission-error for use in Service Level measurements - Added sent/received bytes (ftp_sent_bytes, ftp_received_bytes) for all transfers (list, retr, stor) updated 'in flight' for average speed calculation of file and directory listing transfers. (This is implemented with a custom MeasuringWriter) - Clarified description that backend read byte and file count only count the successful ones Logging changes: - Changed ControlChanMiddleware to log events and replies at DEBUG level (better readable INFO messages replace it) - Log human readable duration, bytes copied and transfer speed for each transfer command (retr, stor and the list commands)
This change makes sure the start position is copied from the session at the moment the RETR or STOR command is received.
robklg
force-pushed
the
robby/logging-and-metrics
branch
from
f5edca0
to
8158e1b
Compare
robklg
force-pushed
the
robby/logging-and-metrics
branch
from
e7f800d
to
82a16cb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
Metrics:
Some bug fixes: