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

implement library for tracking net.Listener connections #42

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 91 additions & 0 deletions ntrack/listener.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package ntrack

import (
"context"
"fmt"
"net"
"sync/atomic"

"github.com/pkg/errors"
"go.opencensus.io/stats"
"go.opencensus.io/stats/view"
"go.opencensus.io/tag"
)

type trackingListener struct {
net.Listener
stats *Stats
}

func NewInstrumentedListener(lis net.Listener) (net.Listener, *Stats) {

Choose a reason for hiding this comment

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

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.

listenerStats := &Stats{}
listenerStats.init()

return &trackingListener{
Listener: lis,
stats: listenerStats,
}, listenerStats
}

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))
Copy link

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Choose a reason for hiding this comment

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

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.

if err != nil {
return nil, errors.Wrap(err, "accept from base listener")
}

groob marked this conversation as resolved.
Show resolved Hide resolved
atomic.AddInt64(&tl.stats.openConnections, 1)
groob marked this conversation as resolved.
Show resolved Hide resolved
open := atomic.LoadInt64(&tl.stats.openConnections)
stats.Record(context.TODO(), tl.stats.OpenConnections.M(open))
return &serverConn{Conn: conn, stats: tl.stats}, nil
}

type serverConn struct {
net.Conn
stats *Stats
}

func (sc *serverConn) Close() error {
err := sc.Conn.Close()
atomic.AddInt64(&sc.stats.openConnections, -1)
open := atomic.LoadInt64(&sc.stats.openConnections)
stats.Record(context.TODO(),
sc.stats.OpenConnections.M(open),
sc.stats.LifetimeClosedConnections.M(1),
)
return errors.Wrap(err, "close server conn")
}

type Stats struct {
ListenerAccepted *stats.Int64Measure
LifetimeClosedConnections *stats.Int64Measure
OpenConnections *stats.Int64Measure
openConnections int64

TagSuccess tag.Key

views []*view.View
groob marked this conversation as resolved.
Show resolved Hide resolved
groob marked this conversation as resolved.
Show resolved Hide resolved
}

func (s *Stats) init() {
s.ListenerAccepted = stats.Int64("ntrack/listener/accepts", "The number of Accept calls on the net.Listener", stats.UnitDimensionless)
s.LifetimeClosedConnections = stats.Int64("ntrack/listener/closed", "The number of Close calls on the net.Listener", stats.UnitDimensionless)
s.OpenConnections = stats.Int64("ntrack/listener/open", "The number of Open connections from the net.Listener", stats.UnitDimensionless)

s.TagSuccess, _ = tag.NewKey("success")

tags := []tag.Key{s.TagSuccess}
s.views = append(s.views, viewFromStat(s.ListenerAccepted, tags, view.Count()))
s.views = append(s.views, viewFromStat(s.OpenConnections, nil, view.LastValue()))
s.views = append(s.views, viewFromStat(s.LifetimeClosedConnections, nil, view.Count()))
}

func viewFromStat(ss *stats.Int64Measure, tags []tag.Key, agg *view.Aggregation) *view.View {
return &view.View{
Name: ss.Name(),
Measure: ss,
Description: ss.Description(),
TagKeys: tags,
Aggregation: agg,
}
}