Skip to content

Commit

Permalink
refactor(measurexlite): depend on model.MeasuringNetwork (#1260)
Browse files Browse the repository at this point in the history
With this diff, we detach measurexlite from netxlite. It was already
quite detached through functions used for testing. However, the changes
we implement are allow us to test measurexlite code by changing the
.Netx field of a *Trace, which means we can avoid using netxlite's
singleton for new code.

Another benefit of this diff is that we have clearly spelled out and
packaged into an interface the dependencies required to perform
measurements with measurexlite. Therefore, we can gracefully continue
separating the code used for measuring from the code for contacting the
backend, as detailed in ooni/probe#2531.
  • Loading branch information
bassosimone authored Sep 12, 2023
1 parent a76ac68 commit 56e4587
Show file tree
Hide file tree
Showing 11 changed files with 226 additions and 518 deletions.
2 changes: 1 addition & 1 deletion internal/measurexlite/dialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
// except that it returns a model.Dialer that uses this trace.
func (tx *Trace) NewDialerWithoutResolver(dl model.DebugLogger) model.Dialer {
return &dialerTrace{
d: tx.newDialerWithoutResolver(dl),
d: tx.Netx.NewDialerWithoutResolver(dl),
tx: tx,
}
}
Expand Down
18 changes: 12 additions & 6 deletions internal/measurexlite/dialer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ func TestNewDialerWithoutResolver(t *testing.T) {
underlying := &mocks.Dialer{}
zeroTime := time.Now()
trace := NewTrace(0, zeroTime)
trace.newDialerWithoutResolverFn = func(dl model.DebugLogger) model.Dialer {
return underlying
trace.Netx = &mocks.MeasuringNetwork{
MockNewDialerWithoutResolver: func(dl model.DebugLogger, w ...model.DialerWrapper) model.Dialer {
return underlying
},
}
dialer := trace.NewDialerWithoutResolver(model.DiscardLogger)
dt := dialer.(*dialerTrace)
Expand All @@ -46,8 +48,10 @@ func TestNewDialerWithoutResolver(t *testing.T) {
return nil, expectedErr
},
}
trace.newDialerWithoutResolverFn = func(dl model.DebugLogger) model.Dialer {
return underlying
trace.Netx = &mocks.MeasuringNetwork{
MockNewDialerWithoutResolver: func(dl model.DebugLogger, w ...model.DialerWrapper) model.Dialer {
return underlying
},
}
dialer := trace.NewDialerWithoutResolver(model.DiscardLogger)
ctx := context.Background()
Expand All @@ -72,8 +76,10 @@ func TestNewDialerWithoutResolver(t *testing.T) {
called = true
},
}
trace.newDialerWithoutResolverFn = func(dl model.DebugLogger) model.Dialer {
return underlying
trace.Netx = &mocks.MeasuringNetwork{
MockNewDialerWithoutResolver: func(dl model.DebugLogger, w ...model.DialerWrapper) model.Dialer {
return underlying
},
}
dialer := trace.NewDialerWithoutResolver(model.DiscardLogger)
dialer.CloseIdleConnections()
Expand Down
6 changes: 3 additions & 3 deletions internal/measurexlite/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,17 @@ func (r *resolverTrace) LookupNS(ctx context.Context, domain string) ([]*net.NS,

// NewStdlibResolver returns a trace-ware system resolver
func (tx *Trace) NewStdlibResolver(logger model.Logger) model.Resolver {
return tx.wrapResolver(tx.newStdlibResolver(logger))
return tx.wrapResolver(tx.Netx.NewStdlibResolver(logger))
}

// NewParallelUDPResolver returns a trace-ware parallel UDP resolver
func (tx *Trace) NewParallelUDPResolver(logger model.Logger, dialer model.Dialer, address string) model.Resolver {
return tx.wrapResolver(tx.newParallelUDPResolver(logger, dialer, address))
return tx.wrapResolver(tx.Netx.NewParallelUDPResolver(logger, dialer, address))
}

// NewParallelDNSOverHTTPSResolver returns a trace-aware parallel DoH resolver
func (tx *Trace) NewParallelDNSOverHTTPSResolver(logger model.Logger, URL string) model.Resolver {
return tx.wrapResolver(tx.newParallelDNSOverHTTPSResolver(logger, URL))
return tx.wrapResolver(tx.Netx.NewParallelDNSOverHTTPSResolver(logger, URL))
}

// OnDNSRoundTripForLookupHost implements model.Trace.OnDNSRoundTripForLookupHost
Expand Down
2 changes: 1 addition & 1 deletion internal/measurexlite/quic.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
// except that it returns a model.QUICDialer that uses this trace.
func (tx *Trace) NewQUICDialerWithoutResolver(listener model.UDPListener, dl model.DebugLogger) model.QUICDialer {
return &quicDialerTrace{
qd: tx.newQUICDialerWithoutResolver(listener, dl),
qd: tx.Netx.NewQUICDialerWithoutResolver(listener, dl),
tx: tx,
}
}
Expand Down
18 changes: 12 additions & 6 deletions internal/measurexlite/quic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ func TestNewQUICDialerWithoutResolver(t *testing.T) {
underlying := &mocks.QUICDialer{}
zeroTime := time.Now()
trace := NewTrace(0, zeroTime)
trace.newQUICDialerWithoutResolverFn = func(listener model.UDPListener, dl model.DebugLogger) model.QUICDialer {
return underlying
trace.Netx = &mocks.MeasuringNetwork{
MockNewQUICDialerWithoutResolver: func(listener model.UDPListener, logger model.DebugLogger, w ...model.QUICDialerWrapper) model.QUICDialer {
return underlying
},
}
listener := &mocks.UDPListener{}
dialer := trace.NewQUICDialerWithoutResolver(listener, model.DiscardLogger)
Expand All @@ -50,8 +52,10 @@ func TestNewQUICDialerWithoutResolver(t *testing.T) {
return nil, expectedErr
},
}
trace.newQUICDialerWithoutResolverFn = func(listener model.UDPListener, dl model.DebugLogger) model.QUICDialer {
return underlying
trace.Netx = &mocks.MeasuringNetwork{
MockNewQUICDialerWithoutResolver: func(listener model.UDPListener, logger model.DebugLogger, w ...model.QUICDialerWrapper) model.QUICDialer {
return underlying
},
}
listener := &mocks.UDPListener{}
dialer := trace.NewQUICDialerWithoutResolver(listener, model.DiscardLogger)
Expand All @@ -77,8 +81,10 @@ func TestNewQUICDialerWithoutResolver(t *testing.T) {
called = true
},
}
trace.newQUICDialerWithoutResolverFn = func(listener model.UDPListener, dl model.DebugLogger) model.QUICDialer {
return underlying
trace.Netx = &mocks.MeasuringNetwork{
MockNewQUICDialerWithoutResolver: func(listener model.UDPListener, logger model.DebugLogger, w ...model.QUICDialerWrapper) model.QUICDialer {
return underlying
},
}
listener := &mocks.UDPListener{}
dialer := trace.NewQUICDialerWithoutResolver(listener, model.DiscardLogger)
Expand Down
2 changes: 1 addition & 1 deletion internal/measurexlite/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
// except that it returns a model.TLSHandshaker that uses this trace.
func (tx *Trace) NewTLSHandshakerStdlib(dl model.DebugLogger) model.TLSHandshaker {
return &tlsHandshakerTrace{
thx: tx.newTLSHandshakerStdlib(dl),
thx: tx.Netx.NewTLSHandshakerStdlib(dl),
tx: tx,
}
}
Expand Down
12 changes: 8 additions & 4 deletions internal/measurexlite/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ func TestNewTLSHandshakerStdlib(t *testing.T) {
underlying := &mocks.TLSHandshaker{}
zeroTime := time.Now()
trace := NewTrace(0, zeroTime)
trace.newTLSHandshakerStdlibFn = func(dl model.DebugLogger) model.TLSHandshaker {
return underlying
trace.Netx = &mocks.MeasuringNetwork{
MockNewTLSHandshakerStdlib: func(logger model.DebugLogger) model.TLSHandshaker {
return underlying
},
}
thx := trace.NewTLSHandshakerStdlib(model.DiscardLogger)
thxt := thx.(*tlsHandshakerTrace)
Expand All @@ -49,8 +51,10 @@ func TestNewTLSHandshakerStdlib(t *testing.T) {
return nil, tls.ConnectionState{}, expectedErr
},
}
trace.newTLSHandshakerStdlibFn = func(dl model.DebugLogger) model.TLSHandshaker {
return underlying
trace.Netx = &mocks.MeasuringNetwork{
MockNewTLSHandshakerStdlib: func(logger model.DebugLogger) model.TLSHandshaker {
return underlying
},
}
thx := trace.NewTLSHandshakerStdlib(model.DiscardLogger)
ctx := context.Background()
Expand Down
119 changes: 13 additions & 106 deletions internal/measurexlite/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/netxlite"
utls "gitlab.com/yawning/utls.git"
)

// Trace implements [model.Trace]. We use a [context.Context] to register ourselves
Expand All @@ -31,6 +30,11 @@ type Trace struct {
// once you have constructed a trace MAY lead to data races.
Index int64

// Netx is the network to use for measuring. The constructor inits this
// field using a [*netxlite.Netx]. You MAY override this field for testing. Make
// sure you do that before you start measuring to avoid data races.
Netx model.MeasuringNetwork

// bytesReceivedMap maps a remote host with the bytes we received
// from such a remote host. Accessing this map requires one to
// additionally hold the bytesReceivedMu mutex.
Expand All @@ -40,43 +44,15 @@ type Trace struct {
// access from multiple goroutines.
bytesReceivedMu *sync.Mutex

// networkEvent is MANDATORY and buffers network events.
networkEvent chan *model.ArchivalNetworkEvent

// newStdlibResolverFn is OPTIONAL and can be used to overide
// calls to the netxlite.NewStdlibResolver factory.
newStdlibResolverFn func(logger model.Logger) model.Resolver

// newParallelUDPResolverFn is OPTIONAL and can be used to overide
// calls to the netxlite.NewParallelUDPResolver factory.
newParallelUDPResolverFn func(logger model.Logger, dialer model.Dialer, address string) model.Resolver

// newParallelDNSOverHTTPSResolverFn is OPTIONAL and can be used to overide
// calls to the netxlite.NewParallelDNSOverHTTPSUDPResolver factory.
newParallelDNSOverHTTPSResolverFn func(logger model.Logger, URL string) model.Resolver

// newDialerWithoutResolverFn is OPTIONAL and can be used to override
// calls to the netxlite.NewDialerWithoutResolver factory.
newDialerWithoutResolverFn func(dl model.DebugLogger) model.Dialer

// newTLSHandshakerStdlibFn is OPTIONAL and can be used to overide
// calls to the netxlite.NewTLSHandshakerStdlib factory.
newTLSHandshakerStdlibFn func(dl model.DebugLogger) model.TLSHandshaker

// newTLSHandshakerUTLSFn is OPTIONAL and can be used to overide
// calls to the netxlite.NewTLSHandshakerUTLS factory.
newTLSHandshakerUTLSFn func(dl model.DebugLogger, id *utls.ClientHelloID) model.TLSHandshaker

// NewDialerWithoutResolverFn is OPTIONAL and can be used to override
// calls to the netxlite.NewQUICDialerWithoutResolver factory.
newQUICDialerWithoutResolverFn func(listener model.UDPListener, dl model.DebugLogger) model.QUICDialer

// dnsLookup is MANDATORY and buffers DNS Lookup observations.
dnsLookup chan *model.ArchivalDNSLookupResult

// delayedDNSResponse is MANDATORY and buffers delayed DNS responses.
delayedDNSResponse chan *model.ArchivalDNSLookupResult

// networkEvent is MANDATORY and buffers network events.
networkEvent chan *model.ArchivalNetworkEvent

// tcpConnect is MANDATORY and buffers TCP connect observations.
tcpConnect chan *model.ArchivalTCPConnectResult

Expand Down Expand Up @@ -134,19 +110,9 @@ const QUICHandshakeBufferSize = 8
func NewTrace(index int64, zeroTime time.Time, tags ...string) *Trace {
return &Trace{
Index: index,
Netx: &netxlite.Netx{Underlying: nil}, // use the host network
bytesReceivedMap: make(map[string]int64),
bytesReceivedMu: &sync.Mutex{},
networkEvent: make(
chan *model.ArchivalNetworkEvent,
NetworkEventBufferSize,
),
newStdlibResolverFn: nil, // use default
newParallelUDPResolverFn: nil, // use default
newParallelDNSOverHTTPSResolverFn: nil, // use default
newDialerWithoutResolverFn: nil, // use default
newTLSHandshakerStdlibFn: nil, // use default
newTLSHandshakerUTLSFn: nil, // use default
newQUICDialerWithoutResolverFn: nil, // use default
dnsLookup: make(
chan *model.ArchivalDNSLookupResult,
DNSLookupBufferSize,
Expand All @@ -155,6 +121,10 @@ func NewTrace(index int64, zeroTime time.Time, tags ...string) *Trace {
chan *model.ArchivalDNSLookupResult,
DelayedDNSResponseBufferSize,
),
networkEvent: make(
chan *model.ArchivalNetworkEvent,
NetworkEventBufferSize,
),
tcpConnect: make(
chan *model.ArchivalTCPConnectResult,
TCPConnectBufferSize,
Expand All @@ -173,69 +143,6 @@ func NewTrace(index int64, zeroTime time.Time, tags ...string) *Trace {
}
}

// newStdlibResolver indirectly calls the passed netxlite.NewStdlibResolver
// thus allowing us to mock this function for testing
func (tx *Trace) newStdlibResolver(logger model.Logger) model.Resolver {
if tx.newStdlibResolverFn != nil {
return tx.newStdlibResolverFn(logger)
}
return netxlite.NewStdlibResolver(logger)
}

// newParallelUDPResolver indirectly calls the passed netxlite.NewParallerUDPResolver
// thus allowing us to mock this function for testing
func (tx *Trace) newParallelUDPResolver(logger model.Logger, dialer model.Dialer, address string) model.Resolver {
if tx.newParallelUDPResolverFn != nil {
return tx.newParallelUDPResolverFn(logger, dialer, address)
}
return netxlite.NewParallelUDPResolver(logger, dialer, address)
}

// newParallelDNSOverHTTPSResolver indirectly calls the passed netxlite.NewParallerDNSOverHTTPSResolver
// thus allowing us to mock this function for testing
func (tx *Trace) newParallelDNSOverHTTPSResolver(logger model.Logger, URL string) model.Resolver {
if tx.newParallelDNSOverHTTPSResolverFn != nil {
return tx.newParallelDNSOverHTTPSResolverFn(logger, URL)
}
return netxlite.NewParallelDNSOverHTTPSResolver(logger, URL)
}

// newDialerWithoutResolver indirectly calls netxlite.NewDialerWithoutResolver
// thus allowing us to mock this func for testing.
func (tx *Trace) newDialerWithoutResolver(dl model.DebugLogger) model.Dialer {
if tx.newDialerWithoutResolverFn != nil {
return tx.newDialerWithoutResolverFn(dl)
}
return netxlite.NewDialerWithoutResolver(dl)
}

// newTLSHandshakerStdlib indirectly calls netxlite.NewTLSHandshakerStdlib
// thus allowing us to mock this func for testing.
func (tx *Trace) newTLSHandshakerStdlib(dl model.DebugLogger) model.TLSHandshaker {
if tx.newTLSHandshakerStdlibFn != nil {
return tx.newTLSHandshakerStdlibFn(dl)
}
return netxlite.NewTLSHandshakerStdlib(dl)
}

// newTLSHandshakerUTLS indirectly calls netxlite.NewTLSHandshakerUTLS
// thus allowing us to mock this func for testing.
func (tx *Trace) newTLSHandshakerUTLS(dl model.DebugLogger, id *utls.ClientHelloID) model.TLSHandshaker {
if tx.newTLSHandshakerUTLSFn != nil {
return tx.newTLSHandshakerUTLSFn(dl, id)
}
return netxlite.NewTLSHandshakerUTLS(dl, id)
}

// newQUICDialerWithoutResolver indirectly calls netxlite.NewQUICDialerWithoutResolver
// thus allowing us to mock this func for testing.
func (tx *Trace) newQUICDialerWithoutResolver(listener model.UDPListener, dl model.DebugLogger) model.QUICDialer {
if tx.newQUICDialerWithoutResolverFn != nil {
return tx.newQUICDialerWithoutResolverFn(listener, dl)
}
return netxlite.NewQUICDialerWithoutResolver(listener, dl)
}

// TimeNow implements model.Trace.TimeNow.
func (tx *Trace) TimeNow() time.Time {
if tx.timeNowFn != nil {
Expand Down
Loading

0 comments on commit 56e4587

Please sign in to comment.