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

kms: add support for fetching server logs #19

Merged
merged 2 commits into from
Jul 17, 2024
Merged

kms: add support for fetching server logs #19

merged 2 commits into from
Jul 17, 2024

Conversation

aead
Copy link
Member

@aead aead commented Jul 15, 2024

This commit adds support for fetching server logs
via the /v1/log API.

The log API returns a stream of log records. Each
record is a protobuf (or JSON) encoded message
containing the log leve, message, timestamp and
other information about a log event.

Clients can use a LogRequest to filter based on
log level, message, IP address etc.

@aead aead force-pushed the logging branch 3 times, most recently from c4489ae to e53ac81 Compare July 15, 2024 14:41
This commit adds support for fetching server logs
via the `/v1/log` API.

The log API returns a stream of log records. Each
record is a protobuf (or JSON) encoded message
containing the log leve, message, timestamp and
other information about a log event.

Clients can use a `LogRequest` to filter based on
log level, message, IP address etc.

Signed-off-by: Andreas Auernhammer <[email protected]>
Copy link

@ramondeklein ramondeklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this Go client is meant for minkms, but I couldn't find any matching protobuf specifications in minkms.

I wonder why we are using custom serialization formats, but that's probably a choice that was made when the server-side was created.

kms/client-examples_test.go Show resolved Hide resolved
kms/client.go Show resolved Hide resolved
kms/client.go Show resolved Hide resolved
Signed-off-by: Andreas Auernhammer <[email protected]>
@aead
Copy link
Member Author

aead commented Jul 17, 2024

I guess this Go client is meant for minkms, but I couldn't find any matching protobuf specifications in minkms.
I wonder why we are using custom serialization formats, but that's probably a choice that was made when the server-side > was created.

The protobuf specification is in github.com/minio/kms-go/kms/protobuf. The server imports this directly. There is no need to specify this twice (and potentially get out-of-sync).

Are you referring to the lenght-prefixed protobuf messages?

If you want to write multiple messages to a single file or stream, it is up to you to keep track of where one message ends and the next begins. The Protocol Buffer wire format is not self-delimiting, so protocol buffer parsers cannot determine where a message ends on their own. The easiest way to solve this problem is to write the size of each message before you write the message itself. When you read the messages back in, you read the size, then read the bytes into a separate buffer, then parse from that buffer.

Ref: https://protobuf.dev/programming-guides/techniques/

@aead
Copy link
Member Author

aead commented Jul 17, 2024

For some context @ramondeklein
The KMS server supports log streaming in the sense that a client can subscribe to the KMS logging system and receive a continous stream of logs. It's not an API that receives "just" the last N log messages. It's an API that allows direct monitoring.

@ramondeklein
Copy link

The protobuf specification is in github.com/minio/kms-go/kms/protobuf. The server imports this directly. There is no need to specify this twice (and potentially get out-of-sync).

I guess I'm used to specify the specification in the server and let the client reference that one. Agree that you shouldn't specify it twice. Fortunately, Go makes it rather easy to reference code from another repo. Try that with Java or .NET and not reach Git submodule hell...

@aead aead merged commit 174e995 into main Jul 17, 2024
5 checks passed
@aead aead deleted the logging branch July 17, 2024 09:52
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 this pull request may close these issues.

2 participants