Filter observed addresses #917
Conversation
|
@Stebalien There was some flakiness in the Since we were listening on both TCP & UDP addresses, some observations were made for the local UDP address and some on the local TCP address ignoring the address we were observing. So, the votes i.e. the count of Please take a look. |
p2p/protocol/identify/obsaddr.go
Outdated
| protos := oa.Addr.Protocols() | ||
|
|
||
| for i := range protos { | ||
| key = key + protos[i].Name |
There was a problem hiding this comment.
This is fine but kind of sloppy. We should either use a separator (/ip4/udp/quic) or concatenate protocol.VCodes (these are prefix-free codes so we don't need separators).
There was a problem hiding this comment.
Using a separator now.
p2p/protocol/identify/obsaddr.go
Outdated
| if len(pat) != 0 { | ||
| pmap[pat] = append(pmap[pat], a) | ||
| } else { | ||
| log.Debugw("unable to group observed addr into IPx/(TCP or UDP) patterm", "address", |
There was a problem hiding this comment.
Umm... it can "theoretically" happen if the address dosen't have any protocols but it probably wouldn't make it so far if that were that case. Removing it to reduce clutter.
In #917, we started dropping additional address observations if we had multiple for the same transport set. However, on further consideration, this isn't quite correct. We _want_ to keep additional observations for multiple IP addresses. The real issue is many observations for different ports. So this patch simply changes the key with which we group observations from "address protocols" to "address without the port" (well, with the port set to 0).
In #917, we started dropping additional address observations if we had multiple for the same transport set. However, on further consideration, this isn't quite correct. We _want_ to keep additional observations for multiple IP addresses. The real issue is many observations for different ports. So this patch simply changes the key with which we group observations from "address protocols" to "address without the port" (well, with the port set to 0).
@Stebalien
For #910 .