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

Add a WithKeyFormatter option #16

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

MarkusFreitag
Copy link
Contributor

@MarkusFreitag MarkusFreitag commented Jan 22, 2025

Hi @rodaine ,
today I stumbled across this library and would like to propose another protoslog.Option.
I have the need to generate the key for a field based on its type. Please give me some feedback whether you would accept this addition to your library.

Lets assume there are the following proto messages.

message Event {
  google.protobuf.Any payload = 1;
}
message PayloadA {
  string id = 1;
}
message PayloadB {
  uint64 id = 1;
}

Logging an incoming event using your library will produces the following log entries. For simplicity log attributes like time and level are removed.

{"msg":"incoming event","event":{"payload":{"@type":"type.googleapis.com/protobuf.PayloadA","id":"fc2ea33d-59bf-4902-8b55-2ac334f61704"}}}
{"msg":"incoming event","event":{"payload":{"@type":"type.googleapis.com/protobuf.PayloadB","id":123}}}

These logs are collected and pushed into a logstorage. When it parses the log entries, having two equal keys (event.id from both payloads) with different types will result in an error. Using the added option, I can provide a function that will extend the key with a suffix based on the actual payloads type. Afterwards the log entries would look like this:

{"msg":"incoming event","event":{"payload[protobuf/PayloadA]":{"@type":"type.googleapis.com/protobuf.PayloadA","id":"fc2ea33d-59bf-4902-8b55-2ac334f61704"}}}
{"msg":"incoming event","event":{"payload[protobuf/PayloadB]":{"@type":"type.googleapis.com/protobuf.PayloadB","id":123}}}

If you consider to accept this PR, I will add some tests using the above mentioned example.

@rodaine
Copy link
Owner

rodaine commented Jan 22, 2025

Hi, @MarkusFreitag! Out of curiosity, which log storage utility do you use?

@rodaine
Copy link
Owner

rodaine commented Jan 22, 2025

Also, this PR is great, just missing tests!

Currently the key used when converting a message field into a slog.Attr
defaults to the fields name. The function provided via this option can
alter the key depending on the field and its value.
@MarkusFreitag
Copy link
Contributor Author

Hi, @MarkusFreitag! Out of curiosity, which log storage utility do you use?

The logs are pushed into OpenSearch.

Also, this PR is great, just missing tests!

I've added some unit-tests, please led me know whether anything else is needed.

@rodaine
Copy link
Owner

rodaine commented Jan 24, 2025

Thanks, @MarkusFreitag!

@rodaine rodaine merged commit 056c307 into rodaine:main Jan 24, 2025
2 checks passed
@MarkusFreitag MarkusFreitag deleted the key-formatter-option branch January 27, 2025 08:53
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