-
Notifications
You must be signed in to change notification settings - Fork 17
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
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) { | ||
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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps you might want to have two tag keys 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. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
open := atomic.AddInt64(&tl.stats.openConnections, 1) | ||
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() | ||
open := atomic.AddInt64(&sc.stats.openConnections, -1) | ||
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 | ||
|
||
ListenerAcceptedView *view.View | ||
LifetimeClosedConnectionsView *view.View | ||
OpenConnectionsView *view.View | ||
} | ||
|
||
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.ListenerAcceptedView = viewFromStat(s.ListenerAccepted, tags, view.Count()) | ||
s.OpenConnectionsView = viewFromStat(s.OpenConnections, nil, view.LastValue()) | ||
s.LifetimeClosedConnectionsView = 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, | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
package ntrack | ||
|
||
import ( | ||
"fmt" | ||
"net" | ||
"net/http" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
"go.opencensus.io/stats/view" | ||
) | ||
|
||
func TestListener(t *testing.T) { | ||
var tests = []struct { | ||
viewName string | ||
disableKeepalive bool | ||
expectedValue int64 | ||
}{ | ||
{ | ||
viewName: "ntrack/listener/accepts", | ||
disableKeepalive: true, | ||
expectedValue: 5, | ||
}, | ||
{ | ||
viewName: "ntrack/listener/accepts", | ||
disableKeepalive: false, | ||
expectedValue: 1, | ||
}, | ||
{ | ||
viewName: "ntrack/listener/closed", | ||
disableKeepalive: true, | ||
expectedValue: 5, | ||
}, | ||
{ | ||
viewName: "ntrack/listener/open", | ||
disableKeepalive: true, | ||
expectedValue: 0, | ||
}, | ||
{ | ||
viewName: "ntrack/listener/open", | ||
disableKeepalive: false, | ||
expectedValue: 1, | ||
}, | ||
} | ||
|
||
for _, tt := range tests { | ||
t.Run(tt.viewName, func(t *testing.T) { | ||
lis, err := net.Listen("tcp", "127.0.0.1:0") | ||
require.NoError(t, err) | ||
|
||
ilis, stats := NewInstrumentedListener(lis) | ||
registerViewByName(t, tt.viewName, stats, false) | ||
|
||
testClientConnections(t, ilis, tt.disableKeepalive) | ||
|
||
rows, err := view.RetrieveData(tt.viewName) | ||
require.NoError(t, err) | ||
|
||
switch data := rows[0].Data.(type) { | ||
case *view.CountData: | ||
assert.Equal(t, tt.expectedValue, data.Value) | ||
case *view.LastValueData: | ||
assert.Equal(t, float64(tt.expectedValue), data.Value) | ||
} | ||
registerViewByName(t, tt.viewName, stats, true) | ||
}) | ||
} | ||
} | ||
|
||
func registerViewByName(t *testing.T, name string, stats *Stats, unregister bool) { | ||
var v *view.View | ||
switch name { | ||
case "ntrack/listener/accepts": | ||
v = stats.ListenerAcceptedView | ||
case "ntrack/listener/open": | ||
v = stats.OpenConnectionsView | ||
case "ntrack/listener/closed": | ||
v = stats.LifetimeClosedConnectionsView | ||
} | ||
if unregister { | ||
view.Unregister(v) | ||
} else { | ||
view.Register(v) | ||
} | ||
} | ||
|
||
func testClientConnections(t *testing.T, lis net.Listener, disableKeepalive bool) { | ||
t.Helper() | ||
|
||
srv := &http.Server{ | ||
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
w.WriteHeader(http.StatusOK) | ||
}), | ||
} | ||
|
||
go func() { | ||
if err := srv.Serve(lis); err != nil { | ||
t.Fatal(err) | ||
} | ||
}() | ||
|
||
tr := &http.Transport{DisableKeepAlives: disableKeepalive} | ||
client := &http.Client{Transport: tr} | ||
|
||
requestCount := 5 | ||
for i := 0; i < requestCount; i++ { | ||
resp, err := client.Get(fmt.Sprintf("http://%s", lis.Addr())) | ||
require.NoError(t, err) | ||
resp.Body.Close() | ||
} | ||
|
||
} |
There was a problem hiding this comment.
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.