From cbcdd7908474784d4a79452e6b33faefaff566e4 Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Fri, 24 Feb 2023 13:23:27 -0800 Subject: [PATCH] Fix flaky TestMetricsNoAllocNoCover test (#2142) --- p2p/protocol/identify/id.go | 16 ++++++++-------- p2p/protocol/identify/metrics.go | 12 ++++++------ .../{ => metrics_alloc_test}/metrics_test.go | 19 +++++++++++++------ 3 files changed, 27 insertions(+), 20 deletions(-) rename p2p/protocol/identify/{ => metrics_alloc_test}/metrics_test.go (61%) diff --git a/p2p/protocol/identify/id.go b/p2p/protocol/identify/id.go index 72d591ec55..91c351e5bd 100644 --- a/p2p/protocol/identify/id.go +++ b/p2p/protocol/identify/id.go @@ -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 { @@ -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 } @@ -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) } } @@ -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 { diff --git a/p2p/protocol/identify/metrics.go b/p2p/protocol/identify/metrics.go index 28598fa33b..f11a5851db 100644 --- a/p2p/protocol/identify/metrics.go +++ b/p2p/protocol/identify/metrics.go @@ -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) @@ -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) @@ -186,7 +186,7 @@ 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) @@ -194,11 +194,11 @@ func (t *metricsTracer) ConnPushSupport(support identifyPushSupport) { 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" diff --git a/p2p/protocol/identify/metrics_test.go b/p2p/protocol/identify/metrics_alloc_test/metrics_test.go similarity index 61% rename from p2p/protocol/identify/metrics_test.go rename to p2p/protocol/identify/metrics_alloc_test/metrics_test.go index 2cf5a209a1..961daf1406 100644 --- a/p2p/protocol/identify/metrics_test.go +++ b/p2p/protocol/identify/metrics_alloc_test/metrics_test.go @@ -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) { @@ -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 {