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.

Sign up for GitHub

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

Jump to bottom

identify: Fix flaky TestMetricsNoAllocNoCover test #2142

Merged
merged 1 commit into from
Feb 24, 2023
Merged

identify: Fix flaky TestMetricsNoAllocNoCover test #2142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 8 additions & 8 deletions p2p/protocol/identify/id.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@ type IDService interface {
io.Closer
}

type identifyPushSupport uint8
type IdentifyPushSupport uint8

const (
identifyPushSupportUnknown identifyPushSupport = iota
identifyPushSupported
identifyPushUnsupported
IdentifyPushSupportUnknown IdentifyPushSupport = iota
IdentifyPushSupported
IdentifyPushUnsupported
)

type entry struct {
Expand All @@ -96,7 +96,7 @@ type entry struct {

// PushSupport saves our knowledge about the peer's support of the Identify Push protocol.
// Before the identify request returns, we don't know yet if the peer supports Identify Push.
PushSupport identifyPushSupport
PushSupport IdentifyPushSupport
// Sequence is the sequence number of the last snapshot we sent to this peer.
Sequence uint64
}
Expand Down Expand Up @@ -271,7 +271,7 @@ func (ids *idService) sendPushes(ctx context.Context) {
for c, e := range ids.conns {
// Push even if we don't know if push is supported.
// This will be only the case while the IdentifyWaitChan call is in flight.
if e.PushSupport == identifyPushSupported || e.PushSupport == identifyPushSupportUnknown {
if e.PushSupport == IdentifyPushSupported || e.PushSupport == IdentifyPushSupportUnknown {
conns = append(conns, c)
}
}
Expand Down Expand Up @@ -496,9 +496,9 @@ func (ids *idService) handleIdentifyResponse(s network.Stream, isPush bool) erro
}
sup, err := ids.Host.Peerstore().SupportsProtocols(c.RemotePeer(), IDPush)
if supportsIdentifyPush := err == nil && len(sup) > 0; supportsIdentifyPush {
e.PushSupport = identifyPushSupported
e.PushSupport = IdentifyPushSupported
} else {
e.PushSupport = identifyPushUnsupported
e.PushSupport = IdentifyPushUnsupported
}

if ids.metricsTracer != nil {
Expand Down
12 changes: 6 additions & 6 deletions p2p/protocol/identify/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ type MetricsTracer interface {
TriggeredPushes(event any)

// ConnPushSupport counts peers by Push Support
ConnPushSupport(identifyPushSupport)
ConnPushSupport(IdentifyPushSupport)

// IdentifyReceived tracks metrics on receiving an identify response
IdentifyReceived(isPush bool, numProtocols int, numAddrs int)
Expand Down Expand Up @@ -146,7 +146,7 @@ func (t *metricsTracer) TriggeredPushes(ev any) {
pushesTriggered.WithLabelValues(*tags...).Inc()
}

func (t *metricsTracer) IncrementPushSupport(s identifyPushSupport) {
func (t *metricsTracer) IncrementPushSupport(s IdentifyPushSupport) {
tags := metricshelper.GetStringSlice()
defer metricshelper.PutStringSlice(tags)

Expand Down Expand Up @@ -186,19 +186,19 @@ func (t *metricsTracer) IdentifyReceived(isPush bool, numProtocols int, numAddrs
numAddrsReceived.Observe(float64(numAddrs))
}

func (t *metricsTracer) ConnPushSupport(support identifyPushSupport) {
func (t *metricsTracer) ConnPushSupport(support IdentifyPushSupport) {
tags := metricshelper.GetStringSlice()
defer metricshelper.PutStringSlice(tags)

*tags = append(*tags, getPushSupport(support))
connPushSupportTotal.WithLabelValues(*tags...).Inc()
}

func getPushSupport(s identifyPushSupport) string {
func getPushSupport(s IdentifyPushSupport) string {
switch s {
case identifyPushSupported:
case IdentifyPushSupported:
return "supported"
case identifyPushUnsupported:
case IdentifyPushUnsupported:
return "not supported"
default:
return "unknown"
Expand Down
19 changes: 13 additions & 6 deletions p2p/protocol/identify/metrics_test.go → ...entify/metrics_alloc_test/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
//go:build nocover

package identify
// These tests are in their own package to avoid transitively pulling in other
// deps that may run background tasks in their init and thus allocate. Looking
// at you
// [go-libp2p-asn-util](https://github.com/libp2p/go-libp2p-asn-util/blob/master/asn.go#L14)

package identify_alloc_test

import (
"math/rand"
"testing"

"github.com/libp2p/go-libp2p/core/event"
"github.com/libp2p/go-libp2p/p2p/protocol/identify"
)

func TestMetricsNoAllocNoCover(t *testing.T) {
Expand All @@ -16,19 +22,20 @@ func TestMetricsNoAllocNoCover(t *testing.T) {
event.EvtNATDeviceTypeChanged{},
}

pushSupport := []identifyPushSupport{
identifyPushSupportUnknown,
identifyPushSupported,
identifyPushUnsupported,
pushSupport := []identify.IdentifyPushSupport{
identify.IdentifyPushSupportUnknown,
identify.IdentifyPushSupported,
identify.IdentifyPushUnsupported,
}

tr := NewMetricsTracer()
tr := identify.NewMetricsTracer()
tests := map[string]func(){
"TriggeredPushes": func() { tr.TriggeredPushes(events[rand.Intn(len(events))]) },
"ConnPushSupport": func() { tr.ConnPushSupport(pushSupport[rand.Intn(len(pushSupport))]) },
"IdentifyReceived": func() { tr.IdentifyReceived(rand.Intn(2) == 0, rand.Intn(20), rand.Intn(20)) },
"IdentifySent": func() { tr.IdentifySent(rand.Intn(2) == 0, rand.Intn(20), rand.Intn(20)) },
}

for method, f := range tests {
allocs := testing.AllocsPerRun(1000, f)
if allocs > 0 {
Expand Down