slash-init commented on PR #3464:
URL: https://github.com/apache/iggy/pull/3464#issuecomment-4718562824
> nit: Just a minor stype recommendation For example:
>
> ```go
> c.logger.Info("Retrying to connect to server...", "retry_count", n+1,
"max_retries", attempts, "error", err)
> ```
>
> can be changed to
>
> ```go
> c.logger.Info("Retrying to connect to server...",
> slog.Uint64("retry_count", uint64(n+1)),
> slog.Uint64("max_retries", uint64(attempts)),
> slog.Any("error", err))
> ```
>
> For detailed reason, check this reference:
https://www.dash0.com/guides/logging-in-go-with-slog#adding-contextual-attributes-to-your-logs
>
> To be honest I think the previous approach may be more clear. But if we
have a lot of KVs, we probably need this strong but ugly approach.
>
> I don't know why Go doesn't provide function like slog.Uint() for
convenience. 🤦♂️
Thanks. I went with slog attributes.
Also, one thing I noticed is that Rust's SDK has a connection state guard in
the `send_raw()` function before attempting any network operations. The Go SDK
doesn't have one in `sendWireAndFetchResponse()`. Unless I'm missing something,
it looks like calling an operation before `Connect()` succeeds could eventually
reach `c.conn.Write(...)` while `c.conn` is still nil and result in a panic
rather than `ErrNotConnected`.
Do we need one here too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
