Adding RPC log plugin: catch each rpc request and send log into GCS b… - #8
Adding RPC log plugin: catch each rpc request and send log into GCS b…#8olegfomenko wants to merge 8 commits into
Conversation
…ucket. WIP Co-authored-by: Mykhailo Khotian <khotyanmisha@gmail.com> Co-authored-by: Oleg Fomenko <oleg.fomenko2002@gmail.com>
|
TODO: from rune metadata get a requester info |
|
Credits to @nepet for the idea and reference |
Co-authored-by: Oleg Fomenko <oleg.fomenko2002@gmail.com>
Co-authored-by: Oleg Fomenko <oleg.fomenko2002@gmail.com>
Nazarevsky
left a comment
Nazarevsky
left a comment
There was a problem hiding this comment.
That's already a lot of honest work here, Oleg! I totally understand that's the current implementation is sort of 'in draft', however, I left you a couple of suggestions aimed for the inner logic and possible (maybe not in the current PR) improvements that users might benefit from.
|
|
||
| async fn upload_rpc_log(client: &Storage, bucket: &str, log: &RpcLog<'_>) -> Result<String> { | ||
| let timestamp = OffsetDateTime::now_utc(); | ||
| let id = Uuid::new_v4(); |
There was a problem hiding this comment.
I think we may cosider using UUIDv6 here since it supports time ordering. That way file names will explicitly tell about the log order.
There was a problem hiding this comment.
Is it really needed? We already have a timestamp prefix for each file. The use of a UUID here is just additional protection to ensure that, with high probability, two files with the same name can't exist.
| } | ||
| }; | ||
|
|
||
| upload_rpc_log( |
There was a problem hiding this comment.
So, right now we're producing a log on an every RPC call. That means that on every command a plugin should communicate GCP in order to send log data. It might be ok in case of the node won't be queried with RPCs too much but it might be pretty nasty in big load periods.
Let's consider the other approach here. What about instead of querying GCS on an every call we have a local file. The file has append only attributes set up, so it is not possible to modify data in any way unless to append some more. On an every RPC call the hook appends a new log line into the file. Every 5 minutes the file gets rotated - it is pushed to GCS, new file is created. If no logs are produced within 5 minutes - no file is pushed.
The '5 min' constant is obviously configured here
There was a problem hiding this comment.
We are ok with pushing logs into GCS each time for now.
| } | ||
| }; | ||
|
|
||
| upload_rpc_log( |
There was a problem hiding this comment.
Also, I think that we may allow a node operator to choose a list of RPC call for being logged in that way (or exclude some). The thing is that some of calls are not really crucial to be logged. E.g. observational calls like getinfo.
There was a problem hiding this comment.
Its actually a great idea!
There was a problem hiding this comment.
Added. By default, it will log only checkrune, which suits our use case - log mostly authorized methods
| async fn upload_rpc_log(client: &Storage, bucket: &str, log: &RpcLog<'_>) -> Result<String> { | ||
| let timestamp = OffsetDateTime::now_utc(); | ||
| let id = Uuid::new_v4(); | ||
| let object_name = format!("rpc/{}-{id}.json", timestamp.format(&Rfc3339)?,); |
There was a problem hiding this comment.
Hm, how about using jsonl instead? This file extension allows parsing json data line by line that might be helpful in case of really big files. Yes, currently we have one file for each log, but that's something that we can consider in case of using the 'rotation' approach that was described a couple of comments up from here
There was a problem hiding this comment.
As I said above, I don't want to overcomplicate the plugin at this stage, so saving each log to the bucket is ok for now
There was a problem hiding this comment.
Actually, you can create an issue with this proposal, can be a good starting point for future improvements
…runes in rpc call params and tries to parse it. Co-authored-by: Oleg Fomenko <oleg.fomenko2002@gmail.com>
# Conflicts: # Cargo.lock
Co-authored-by: Oleg Fomenko <oleg.fomenko2002@gmail.com>
…g. By default - only checkrune rpc command will be logged. Co-authored-by: Oleg Fomenko <oleg.fomenko2002@gmail.com>
…ing integration tests. Co-authored-by: Oleg Fomenko <oleg.fomenko2002@gmail.com>
2 participants
Adding RPC log plugin: it catches each RPC request and sends the log to the GCS bucket