Skip to content

Commit

Permalink
bs: add keepalive metrics
Browse files Browse the repository at this point in the history 
* remove goconvey from keepalive tests

Contributes #3104
  • Loading branch information
@karampok
karampok committed Sep 18, 2019
1 parent fba6b8c commit fbb7782
Show file tree
Hide file tree
Showing 10 changed files with 244 additions and 137 deletions.
1 change: 0 additions & 1 deletion go/beacon_srv/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ go_library(
"//go/beacon_srv/internal/config:go_default_library",
"//go/beacon_srv/internal/ifstate:go_default_library",
"//go/beacon_srv/internal/keepalive:go_default_library",
"//go/beacon_srv/internal/metrics:go_default_library",
"//go/beacon_srv/internal/onehop:go_default_library",
"//go/beacon_srv/internal/revocation:go_default_library",
"//go/lib/addr:go_default_library",
Expand Down
4 changes: 3 additions & 1 deletion go/beacon_srv/internal/keepalive/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ go_library(
visibility = ["//go/beacon_srv:__subpackages__"],
deps = [
"//go/beacon_srv/internal/ifstate:go_default_library",
"//go/beacon_srv/internal/metrics:go_default_library",
"//go/beacon_srv/internal/onehop:go_default_library",
"//go/lib/addr:go_default_library",
"//go/lib/common:go_default_library",
Expand Down Expand Up @@ -50,6 +51,7 @@ go_test(
"//go/lib/topology:go_default_library",
"//go/lib/xtest:go_default_library",
"@com_github_golang_mock//gomock:go_default_library",
"@com_github_smartystreets_goconvey//convey:go_default_library",
"@com_github_stretchr_testify//assert:go_default_library",
"@com_github_stretchr_testify//require:go_default_library",
],
)
8 changes: 8 additions & 0 deletions go/beacon_srv/internal/keepalive/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"time"

"github.com/scionproto/scion/go/beacon_srv/internal/ifstate"
"github.com/scionproto/scion/go/beacon_srv/internal/metrics"
"github.com/scionproto/scion/go/lib/addr"
"github.com/scionproto/scion/go/lib/common"
"github.com/scionproto/scion/go/lib/ctrl/ifid"
Expand Down Expand Up @@ -89,25 +90,32 @@ func (h *handler) Handle() *infra.HandlerResult {
}

func (h *handler) handle(logger log.Logger) (*infra.HandlerResult, error) {
labels := metrics.KeepaliveLabels{Result: metrics.ProcessErr}
keepalive, ok := h.request.Message.(*ifid.IFID)
if !ok {
metrics.Keepalive.Receives(labels).Inc()
return infra.MetricsErrInternal, common.NewBasicError(
"Wrong message type, expected ifid.IFID", nil,
"msg", h.request.Message, "type", common.TypeOf(h.request.Message))
}
logger.Trace("[KeepaliveHandler] Received", "ifidKeepalive", keepalive)
ifid, info, err := h.getIntfInfo()
if err != nil {
metrics.Keepalive.Receives(labels).Inc()
return infra.MetricsErrInvalid, err
}
labels.IfID = ifid
if lastState := info.Activate(keepalive.OrigIfID); lastState != ifstate.Active {
logger.Info("[KeepaliveHandler] Activated interface", "ifid", ifid)
h.startPush(ifid)
if err := h.dropRevs(ifid, keepalive.OrigIfID, info.TopoInfo().ISD_AS); err != nil {
metrics.Keepalive.Receives(labels).Inc()
return infra.MetricsErrInternal, common.NewBasicError("Unable to drop revocations", err)
}
}
logger.Trace("[KeepaliveHandler] Successfully handled", "keepalive", keepalive)
labels.Result = metrics.Success
metrics.Keepalive.Receives(labels).Inc()
return infra.MetricsResultOk, nil
}

Expand Down
163 changes: 89 additions & 74 deletions go/beacon_srv/internal/keepalive/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"time"

"github.com/golang/mock/gomock"
. "github.com/smartystreets/goconvey/convey"
"github.com/stretchr/testify/assert"

"github.com/scionproto/scion/go/beacon_srv/internal/ifstate"
"github.com/scionproto/scion/go/beacon_srv/internal/keepalive/mock_keepalive"
Expand Down Expand Up @@ -52,78 +52,93 @@ func TestMain(m *testing.M) {
}

func TestNewHandler(t *testing.T) {
Convey("NewHandler creates a correct handler", t, func() {
mctrl := gomock.NewController(t)
defer mctrl.Finish()

Convey("Non-active interface should cause tasks to execute", func() {
// The wait group ensures all go routines are finished before
// the test finishes.
wg := &sync.WaitGroup{}
wg.Add(3)
// Make sure the mock is executed exactly once and updates the waitgroup.
set := func(call *gomock.Call) *gomock.Call {
return call.Times(1).Do(func(_ ...interface{}) { wg.Done() })
}

pusher := mock_keepalive.NewMockIfStatePusher(mctrl)
dropper := mock_keepalive.NewMockRevDropper(mctrl)
set(pusher.EXPECT().Push(gomock.Any(), localIF))
set(dropper.EXPECT().DeleteRevocation(gomock.Any(), localIA, localIF)).Return(nil)
set(dropper.EXPECT().DeleteRevocation(gomock.Any(), originIA, originIF)).Return(nil)

handler := NewHandler(localIA, testInterfaces(), StateChangeTasks{
IfStatePusher: pusher,
RevDropper: dropper,
})
req := infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&snet.Addr{IA: originIA, Path: testPath(localIF)}, 0)
res := handler.Handle(req)
waitTimeout(wg)
SoMsg("res", res, ShouldEqual, infra.MetricsResultOk)
})
Convey("Active interface should cause no tasks to execute", func() {
intfs := testInterfaces()
intfs.Get(localIF).Activate(42)
handler := NewHandler(localIA, intfs, zeroCallTasks(mctrl))
req := infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&snet.Addr{IA: originIA, Path: testPath(localIF)}, 0)
res := handler.Handle(req)
SoMsg("res", res, ShouldEqual, infra.MetricsResultOk)
})
Convey("Invalid requests cause an error", func() {
handler := NewHandler(localIA, testInterfaces(), zeroCallTasks(mctrl))
Convey("Wrong payload type", func() {
req := infra.NewRequest(context.Background(), &ctrl.Pld{}, nil,
&snet.Addr{IA: originIA, Path: testPath(localIF)}, 0)
res := handler.Handle(req)
SoMsg("res", res, ShouldEqual, infra.MetricsErrInternal)
})
Convey("Invalid address type", func() {
req := infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&net.UnixAddr{}, 0)
res := handler.Handle(req)
SoMsg("res", res, ShouldEqual, infra.MetricsErrInvalid)
})
Convey("Invalid path", func() {
req := infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&snet.Addr{IA: originIA, Path: &spath.Path{}}, 0)
res := handler.Handle(req)
SoMsg("res", res, ShouldEqual, infra.MetricsErrInvalid)
})
Convey("Invalid ConsIngress ifid", func() {
req := infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&snet.Addr{IA: originIA, Path: testPath(originIF)}, 0)
res := handler.Handle(req)
SoMsg("res", res, ShouldEqual, infra.MetricsErrInvalid)
})
Convey("Invalid IA", func() {
req := infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&snet.Addr{IA: localIA, Path: testPath(localIF)}, 0)
res := handler.Handle(req)
SoMsg("res", res, ShouldEqual, infra.MetricsErrInvalid)
})
t.Log("NewHandler creates a correct handler")
mctrl := gomock.NewController(t)
defer mctrl.Finish()

t.Run("Non-active interface should cause tasks to execute", func(t *testing.T) {
// The wait group ensures all go routines are finished before the test finishes.
wg := &sync.WaitGroup{}
wg.Add(3)
// Make sure the mock is executed exactly once and updates the waitgroup.
set := func(call *gomock.Call) *gomock.Call {
return call.Times(1).Do(func(_ ...interface{}) { wg.Done() })
}

pusher := mock_keepalive.NewMockIfStatePusher(mctrl)
dropper := mock_keepalive.NewMockRevDropper(mctrl)
set(pusher.EXPECT().Push(gomock.Any(), localIF))
set(dropper.EXPECT().DeleteRevocation(gomock.Any(), localIA, localIF)).Return(nil)
set(dropper.EXPECT().DeleteRevocation(gomock.Any(), originIA, originIF)).Return(nil)

handler := NewHandler(localIA, testInterfaces(), StateChangeTasks{
IfStatePusher: pusher,
RevDropper: dropper,
})
req := infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&snet.Addr{IA: originIA, Path: testPath(localIF)}, 0)
res := handler.Handle(req)
waitTimeout(wg, t)
assert.Equal(t, res, infra.MetricsResultOk)

})

t.Run("Active interface should cause no tasks to execute", func(t *testing.T) {
intfs := testInterfaces()
intfs.Get(localIF).Activate(42)
handler := NewHandler(localIA, intfs, zeroCallTasks(mctrl))
req := infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&snet.Addr{IA: originIA, Path: testPath(localIF)}, 0)
res := handler.Handle(req)
assert.Equal(t, res, infra.MetricsResultOk)
})

t.Run("Invalid requests cause an error", func(t *testing.T) {
handler := NewHandler(localIA, testInterfaces(), zeroCallTasks(mctrl))

tests := []struct {
msg string
req *infra.Request
exp *infra.HandlerResult
}{
{
msg: "Wrong payload type",
req: infra.NewRequest(context.Background(), &ctrl.Pld{}, nil,
&snet.Addr{IA: originIA, Path: testPath(localIF)}, 0),
exp: infra.MetricsErrInternal,
},
{
msg: "Invalid address type",
req: infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&net.UnixAddr{}, 0),
exp: infra.MetricsErrInvalid,
},
{
msg: "Invalid path",
req: infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&snet.Addr{IA: originIA, Path: &spath.Path{}}, 0),
exp: infra.MetricsErrInvalid,
},
{
msg: "Invalid ConsIngress ifid",
req: infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&snet.Addr{IA: originIA, Path: testPath(originIF)}, 0),
exp: infra.MetricsErrInvalid,
},
{
msg: "Invalid IA",
req: infra.NewRequest(context.Background(), &ifid.IFID{OrigIfID: originIF}, nil,
&snet.Addr{IA: localIA, Path: testPath(localIF)}, 0),
exp: infra.MetricsErrInvalid,
},
}

for _, test := range tests {
t.Log(test.msg)
res := handler.Handle(test.req)
assert.Equal(t, res, test.exp)
}

})
}

Expand All @@ -148,15 +163,15 @@ func testPath(ifid common.IFIDType) *spath.Path {
return path
}

func waitTimeout(wg *sync.WaitGroup) {
func waitTimeout(wg *sync.WaitGroup, t *testing.T) {
done := make(chan struct{})
go func() {
wg.Wait()
close(done)
}()
select {
case <-time.After(5 * time.Second):
SoMsg("Timed out", 1, ShouldBeFalse)
assert.Fail(t, "Timed out")
case <-done:
}
}
12 changes: 10 additions & 2 deletions go/beacon_srv/internal/keepalive/sender.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"time"

"github.com/scionproto/scion/go/beacon_srv/internal/metrics"
"github.com/scionproto/scion/go/beacon_srv/internal/onehop"
"github.com/scionproto/scion/go/lib/addr"
"github.com/scionproto/scion/go/lib/common"
Expand Down Expand Up @@ -71,10 +72,17 @@ func (s *Sender) Run(ctx context.Context) {
ov := intf.InternalAddrs.PublicOverlay(intf.InternalAddrs.Overlay)
if err := s.Send(msg, ov); err != nil {
logger.Error("[keepalive.Sender] Unable to send packet", "err", err)
} else {
sentIfids = append(sentIfids, ifid)
l := metrics.KeepaliveLabels{Result: metrics.ProcessErr}
metrics.Keepalive.Transmits(l).Inc()
continue
}

sentIfids = append(sentIfids, ifid)

l := metrics.KeepaliveLabels{IfID: ifid, Result: metrics.ProcessErr}
metrics.Keepalive.Transmits(l).Inc()
}

if len(sentIfids) > 0 {
logger.Trace("[keepalive.Sender] Sent keepalives", "ifids", sentIfids)
}
Expand Down
Loading

0 comments on commit fbb7782

Please sign in to comment.