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
Better user tracking in the logs #38
Comments
thanks for the suggestions, I'll try to improve the logs
ok
I'm a bit busy now and maybe I'm missing something, can you better explain with an example what do you would like to see here? If we add the
ok, I'll try to add the connection id where applicable
ok
|
@drakkan I think you got what I meant: the connection_id is relevant in every logline, the exact username and other info (public key info eg.) only when the user authenticates. What I meant is that I don't know the sftp protocol, so not sure if the user authenticates for every request, or ... But your suggestion sounds perfect. |
ok thanks, for your info the user authenticate only once, so for example if you change the password or the public keys the change will be valid only for the new logged in users. Initially I wanted to disconnect the logged in users if their auth info were updated using the REST API and or update their connection info (such as quota ecc..) but this is not so easy to do the right way and other servers apply the new info only for new logins too |
I think I addressed b, c & d in PR's now; I may look at a tomorrow, unless you beat me to it |
However, I'm thinking of also including the username everywhere as a separate field too ... Just for easy grepping. What do you think? |
Basically we are adding more context to the generic loggers and their are becoming more similar to specialized loggers such as https://github.com/drakkan/sftpgo/blob/master/logger/logger.go#L102 I initially added these specialized loggers to allow to extract/parse the more relevant info in an easy way, while the generic loggers were used to log info useful to debug the daemon operations but not so useful to generate/extract a report of the user's activities. The same concept apply to the HTTP logs: https://github.com/drakkan/sftpgo/blob/master/logger/request_logger.go#L56 Maybe we can add more context to important logs, for example the error ones adding a new log category or adding additional fields to the exisisting ones. But maybe we are creating a monster :) |
I was thinking of adding a pointer to the connection (instead of the connection id), and if the pointer is not nil, adding some relevant info to the log lines. |
I'm not sure that this will work, the logger package is imported everywhere, if you import the sftpd package inside the logger you will probaly introduce a circular dependency |
If you want an IT guys's opinion who work daily with this use case, Include username everywhere is useless because this Connection_ID does the job. |
@drakkan you are right:
|
@binou-31 the connection id is required for granular tracing of connections in my opinion, but the username is useful for simple greps without having to do extra lookups... that's a sysadmin's opinion ;) but I'd be happy with the connection id and a log entry stating the user info (which is mostly ready) |
I think I implemented (a) too now (see PR #43) |
is there still something missing here? |
I don't think anything is missing. I'll close this then. |
I have a few thoughts/requests, I can make separate issues if you think that would be better.
a) It would be great to be able to identify connecting users from the logs by the fingerprint and the comment (the last part) of the public key. Now we only see the user name.
b) I only see the user name when the user does actions. Is the user not authenticated earlier?
c) I think the logs should use the
connection_id
to be able to trace connections/actions.d) Somtimes paths are quoted in the logs, sometimes not; this should obviously be consistent :-)
"requested list/stat" entries are not quoted
"fileread requested" is quoted
What I see now:
What I expect to see (more or less):
The text was updated successfully, but these errors were encountered: