c/ooni-probe-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

cleanup: remove ConnID, DialID, TransactionID (#395)

Browse Source 
We are not using them anymore. The only nettest still using the
legacy netx implementation is tor, for which setting these fields
is useless, because it performs each measurement into a separate
goroutine. Hence, let us start removing this part of the legacy
netx codebase, which is hampering progress in other areas.

Occurred to me while doing testing for the recent changes in
error mapping (https://github.com/ooni/probe/issues/1505).
This commit is contained in:
Simone Basso
2021-06-23 13:36:45 +02:00
committed by GitHub
parent 1fefe5d9b8
commit c74c94d616
32 changed files with 78 additions and 896 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/engine/legacy/netx/oldhttptransport/bodytracer.go
-5
View File
@@ -6,7 +6,6 @@ import (
"time"
"github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx"
"github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/transactionid"
)
// BodyTracer performs single HTTP transactions and emits
@@ -33,7 +32,6 @@ func (t *BodyTracer) RoundTrip(req *http.Request) (resp *http.Response, err erro
resp.Body = &bodyWrapper{
ReadCloser: resp.Body,
root: modelx.ContextMeasurementRootOrDefault(req.Context()),
tid: transactionid.ContextTransactionID(req.Context()),
}
return
}
@@ -52,7 +50,6 @@ func (t *BodyTracer) CloseIdleConnections() {
type bodyWrapper struct {
io.ReadCloser
root *modelx.MeasurementRoot
tid int64
}
func (bw *bodyWrapper) Read(b []byte) (n int, err error) {
@@ -64,7 +61,6 @@ func (bw *bodyWrapper) Read(b []byte) (n int, err error) {
Data: b[:n],
Error: err,
DurationSinceBeginning: time.Since(bw.root.Beginning),
TransactionID: bw.tid,
},
})
return
@@ -75,7 +71,6 @@ func (bw *bodyWrapper) Close() (err error) {
bw.root.Handler.OnMeasurement(modelx.Measurement{
HTTPResponseDone: &modelx.HTTPResponseDoneEvent{
DurationSinceBeginning: time.Since(bw.root.Beginning),
TransactionID: bw.tid,
},
})
return
internal/engine/legacy/netx/oldhttptransport/httptransport.go
+1 -2
View File
@@ -15,8 +15,7 @@ type Transport struct {
// New creates a new Transport.
func New(roundTripper http.RoundTripper) *Transport {
return &Transport{
roundTripper: NewTransactioner(NewBodyTracer(
NewTraceTripper(roundTripper))),
roundTripper: NewBodyTracer(NewTraceTripper(roundTripper)),
}
}
internal/engine/legacy/netx/oldhttptransport/tracetripper.go
+7 -28
View File
@@ -11,10 +11,7 @@ import (
"time"
"github.com/ooni/probe-cli/v3/internal/atomicx"
"github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/connid"
"github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/dialid"
"github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx"
"github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/transactionid"
"github.com/ooni/probe-cli/v3/internal/engine/netx/errorx"
"github.com/ooni/probe-cli/v3/internal/iox"
)
@@ -76,13 +73,10 @@ func readSnap(
func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) {
root := modelx.ContextMeasurementRootOrDefault(req.Context())
tid := transactionid.ContextTransactionID(req.Context())
root.Handler.OnMeasurement(modelx.Measurement{
HTTPRoundTripStart: &modelx.HTTPRoundTripStartEvent{
DialID: dialid.ContextDialID(req.Context()),
DurationSinceBeginning: time.Since(root.Beginning),
Method: req.Method,
TransactionID: tid,
URL: req.URL.String(),
},
})
@@ -116,7 +110,6 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) {
root.Handler.OnMeasurement(modelx.Measurement{
TLSHandshakeStart: &modelx.TLSHandshakeStartEvent{
DurationSinceBeginning: time.Since(root.Beginning),
TransactionID: tid,
},
})
},
@@ -124,9 +117,8 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) {
// Wrapping the error even if we're not returning it because it may
// less confusing to users to see the wrapped name
err = errorx.SafeErrWrapperBuilder{
Error: err,
Operation: errorx.TLSHandshakeOperation,
TransactionID: tid,
Error: err,
Operation: errorx.TLSHandshakeOperation,
}.MaybeBuild()
durationSinceBeginning := time.Since(root.Beginning)
// Event emitted by net/http when DialTLS is not
@@ -136,7 +128,6 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) {
ConnectionState: modelx.NewTLSConnectionState(state),
Error: err,
DurationSinceBeginning: durationSinceBeginning,
TransactionID: tid,
},
})
},
@@ -146,12 +137,7 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) {
majorOpMu.Unlock()
root.Handler.OnMeasurement(modelx.Measurement{
HTTPConnectionReady: &modelx.HTTPConnectionReadyEvent{
ConnID: connid.Compute(
info.Conn.LocalAddr().Network(),
info.Conn.LocalAddr().String(),
),
DurationSinceBeginning: time.Since(root.Beginning),
TransactionID: tid,
},
})
},
@@ -168,7 +154,6 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) {
HTTPRequestHeader: &modelx.HTTPRequestHeaderEvent{
DurationSinceBeginning: time.Since(root.Beginning),
Key: key,
TransactionID: tid,
Value: values,
},
})
@@ -179,8 +164,7 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) {
DurationSinceBeginning: time.Since(root.Beginning),
Headers: requestHeaders, // [*]
Method: req.Method, // [*]
TransactionID: tid,
URL: req.URL, // [*]
URL: req.URL, // [*]
},
})
},
@@ -188,15 +172,13 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) {
// Wrapping the error even if we're not returning it because it may
// less confusing to users to see the wrapped name
err := errorx.SafeErrWrapperBuilder{
Error: info.Err,
Operation: errorx.HTTPRoundTripOperation,
TransactionID: tid,
Error: info.Err,
Operation: errorx.HTTPRoundTripOperation,
}.MaybeBuild()
root.Handler.OnMeasurement(modelx.Measurement{
HTTPRequestDone: &modelx.HTTPRequestDoneEvent{
DurationSinceBeginning: time.Since(root.Beginning),
Error: err,
TransactionID: tid,
},
})
},
@@ -204,7 +186,6 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) {
root.Handler.OnMeasurement(modelx.Measurement{
HTTPResponseStart: &modelx.HTTPResponseStartEvent{
DurationSinceBeginning: time.Since(root.Beginning),
TransactionID: tid,
},
})
},
@@ -227,9 +208,8 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) {
resp, err := t.roundTripper.RoundTrip(req)
err = errorx.SafeErrWrapperBuilder{
Error: err,
Operation: majorOp,
TransactionID: tid,
Error: err,
Operation: majorOp,
}.MaybeBuild()
// [*] Require less event joining work by providing info that
// makes this event alone actionable for OONI
@@ -241,7 +221,6 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) {
RequestMethod: req.Method, // [*]
RequestURL: req.URL.String(), // [*]
MaxBodySnapSize: snapSize,
TransactionID: tid,
}
if resp != nil {
event.ResponseHeaders = resp.Header
internal/engine/legacy/netx/oldhttptransport/transactioner.go
-38
View File
@@ -1,38 +0,0 @@
package oldhttptransport
import (
"net/http"
"github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/transactionid"
)
// Transactioner performs single HTTP transactions.
type Transactioner struct {
roundTripper http.RoundTripper
}
// NewTransactioner creates a new Transport.
func NewTransactioner(roundTripper http.RoundTripper) *Transactioner {
return &Transactioner{
roundTripper: roundTripper,
}
}
// RoundTrip executes a single HTTP transaction, returning
// a Response for the provided Request.
func (t *Transactioner) RoundTrip(req *http.Request) (*http.Response, error) {
return t.roundTripper.RoundTrip(req.WithContext(
transactionid.WithTransactionID(req.Context()),
))
}
// CloseIdleConnections closes the idle connections.
func (t *Transactioner) CloseIdleConnections() {
// Adapted from net/http code
type closeIdler interface {
CloseIdleConnections()
}
if tr, ok := t.roundTripper.(closeIdler); ok {
tr.CloseIdleConnections()
}
}
internal/engine/legacy/netx/oldhttptransport/transactioner_test.go
-58
View File
@@ -1,58 +0,0 @@
package oldhttptransport
import (
"context"
"net/http"
"testing"
"github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/transactionid"
"github.com/ooni/probe-cli/v3/internal/iox"
)
type transactionerCheckTransactionID struct {
roundTripper http.RoundTripper
t *testing.T
}
func (t *transactionerCheckTransactionID) RoundTrip(req *http.Request) (*http.Response, error) {
ctx := req.Context()
if id := transactionid.ContextTransactionID(ctx); id == 0 {
t.t.Fatal("transaction ID not set")
}
return t.roundTripper.RoundTrip(req)
}
func TestTransactionerSuccess(t *testing.T) {
client := &http.Client{
Transport: NewTransactioner(&transactionerCheckTransactionID{
roundTripper: http.DefaultTransport,
t: t,
}),
}
resp, err := client.Get("https://www.google.com")
if err != nil {
t.Fatal(err)
}
defer resp.Body.Close()
_, err = iox.ReadAllContext(context.Background(), resp.Body)
if err != nil {
t.Fatal(err)
}
client.CloseIdleConnections()
}
func TestTransactionerFailure(t *testing.T) {
client := &http.Client{
Transport: NewTransactioner(http.DefaultTransport),
}
// This fails the request because we attempt to speak cleartext HTTP with
// a server that instead is expecting TLS.
resp, err := client.Get("http://www.google.com:443")
if err == nil {
t.Fatal("expected an error here")
}
if resp != nil {
t.Fatal("expected a nil response here")
}
client.CloseIdleConnections()
}