implement library for tracking net.Listener connections#42
implement library for tracking net.Listener connections#42groob wants to merge 3 commits intokolide:mainfrom
Conversation
|
|
||
| func (tl *trackingListener) Accept() (net.Conn, error) { | ||
| conn, err := tl.Listener.Accept() | ||
| stats.RecordWithTags(context.TODO(), []tag.Mutator{tag.Upsert(tl.stats.TagSuccess, fmt.Sprintf("%v", err == nil))}, tl.stats.ListenerAccepted.M(1)) |
There was a problem hiding this comment.
Perhaps you might want to have two tag keys
status --> whose values can either be "OK" and "ERROR"
error --> whose value is set only if key "status" is set and its value is the full error encountered.
This will aid a lot with grouping errors, creating alerts on various errors and determination of error/success rates.
There was a problem hiding this comment.
Interesting. having arbitrary strings end up as tags is a little icky, but I suspect there's only a small number of them you'd possibly encounter from the stdlib.
I can see how this would be very useful, although my instinct would be to check for the actual error strings in logs/traces.
There was a problem hiding this comment.
I also don't like arbitrary strings as tags (if strings have values like port number, ip address, stacks, etc the cardinality can blow up). A compromise is to add it here but not add it by default in the respective view and have some way (either code or doc) to add it explicitly if one wants it.
I would recommend pre-defining two mutator slices and select the one to be used according to the value of err instead of creating them on every call.
|
|
||
| func (tl *trackingListener) Accept() (net.Conn, error) { | ||
| conn, err := tl.Listener.Accept() | ||
| stats.RecordWithTags(context.TODO(), []tag.Mutator{tag.Upsert(tl.stats.TagSuccess, fmt.Sprintf("%v", err == nil))}, tl.stats.ListenerAccepted.M(1)) |
There was a problem hiding this comment.
I also don't like arbitrary strings as tags (if strings have values like port number, ip address, stacks, etc the cardinality can blow up). A compromise is to add it here but not add it by default in the respective view and have some way (either code or doc) to add it explicitly if one wants it.
I would recommend pre-defining two mutator slices and select the one to be used according to the value of err instead of creating them on every call.
| stats *Stats | ||
| } | ||
|
|
||
| func NewInstrumentedListener(lis net.Listener) (net.Listener, *Stats) { |
There was a problem hiding this comment.
Instead of using context.TODO() what about passing a ctx on the contructor and hold to it? This way caller can even add their own tags and modiiy the views accordingly.
5 participants
No description provided.