Fix backoff / network client interfaces to implement Stringer #7882
Conversation
|
Good finding @andrewvc, it appears to be the stringer interface but actually, the I did a quick look at the other output, console/kafka and they implement the stringer interface. |
libbeat/outputs/redis/backoff.go
Outdated
| @@ -112,3 +113,7 @@ func (b *backoffClient) resetFail() { | |||
| b.reason = failNone | |||
| b.backoff.Reset() | |||
| } | |||
|
|
|||
| func (b *backoffClient) String() string { | |||
| return fmt.Sprintf("BackoffClient: client: %s, reason: %v, backoff: %s", b.client, b.reason, b.backoff) | |||
There was a problem hiding this comment.
The backoffClient here is specific to redis. Let's just return b.client.String(). The string is used to identify a client type + endpoint on error. The actual error message message contains the reason. Printing additional state would be more helpful in form of debug messages when the next connection attempt starts/fails.
There was a problem hiding this comment.
@urso as a go noob I'm a little confused by this. In most languages the canonical String()/toString() function is generally for debugging. Usually if you need a specific formatting of an object you have that logic at a higher level.
Are we sure we want to make String()'s implementation tightly coupled to this specific error message?
It seems to me like we should just not have made this stringer-like interface, and had the print function traverse the object graph itself and format things the specific way it would like them to be formatted.
There was a problem hiding this comment.
We mostly use String to print and identify an object. In go we also have alternative formatting supporting using the fmt package. When using any fmt.X methods, it checks if your type implements fmt.Formatter. This adds additional formatting capabilities by passing the format pattern to the Formatter. When using fmt.XPrintf, one normally uses:
%v: default representation%+v: which asks for more details%#v: which is interpreted by fmt and will add the struct name + print each field by name.
If fmt.Formatter is not implemented, %v will fallback to String() string (fmt.Stringer interface), and %#v will fallback to GoString() string (fmt.GoStringer interface).
See package documentation of fmt.
andrewvc
force-pushed
the
fix-backoff-interface
branch
from
fedb3c7
to
5eb4fa8
Compare
|
I've done the quick fix @urso recommended in the outdated comment. I think we should merge sooner rather than later since this is breaking all builds, but the usage of specific |
…elastic#7882) (cherry picked from commit 551b2a9)
…elastic#7882) (cherry picked from commit 551b2a9)
Merging https://github.com/elastic/beats/pull/6404/files
Caused 512611a to break.
I assume the earlier commit wasn't rebased before merge. This fixes up the interface for the Redis
backoff client to implement Stringer