From c74c94d61654fb63f68d5a15a9eb807fd532b20f Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 23 Jun 2021 13:36:45 +0200 Subject: [PATCH] cleanup: remove ConnID, DialID, TransactionID (#395) 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). --- internal/engine/experiment/hhfm/hhfm_test.go | 6 - internal/engine/legacy/netx/connid/connid.go | 31 ----- .../engine/legacy/netx/connid/connid_test.go | 80 ----------- internal/engine/legacy/netx/dialid/dialid.go | 24 ---- .../engine/legacy/netx/dialid/dialid_test.go | 24 ---- internal/engine/legacy/netx/emitterdialer.go | 22 --- .../engine/legacy/netx/emitterdialer_test.go | 29 +--- internal/engine/legacy/netx/modelx/modelx.go | 128 ------------------ .../netx/oldhttptransport/bodytracer.go | 5 - .../netx/oldhttptransport/httptransport.go | 3 +- .../netx/oldhttptransport/tracetripper.go | 35 +---- .../netx/oldhttptransport/transactioner.go | 38 ------ .../oldhttptransport/transactioner_test.go | 58 -------- .../netx/transactionid/transactionid.go | 25 ---- .../netx/transactionid/transactionid_test.go | 24 ---- .../engine/legacy/netxlogger/netxlogger.go | 54 +++----- .../legacy/oonidatamodel/oonidatamodel.go | 67 +++------ .../oonidatamodel/oonidatamodel_test.go | 74 +--------- .../legacy/oonitemplates/oonitemplates.go | 40 ------ .../oonitemplates/oonitemplates_test.go | 19 --- internal/engine/netx/archival/archival.go | 41 ++---- internal/engine/netx/errorx/errorx.go | 27 +--- internal/engine/netx/errorx/errorx_test.go | 14 +- internal/engine/netx/quicdialer/dns.go | 2 - .../engine/netx/quicdialer/errorwrapper.go | 5 - .../netx/quicdialer/errorwrapper_test.go | 8 +- internal/engine/netx/resolver/emitter.go | 18 +-- internal/engine/netx/resolver/emitter_test.go | 29 ---- internal/engine/netx/resolver/errorwrapper.go | 12 +- .../engine/netx/resolver/errorwrapper_test.go | 13 -- internal/engine/netx/tlsdialer/tls.go | 10 +- internal/engine/netx/tlsdialer/tls_test.go | 9 -- 32 files changed, 78 insertions(+), 896 deletions(-) delete mode 100644 internal/engine/legacy/netx/connid/connid.go delete mode 100644 internal/engine/legacy/netx/connid/connid_test.go delete mode 100644 internal/engine/legacy/netx/dialid/dialid.go delete mode 100644 internal/engine/legacy/netx/dialid/dialid_test.go delete mode 100644 internal/engine/legacy/netx/oldhttptransport/transactioner.go delete mode 100644 internal/engine/legacy/netx/oldhttptransport/transactioner_test.go delete mode 100644 internal/engine/legacy/netx/transactionid/transactionid.go delete mode 100644 internal/engine/legacy/netx/transactionid/transactionid_test.go diff --git a/internal/engine/experiment/hhfm/hhfm_test.go b/internal/engine/experiment/hhfm/hhfm_test.go index 3915670..3a5ca87 100644 --- a/internal/engine/experiment/hhfm/hhfm_test.go +++ b/internal/engine/experiment/hhfm/hhfm_test.go @@ -112,9 +112,6 @@ func TestSuccess(t *testing.T) { if request.T != 0 { t.Fatal("invalid Requests[0].T") } - if request.TransactionID != 0 { - t.Fatal("invalid Requests[0].TransactionID") - } if tk.SOCKSProxy != nil { t.Fatal("invalid SOCKSProxy") } @@ -223,9 +220,6 @@ func TestCancelledContext(t *testing.T) { if request.T != 0 { t.Fatal("invalid Requests[0].T") } - if request.TransactionID != 0 { - t.Fatal("invalid Requests[0].TransactionID") - } if tk.SOCKSProxy != nil { t.Fatal("invalid SOCKSProxy") } diff --git a/internal/engine/legacy/netx/connid/connid.go b/internal/engine/legacy/netx/connid/connid.go deleted file mode 100644 index 5d37ba7..0000000 --- a/internal/engine/legacy/netx/connid/connid.go +++ /dev/null @@ -1,31 +0,0 @@ -// Package connid contains code to generate the connectionID -package connid - -import ( - "net" - "strconv" - "strings" -) - -// Compute computes the connectionID from the local socket address. The zero -// value is conventionally returned to mean "unknown". -func Compute(network, address string) int64 { - _, portstring, err := net.SplitHostPort(address) - if err != nil { - return 0 - } - portnum, err := strconv.Atoi(portstring) - if err != nil { - return 0 - } - if portnum < 0 || portnum > 65535 { - return 0 - } - result := int64(portnum) - if strings.Contains(network, "udp") { - result *= -1 - } else if !strings.Contains(network, "tcp") { - result = 0 - } - return result -} diff --git a/internal/engine/legacy/netx/connid/connid_test.go b/internal/engine/legacy/netx/connid/connid_test.go deleted file mode 100644 index 1f0f984..0000000 --- a/internal/engine/legacy/netx/connid/connid_test.go +++ /dev/null @@ -1,80 +0,0 @@ -package connid - -import "testing" - -func TestTCP(t *testing.T) { - num := Compute("tcp", "1.2.3.4:6789") - if num != 6789 { - t.Fatal("unexpected result") - } -} - -func TestTCP4(t *testing.T) { - num := Compute("tcp4", "130.192.91.211:34566") - if num != 34566 { - t.Fatal("unexpected result") - } -} - -func TestTCP6(t *testing.T) { - num := Compute("tcp4", "[::1]:4444") - if num != 4444 { - t.Fatal("unexpected result") - } -} - -func TestUDP(t *testing.T) { - num := Compute("udp", "1.2.3.4:6789") - if num != -6789 { - t.Fatal("unexpected result") - } -} - -func TestUDP4(t *testing.T) { - num := Compute("udp4", "130.192.91.211:34566") - if num != -34566 { - t.Fatal("unexpected result") - } -} - -func TestUDP6(t *testing.T) { - num := Compute("udp6", "[::1]:4444") - if num != -4444 { - t.Fatal("unexpected result") - } -} - -func TestInvalidAddress(t *testing.T) { - num := Compute("udp6", "[::1]") - if num != 0 { - t.Fatal("unexpected result") - } -} - -func TestInvalidPort(t *testing.T) { - num := Compute("udp6", "[::1]:antani") - if num != 0 { - t.Fatal("unexpected result") - } -} - -func TestNegativePort(t *testing.T) { - num := Compute("udp6", "[::1]:-1") - if num != 0 { - t.Fatal("unexpected result") - } -} - -func TestLargePort(t *testing.T) { - num := Compute("udp6", "[::1]:65536") - if num != 0 { - t.Fatal("unexpected result") - } -} - -func TestInvalidNetwork(t *testing.T) { - num := Compute("unix", "[::1]:65531") - if num != 0 { - t.Fatal("unexpected result") - } -} diff --git a/internal/engine/legacy/netx/dialid/dialid.go b/internal/engine/legacy/netx/dialid/dialid.go deleted file mode 100644 index 6c5185b..0000000 --- a/internal/engine/legacy/netx/dialid/dialid.go +++ /dev/null @@ -1,24 +0,0 @@ -package dialid - -import ( - "context" - - "github.com/ooni/probe-cli/v3/internal/atomicx" -) - -type contextkey struct{} - -var id = &atomicx.Int64{} - -// WithDialID returns a copy of ctx with DialID -func WithDialID(ctx context.Context) context.Context { - return context.WithValue( - ctx, contextkey{}, id.Add(1), - ) -} - -// ContextDialID returns the DialID of the context, or zero -func ContextDialID(ctx context.Context) int64 { - id, _ := ctx.Value(contextkey{}).(int64) - return id -} diff --git a/internal/engine/legacy/netx/dialid/dialid_test.go b/internal/engine/legacy/netx/dialid/dialid_test.go deleted file mode 100644 index 12c788c..0000000 --- a/internal/engine/legacy/netx/dialid/dialid_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package dialid - -import ( - "context" - "testing" -) - -func TestGood(t *testing.T) { - ctx := context.Background() - id := ContextDialID(ctx) - if id != 0 { - t.Fatal("unexpected ID for empty context") - } - ctx = WithDialID(ctx) - id = ContextDialID(ctx) - if id != 1 { - t.Fatal("expected ID equal to 1") - } - ctx = WithDialID(ctx) - id = ContextDialID(ctx) - if id != 2 { - t.Fatal("expected ID equal to 2") - } -} diff --git a/internal/engine/legacy/netx/emitterdialer.go b/internal/engine/legacy/netx/emitterdialer.go index 93f2bd4..f005051 100644 --- a/internal/engine/legacy/netx/emitterdialer.go +++ b/internal/engine/legacy/netx/emitterdialer.go @@ -5,10 +5,7 @@ import ( "net" "time" - "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/dialer" ) @@ -25,14 +22,11 @@ func (d EmitterDialer) DialContext(ctx context.Context, network, address string) root := modelx.ContextMeasurementRootOrDefault(ctx) root.Handler.OnMeasurement(modelx.Measurement{ Connect: &modelx.ConnectEvent{ - ConnID: safeConnID(network, conn), - DialID: dialid.ContextDialID(ctx), DurationSinceBeginning: stop.Sub(root.Beginning), Error: err, Network: network, RemoteAddress: address, SyscallDuration: stop.Sub(start), - TransactionID: transactionid.ContextTransactionID(ctx), }, }) if err != nil { @@ -42,7 +36,6 @@ func (d EmitterDialer) DialContext(ctx context.Context, network, address string) Conn: conn, Beginning: root.Beginning, Handler: root.Handler, - ID: safeConnID(network, conn), }, nil } @@ -51,7 +44,6 @@ type EmitterConn struct { net.Conn Beginning time.Time Handler modelx.Handler - ID int64 } // Read implements net.Conn.Read @@ -61,7 +53,6 @@ func (c EmitterConn) Read(b []byte) (n int, err error) { stop := time.Now() c.Handler.OnMeasurement(modelx.Measurement{ Read: &modelx.ReadEvent{ - ConnID: c.ID, DurationSinceBeginning: stop.Sub(c.Beginning), Error: err, NumBytes: int64(n), @@ -78,7 +69,6 @@ func (c EmitterConn) Write(b []byte) (n int, err error) { stop := time.Now() c.Handler.OnMeasurement(modelx.Measurement{ Write: &modelx.WriteEvent{ - ConnID: c.ID, DurationSinceBeginning: stop.Sub(c.Beginning), Error: err, NumBytes: int64(n), @@ -95,7 +85,6 @@ func (c EmitterConn) Close() (err error) { stop := time.Now() c.Handler.OnMeasurement(modelx.Measurement{ Close: &modelx.CloseEvent{ - ConnID: c.ID, DurationSinceBeginning: stop.Sub(c.Beginning), Error: err, SyscallDuration: stop.Sub(start), @@ -103,14 +92,3 @@ func (c EmitterConn) Close() (err error) { }) return } - -func safeLocalAddress(conn net.Conn) (s string) { - if conn != nil && conn.LocalAddr() != nil { - s = conn.LocalAddr().String() - } - return -} - -func safeConnID(network string, conn net.Conn) int64 { - return connid.Compute(network, safeLocalAddress(conn)) -} diff --git a/internal/engine/legacy/netx/emitterdialer_test.go b/internal/engine/legacy/netx/emitterdialer_test.go index 3bbf657..0ba64c7 100644 --- a/internal/engine/legacy/netx/emitterdialer_test.go +++ b/internal/engine/legacy/netx/emitterdialer_test.go @@ -8,21 +8,18 @@ import ( "testing" "time" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/dialid" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/handlers" "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/mockablex" ) func TestEmitterFailure(t *testing.T) { - ctx := dialid.WithDialID(context.Background()) + ctx := context.Background() saver := &handlers.SavingHandler{} ctx = modelx.WithMeasurementRoot(ctx, &modelx.MeasurementRoot{ Beginning: time.Now(), Handler: saver, }) - ctx = transactionid.WithTransactionID(ctx) d := EmitterDialer{Dialer: mockablex.Dialer{ MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { return nil, io.EOF @@ -43,17 +40,11 @@ func TestEmitterFailure(t *testing.T) { t.Fatal("expected non nil Connect") } conninfo := events[0].Connect - if conninfo.ConnID != 0 { - t.Fatal("unexpected ConnID value") - } emitterCheckConnectEventCommon(t, conninfo, io.EOF) } func emitterCheckConnectEventCommon( t *testing.T, conninfo *modelx.ConnectEvent, err error) { - if conninfo.DialID == 0 { - t.Fatal("unexpected DialID value") - } if conninfo.DurationSinceBeginning == 0 { t.Fatal("unexpected DurationSinceBeginning value") } @@ -69,19 +60,15 @@ func emitterCheckConnectEventCommon( if conninfo.SyscallDuration == 0 { t.Fatal("unexpected SyscallDuration value") } - if conninfo.TransactionID == 0 { - t.Fatal("unexpected TransactionID value") - } } func TestEmitterSuccess(t *testing.T) { - ctx := dialid.WithDialID(context.Background()) + ctx := context.Background() saver := &handlers.SavingHandler{} ctx = modelx.WithMeasurementRoot(ctx, &modelx.MeasurementRoot{ Beginning: time.Now(), Handler: saver, }) - ctx = transactionid.WithTransactionID(ctx) d := EmitterDialer{Dialer: mockablex.Dialer{ MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { return &mockablex.Conn{ @@ -118,9 +105,6 @@ func TestEmitterSuccess(t *testing.T) { t.Fatal("expected non nil Connect") } conninfo := events[0].Connect - if conninfo.ConnID == 0 { - t.Fatal("unexpected ConnID value") - } emitterCheckConnectEventCommon(t, conninfo, nil) if events[1].Read == nil { t.Fatal("expected non nil Read") @@ -137,9 +121,6 @@ func TestEmitterSuccess(t *testing.T) { } func emitterCheckReadEvent(t *testing.T, ev *modelx.ReadEvent) { - if ev.ConnID == 0 { - t.Fatal("unexpected ConnID") - } if ev.DurationSinceBeginning == 0 { t.Fatal("unexpected DurationSinceBeginning") } @@ -155,9 +136,6 @@ func emitterCheckReadEvent(t *testing.T, ev *modelx.ReadEvent) { } func emitterCheckWriteEvent(t *testing.T, ev *modelx.WriteEvent) { - if ev.ConnID == 0 { - t.Fatal("unexpected ConnID") - } if ev.DurationSinceBeginning == 0 { t.Fatal("unexpected DurationSinceBeginning") } @@ -173,9 +151,6 @@ func emitterCheckWriteEvent(t *testing.T, ev *modelx.WriteEvent) { } func emitterCheckCloseEvent(t *testing.T, ev *modelx.CloseEvent) { - if ev.ConnID == 0 { - t.Fatal("unexpected ConnID") - } if ev.DurationSinceBeginning == 0 { t.Fatal("unexpected DurationSinceBeginning") } diff --git a/internal/engine/legacy/netx/modelx/modelx.go b/internal/engine/legacy/netx/modelx/modelx.go index c0f595d..995d007 100644 --- a/internal/engine/legacy/netx/modelx/modelx.go +++ b/internal/engine/legacy/netx/modelx/modelx.go @@ -22,9 +22,6 @@ import ( // uses a monotonic clock and is relative to a preconfigured "zero". type Measurement struct { // DNS events - // - // These are all identifed by a DialID. A ResolveEvent optionally has - // a reference to the TransactionID that started the dial, if any. ResolveStart *ResolveStartEvent `json:",omitempty"` DNSQuery *DNSQueryEvent `json:",omitempty"` DNSReply *DNSReplyEvent `json:",omitempty"` @@ -32,9 +29,6 @@ type Measurement struct { // Syscalls // - // These are all identified by a ConnID. A ConnectEvent has a reference - // to the DialID that caused this connection to be attempted. - // // Because they are syscalls, we don't split them in start/done pairs // but we record the amount of time in which we were blocked. Connect *ConnectEvent `json:",omitempty"` @@ -43,11 +37,6 @@ type Measurement struct { Close *CloseEvent `json:",omitempty"` // TLS events - // - // Identified by either ConnID or TransactionID. In the former case - // the TLS handshake is managed by net code, in the latter case it is - // instead managed by Golang's HTTP engine. It should not happen to - // have both ConnID and TransactionID different from zero. TLSHandshakeStart *TLSHandshakeStartEvent `json:",omitempty"` TLSHandshakeDone *TLSHandshakeDoneEvent `json:",omitempty"` @@ -55,10 +44,6 @@ type Measurement struct { // // A round trip starts when we need a connection to send a request // and ends when we've got the response headers or an error. - // - // The identifer here is TransactionID, where the transaction is - // like the round trip except that it terminates when we've finished - // reading the whole response body. HTTPRoundTripStart *HTTPRoundTripStartEvent `json:",omitempty"` HTTPConnectionReady *HTTPConnectionReadyEvent `json:",omitempty"` HTTPRequestHeader *HTTPRequestHeaderEvent `json:",omitempty"` @@ -68,10 +53,6 @@ type Measurement struct { HTTPRoundTripDone *HTTPRoundTripDoneEvent `json:",omitempty"` // HTTP body events - // - // They are identified by the TransactionID. You are not going to see - // these events if you don't fully read response bodies. But that's - // something you are supposed to do, so you should be fine. HTTPResponseBodyPart *HTTPResponseBodyPartEvent `json:",omitempty"` HTTPResponseDone *HTTPResponseDoneEvent `json:",omitempty"` @@ -86,9 +67,6 @@ type Measurement struct { // CloseEvent is emitted when the CLOSE syscall returns. type CloseEvent struct { - // ConnID is the identifier of this connection. - ConnID int64 - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration @@ -103,13 +81,6 @@ type CloseEvent struct { // ConnectEvent is emitted when the CONNECT syscall returns. type ConnectEvent struct { - // ConnID is the identifier of this connection. - ConnID int64 - - // DialID is the identifier of the dial operation as - // part of which we called CONNECT. - DialID int64 - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration @@ -126,10 +97,6 @@ type ConnectEvent struct { // SyscallDuration is the number of nanoseconds we were // blocked waiting for the syscall to return. SyscallDuration time.Duration - - // TransactionID is the ID of the HTTP transaction that caused the - // current dial to run, or zero if there's no such transaction. - TransactionID int64 `json:",omitempty"` } // DNSQueryEvent is emitted when we send a DNS query. @@ -137,10 +104,6 @@ type DNSQueryEvent struct { // Data is the raw data we're sending to the server. Data []byte - // DialID is the identifier of the dial operation as - // part of which we're sending this query. - DialID int64 - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration @@ -155,10 +118,6 @@ type DNSReplyEvent struct { // Data is the raw data we've received and parsed. Data []byte - // DialID is the identifier of the dial operation as - // part of which we've received this query. - DialID int64 - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration @@ -180,10 +139,6 @@ type ExtensionEvent struct { // Severity of the emitted message ("WARN", "INFO", "DEBUG") Severity string - // TransactionID is the identifier of this transaction, provided - // that we have an active one, otherwise is zero. - TransactionID int64 - // Value is the extension dependent message. This message // has the only requirement of being JSON serializable. Value interface{} @@ -196,12 +151,6 @@ type ExtensionEvent struct { // "transaction" here starts with this event and does not finish // until we have also finished receiving the response body. type HTTPRoundTripStartEvent struct { - // DialID is the identifier of the dial operation that - // caused this round trip to start. Typically, this occures - // when doing DoH. If zero, means that this round trip has - // not been started by any dial operation. - DialID int64 `json:",omitempty"` - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration @@ -209,9 +158,6 @@ type HTTPRoundTripStartEvent struct { // Method is the request method Method string - // TransactionID is the identifier of this transaction - TransactionID int64 - // URL is the request URL URL string } @@ -219,16 +165,9 @@ type HTTPRoundTripStartEvent struct { // HTTPConnectionReadyEvent is emitted when the HTTP transport has got // a connection which is ready for sending the request. type HTTPConnectionReadyEvent struct { - // ConnID is the identifier of the connection that is ready. Knowing - // this ID allows you to bind HTTP events to net events. - ConnID int64 - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration - - // TransactionID is the identifier of this transaction - TransactionID int64 } // HTTPRequestHeaderEvent is emitted when we have written a header, @@ -241,9 +180,6 @@ type HTTPRequestHeaderEvent struct { // Key is the header key Key string - // TransactionID is the identifier of this transaction - TransactionID int64 - // Value is the value/values of this header. Value []string } @@ -264,9 +200,6 @@ type HTTPRequestHeadersDoneEvent struct { // for the same reason of Headers. Method string - // TransactionID is the identifier of this transaction - TransactionID int64 - // URL is the original request URL. This is here // for the same reason of Headers. We use an object // rather than a string, because here you want to @@ -287,9 +220,6 @@ type HTTPRequestDoneEvent struct { // as well. This error however tells you that the issue was // when sending the request, not when receiving the response. Error error - - // TransactionID is the identifier of this transaction - TransactionID int64 } // HTTPResponseStartEvent is emitted when we receive the byte from @@ -298,9 +228,6 @@ type HTTPResponseStartEvent struct { // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration - - // TransactionID is the identifier of this transaction - TransactionID int64 } const defaultBodySnapSize int64 = 1 << 20 @@ -368,9 +295,6 @@ type HTTPRoundTripDoneEvent struct { // MaxBodySnapSize is the maximum size of the bodies snapshot. MaxBodySnapSize int64 - - // TransactionID is the identifier of this transaction - TransactionID int64 } // HTTPResponseBodyPartEvent is emitted after we have received @@ -393,9 +317,6 @@ type HTTPResponseBodyPartEvent struct { // Data is a reference to the body we've just read. Data []byte - - // TransactionID is the identifier of this transaction - TransactionID int64 } // HTTPResponseDoneEvent is emitted after we have received the body, @@ -407,16 +328,10 @@ type HTTPResponseDoneEvent struct { // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration - - // TransactionID is the identifier of this transaction - TransactionID int64 } // ReadEvent is emitted when the READ/RECV syscall returns. type ReadEvent struct { - // ConnID is the identifier of this connection. - ConnID int64 - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration @@ -435,10 +350,6 @@ type ReadEvent struct { // ResolveStartEvent is emitted when we start resolving a domain name. type ResolveStartEvent struct { - // DialID is the identifier of the dial operation as - // part of which we're resolving this domain. - DialID int64 - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration @@ -446,10 +357,6 @@ type ResolveStartEvent struct { // Hostname is the domain name to resolve. Hostname string - // TransactionID is the ID of the HTTP transaction that caused the - // current dial to run, or zero if there's no such transaction. - TransactionID int64 `json:",omitempty"` - // TransportNetwork is the network used by the DNS transport, which // can be one of "doh", "dot", "tcp", "udp", or "system". TransportNetwork string @@ -469,10 +376,6 @@ type ResolveDoneEvent struct { // or more IP addresses that classify as bogons. ContainsBogons bool - // DialID is the identifier of the dial operation as - // part of which we're resolving this domain. - DialID int64 - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration @@ -483,10 +386,6 @@ type ResolveDoneEvent struct { // Hostname is the domain name to resolve. Hostname string - // TransactionID is the ID of the HTTP transaction that caused the - // current dial to run, or zero if there's no such transaction. - TransactionID int64 `json:",omitempty"` - // TransportNetwork is the network used by the DNS transport, which // can be one of "doh", "dot", "tcp", "udp", or "system". TransportNetwork string @@ -532,24 +431,12 @@ func SimplifyCerts(in []*x509.Certificate) (out []X509Certificate) { // TLSHandshakeStartEvent is emitted when the TLS handshake starts. type TLSHandshakeStartEvent struct { - // ConnID is the ID of the connection that started the TLS - // handshake, or zero if we don't know it. Typically, it is - // zero for connections managed by the HTTP transport, for - // which we know instead the TransactionID. - ConnID int64 `json:",omitempty"` - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration // SNI is the SNI used when we force a specific SNI. SNI string - - // TransactionID is the ID of the transaction that started - // this TLS handshake, or zero if we don't know it. Typically, - // it is zero for explicit dials, and it's nonzero instead - // when a connection is managed by HTTP code. - TransactionID int64 `json:",omitempty"` } // TLSHandshakeDoneEvent is emitted when conn.Handshake returns. @@ -558,31 +445,16 @@ type TLSHandshakeDoneEvent struct { // error type, some fields may have little meaning. ConnectionState TLSConnectionState - // ConnID is the ID of the connection that started the TLS - // handshake, or zero if we don't know it. Typically, it is - // zero for connections managed by the HTTP transport, for - // which we know instead the TransactionID. - ConnID int64 - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration // Error is the result of the TLS handshake. Error error - - // TransactionID is the ID of the transaction that started - // this TLS handshake, or zero if we don't know it. Typically, - // it is zero for explicit dials, and it's nonzero instead - // when a connection is managed by HTTP code. - TransactionID int64 } // WriteEvent is emitted when the WRITE/SEND syscall returns. type WriteEvent struct { - // ConnID is the identifier of this connection. - ConnID int64 - // DurationSinceBeginning is the number of nanoseconds since // the time configured as the "zero" time. DurationSinceBeginning time.Duration diff --git a/internal/engine/legacy/netx/oldhttptransport/bodytracer.go b/internal/engine/legacy/netx/oldhttptransport/bodytracer.go index 8d39628..44695b2 100644 --- a/internal/engine/legacy/netx/oldhttptransport/bodytracer.go +++ b/internal/engine/legacy/netx/oldhttptransport/bodytracer.go @@ -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 diff --git a/internal/engine/legacy/netx/oldhttptransport/httptransport.go b/internal/engine/legacy/netx/oldhttptransport/httptransport.go index 4ee50be..40d7058 100644 --- a/internal/engine/legacy/netx/oldhttptransport/httptransport.go +++ b/internal/engine/legacy/netx/oldhttptransport/httptransport.go @@ -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)), } } diff --git a/internal/engine/legacy/netx/oldhttptransport/tracetripper.go b/internal/engine/legacy/netx/oldhttptransport/tracetripper.go index ff88aa6..87c8313 100644 --- a/internal/engine/legacy/netx/oldhttptransport/tracetripper.go +++ b/internal/engine/legacy/netx/oldhttptransport/tracetripper.go @@ -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 diff --git a/internal/engine/legacy/netx/oldhttptransport/transactioner.go b/internal/engine/legacy/netx/oldhttptransport/transactioner.go deleted file mode 100644 index 4e9c5a1..0000000 --- a/internal/engine/legacy/netx/oldhttptransport/transactioner.go +++ /dev/null @@ -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() - } -} diff --git a/internal/engine/legacy/netx/oldhttptransport/transactioner_test.go b/internal/engine/legacy/netx/oldhttptransport/transactioner_test.go deleted file mode 100644 index d7804fe..0000000 --- a/internal/engine/legacy/netx/oldhttptransport/transactioner_test.go +++ /dev/null @@ -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() -} diff --git a/internal/engine/legacy/netx/transactionid/transactionid.go b/internal/engine/legacy/netx/transactionid/transactionid.go deleted file mode 100644 index 6fab055..0000000 --- a/internal/engine/legacy/netx/transactionid/transactionid.go +++ /dev/null @@ -1,25 +0,0 @@ -// Package transactionid contains code to share the transactionID -package transactionid - -import ( - "context" - - "github.com/ooni/probe-cli/v3/internal/atomicx" -) - -type contextkey struct{} - -var id = &atomicx.Int64{} - -// WithTransactionID returns a copy of ctx with TransactionID -func WithTransactionID(ctx context.Context) context.Context { - return context.WithValue( - ctx, contextkey{}, id.Add(1), - ) -} - -// ContextTransactionID returns the TransactionID of the context, or zero -func ContextTransactionID(ctx context.Context) int64 { - id, _ := ctx.Value(contextkey{}).(int64) - return id -} diff --git a/internal/engine/legacy/netx/transactionid/transactionid_test.go b/internal/engine/legacy/netx/transactionid/transactionid_test.go deleted file mode 100644 index 26e8889..0000000 --- a/internal/engine/legacy/netx/transactionid/transactionid_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package transactionid - -import ( - "context" - "testing" -) - -func TestGood(t *testing.T) { - ctx := context.Background() - id := ContextTransactionID(ctx) - if id != 0 { - t.Fatal("unexpected ID for empty context") - } - ctx = WithTransactionID(ctx) - id = ContextTransactionID(ctx) - if id != 1 { - t.Fatal("expected ID equal to 1") - } - ctx = WithTransactionID(ctx) - id = ContextTransactionID(ctx) - if id != 2 { - t.Fatal("expected ID equal to 2") - } -} diff --git a/internal/engine/legacy/netxlogger/netxlogger.go b/internal/engine/legacy/netxlogger/netxlogger.go index be31ec2..7ef14bd 100644 --- a/internal/engine/legacy/netxlogger/netxlogger.go +++ b/internal/engine/legacy/netxlogger/netxlogger.go @@ -33,15 +33,13 @@ func (h *Handler) OnMeasurement(m modelx.Measurement) { // DNS if m.ResolveStart != nil { h.logger.Debugf( - "[httpTxID: %d] resolving: %s", - m.ResolveStart.TransactionID, + "resolving: %s", m.ResolveStart.Hostname, ) } if m.ResolveDone != nil { h.logger.Debugf( - "[httpTxID: %d] resolve done: %s, %s", - m.ResolveDone.TransactionID, + "resolve done: %s, %s", fmtError(m.ResolveDone.Error), m.ResolveDone.Addresses, ) @@ -50,8 +48,7 @@ func (h *Handler) OnMeasurement(m modelx.Measurement) { // Syscalls if m.Connect != nil { h.logger.Debugf( - "[httpTxID: %d] connect done: %s, %s (rtt=%s)", - m.Connect.TransactionID, + "connect done: %s, %s (rtt=%s)", fmtError(m.Connect.Error), m.Connect.RemoteAddress, m.Connect.SyscallDuration, @@ -61,15 +58,13 @@ func (h *Handler) OnMeasurement(m modelx.Measurement) { // TLS if m.TLSHandshakeStart != nil { h.logger.Debugf( - "[httpTxID: %d] TLS handshake: (forceSNI='%s')", - m.TLSHandshakeStart.TransactionID, + "TLS handshake: (forceSNI='%s')", m.TLSHandshakeStart.SNI, ) } if m.TLSHandshakeDone != nil { h.logger.Debugf( - "[httpTxID: %d] TLS done: %s, %s (alpn='%s')", - m.TLSHandshakeDone.TransactionID, + "TLS done: %s, %s (alpn='%s')", fmtError(m.TLSHandshakeDone.Error), tlsx.VersionString(m.TLSHandshakeDone.ConnectionState.Version), m.TLSHandshakeDone.ConnectionState.NegotiatedProtocol, @@ -86,16 +81,14 @@ func (h *Handler) OnMeasurement(m modelx.Measurement) { } } h.logger.Debugf( - "[httpTxID: %d] > %s %s %s", - m.HTTPRequestHeadersDone.TransactionID, + "> %s %s %s", m.HTTPRequestHeadersDone.Method, m.HTTPRequestHeadersDone.URL.RequestURI(), proto, ) if proto == "HTTP/2.0" { h.logger.Debugf( - "[httpTxID: %d] > Host: %s", - m.HTTPRequestHeadersDone.TransactionID, + "> Host: %s", m.HTTPRequestHeadersDone.URL.Host, ) } @@ -105,31 +98,22 @@ func (h *Handler) OnMeasurement(m modelx.Measurement) { } for _, value := range values { h.logger.Debugf( - "[httpTxID: %d] > %s: %s", - m.HTTPRequestHeadersDone.TransactionID, + "> %s: %s", key, value, ) } } - h.logger.Debugf( - "[httpTxID: %d] >", m.HTTPRequestHeadersDone.TransactionID) + h.logger.Debug(">") } if m.HTTPRequestDone != nil { - h.logger.Debugf( - "[httpTxID: %d] request sent; waiting for response", - m.HTTPRequestDone.TransactionID, - ) + h.logger.Debug("request sent; waiting for response") } if m.HTTPResponseStart != nil { - h.logger.Debugf( - "[httpTxID: %d] start receiving response", - m.HTTPResponseStart.TransactionID, - ) + h.logger.Debug("start receiving response") } if m.HTTPRoundTripDone != nil && m.HTTPRoundTripDone.Error == nil { h.logger.Debugf( - "[httpTxID: %d] < %s %d %s", - m.HTTPRoundTripDone.TransactionID, + "< %s %d %s", m.HTTPRoundTripDone.ResponseProto, m.HTTPRoundTripDone.ResponseStatusCode, http.StatusText(int(m.HTTPRoundTripDone.ResponseStatusCode)), @@ -137,29 +121,25 @@ func (h *Handler) OnMeasurement(m modelx.Measurement) { for key, values := range m.HTTPRoundTripDone.ResponseHeaders { for _, value := range values { h.logger.Debugf( - "[httpTxID: %d] < %s: %s", - m.HTTPRoundTripDone.TransactionID, + "< %s: %s", key, value, ) } } - h.logger.Debugf( - "[httpTxID: %d] <", m.HTTPRoundTripDone.TransactionID) + h.logger.Debug("<") } // HTTP response body if m.HTTPResponseBodyPart != nil { h.logger.Debugf( - "[httpTxID: %d] body part: %s, %d", - m.HTTPResponseBodyPart.TransactionID, + "body part: %s, %d", fmtError(m.HTTPResponseBodyPart.Error), len(m.HTTPResponseBodyPart.Data), ) } if m.HTTPResponseDone != nil { - h.logger.Debugf( - "[httpTxID: %d] end of response", - m.HTTPResponseDone.TransactionID, + h.logger.Debug( + "end of response", ) } } diff --git a/internal/engine/legacy/oonidatamodel/oonidatamodel.go b/internal/engine/legacy/oonidatamodel/oonidatamodel.go index 1026391..aacc549 100644 --- a/internal/engine/legacy/oonidatamodel/oonidatamodel.go +++ b/internal/engine/legacy/oonidatamodel/oonidatamodel.go @@ -63,13 +63,10 @@ type TCPConnectStatus struct { // TCPConnectEntry contains one of the entries that are part // of the "tcp_connect" key of a OONI report. type TCPConnectEntry struct { - ConnID int64 `json:"conn_id,omitempty"` - DialID int64 `json:"dial_id,omitempty"` - IP string `json:"ip"` - Port int `json:"port"` - Status TCPConnectStatus `json:"status"` - T float64 `json:"t"` - TransactionID int64 `json:"transaction_id,omitempty"` + IP string `json:"ip"` + Port int `json:"port"` + Status TCPConnectStatus `json:"status"` + T float64 `json:"t"` } // TCPConnectList is a list of TCPConnectEntry @@ -83,16 +80,13 @@ func NewTCPConnectList(results oonitemplates.Results) TCPConnectList { ip, sport, _ := net.SplitHostPort(connect.RemoteAddress) iport, _ := strconv.Atoi(sport) out = append(out, TCPConnectEntry{ - ConnID: connect.ConnID, - DialID: connect.DialID, - IP: ip, - Port: iport, + IP: ip, + Port: iport, Status: TCPConnectStatus{ Failure: makeFailure(connect.Error), Success: connect.Error == nil, }, - T: connect.DurationSinceBeginning.Seconds(), - TransactionID: connect.TransactionID, + T: connect.DurationSinceBeginning.Seconds(), }) } return out @@ -259,10 +253,9 @@ type HTTPResponse struct { // RequestEntry is one of the entries that are part of // the "requests" key of a OONI report. type RequestEntry struct { - Failure *string `json:"failure"` - Request HTTPRequest `json:"request"` - Response HTTPResponse `json:"response"` - TransactionID int64 `json:"transaction_id,omitempty"` + Failure *string `json:"failure"` + Request HTTPRequest `json:"request"` + Response HTTPResponse `json:"response"` } // RequestList is a list of RequestEntry @@ -316,7 +309,6 @@ func NewRequestList(results oonitemplates.Results) RequestList { entry.Response.Body.Value = string(in[idx].ResponseBodySnap) entry.Response.BodyIsTruncated = in[idx].MaxBodySnapSize > 0 && int64(len(in[idx].ResponseBodySnap)) >= in[idx].MaxBodySnapSize - entry.TransactionID = in[idx].TransactionID out = append(out, entry) } return out @@ -334,7 +326,6 @@ type DNSAnswerEntry struct { // DNSQueryEntry is a DNS query with possibly an answer type DNSQueryEntry struct { Answers []DNSAnswerEntry `json:"answers"` - DialID int64 `json:"dial_id,omitempty"` Engine string `json:"engine"` Failure *string `json:"failure"` Hostname string `json:"hostname"` @@ -343,7 +334,6 @@ type DNSQueryEntry struct { ResolverPort *string `json:"resolver_port"` ResolverAddress string `json:"resolver_address"` T float64 `json:"t"` - TransactionID int64 `json:"transaction_id,omitempty"` } type ( @@ -393,28 +383,23 @@ func (qtype dnsQueryType) makeanswerentry(addr string) DNSAnswerEntry { func (qtype dnsQueryType) makequeryentry(resolve *modelx.ResolveDoneEvent) DNSQueryEntry { return DNSQueryEntry{ - DialID: resolve.DialID, Engine: resolve.TransportNetwork, Failure: makeFailure(resolve.Error), Hostname: resolve.Hostname, QueryType: string(qtype), ResolverAddress: resolve.TransportAddress, T: resolve.DurationSinceBeginning.Seconds(), - TransactionID: resolve.TransactionID, } } // NetworkEvent is a network event. type NetworkEvent struct { - Address string `json:"address,omitempty"` - ConnID int64 `json:"conn_id,omitempty"` - DialID int64 `json:"dial_id,omitempty"` - Failure *string `json:"failure"` - NumBytes int64 `json:"num_bytes,omitempty"` - Operation string `json:"operation"` - Proto string `json:"proto"` - T float64 `json:"t"` - TransactionID int64 `json:"transaction_id,omitempty"` + Address string `json:"address,omitempty"` + Failure *string `json:"failure"` + NumBytes int64 `json:"num_bytes,omitempty"` + Operation string `json:"operation"` + Proto string `json:"proto"` + T float64 `json:"t"` } // NetworkEventsList is a list of network events. @@ -431,35 +416,27 @@ func NewNetworkEventsList(results oonitemplates.Results) NetworkEventsList { for _, in := range results.NetworkEvents { if in.Connect != nil { out = append(out, &NetworkEvent{ - Address: in.Connect.RemoteAddress, - ConnID: in.Connect.ConnID, - DialID: in.Connect.DialID, - Failure: makeFailure(in.Connect.Error), - Operation: errorx.ConnectOperation, - Proto: protocolName[in.Connect.ConnID >= 0], - T: in.Connect.DurationSinceBeginning.Seconds(), - TransactionID: in.Connect.TransactionID, + Address: in.Connect.RemoteAddress, + Failure: makeFailure(in.Connect.Error), + Operation: errorx.ConnectOperation, + T: in.Connect.DurationSinceBeginning.Seconds(), }) // fallthrough } if in.Read != nil { out = append(out, &NetworkEvent{ - ConnID: in.Read.ConnID, Failure: makeFailure(in.Read.Error), Operation: errorx.ReadOperation, NumBytes: in.Read.NumBytes, - Proto: protocolName[in.Read.ConnID >= 0], T: in.Read.DurationSinceBeginning.Seconds(), }) // fallthrough } if in.Write != nil { out = append(out, &NetworkEvent{ - ConnID: in.Write.ConnID, Failure: makeFailure(in.Write.Error), Operation: errorx.WriteOperation, NumBytes: in.Write.NumBytes, - Proto: protocolName[in.Write.ConnID >= 0], T: in.Write.DurationSinceBeginning.Seconds(), }) // fallthrough @@ -471,13 +448,11 @@ func NewNetworkEventsList(results oonitemplates.Results) NetworkEventsList { // TLSHandshake contains TLS handshake data type TLSHandshake struct { CipherSuite string `json:"cipher_suite"` - ConnID int64 `json:"conn_id,omitempty"` Failure *string `json:"failure"` NegotiatedProtocol string `json:"negotiated_protocol"` PeerCertificates []MaybeBinaryValue `json:"peer_certificates"` T float64 `json:"t"` TLSVersion string `json:"tls_version"` - TransactionID int64 `json:"transaction_id,omitempty"` } // TLSHandshakesList is a list of TLS handshakes @@ -489,13 +464,11 @@ func NewTLSHandshakesList(results oonitemplates.Results) TLSHandshakesList { for _, in := range results.TLSHandshakes { out = append(out, TLSHandshake{ CipherSuite: tlsx.CipherSuiteString(in.ConnectionState.CipherSuite), - ConnID: in.ConnID, Failure: makeFailure(in.Error), NegotiatedProtocol: in.ConnectionState.NegotiatedProtocol, PeerCertificates: makePeerCerts(in.ConnectionState.PeerCertificates), T: in.DurationSinceBeginning.Seconds(), TLSVersion: tlsx.VersionString(in.ConnectionState.Version), - TransactionID: in.TransactionID, }) } return out diff --git a/internal/engine/legacy/oonidatamodel/oonidatamodel_test.go b/internal/engine/legacy/oonidatamodel/oonidatamodel_test.go index e9f2339..0435d17 100644 --- a/internal/engine/legacy/oonidatamodel/oonidatamodel_test.go +++ b/internal/engine/legacy/oonidatamodel/oonidatamodel_test.go @@ -823,22 +823,18 @@ func TestNewNetworkEventsListGood(t *testing.T) { NetworkEvents: []*modelx.Measurement{ { Connect: &modelx.ConnectEvent{ - ConnID: 555, DurationSinceBeginning: 10 * time.Millisecond, - DialID: 17, RemoteAddress: "1.1.1.1:443", }, }, { Read: &modelx.ReadEvent{ - ConnID: 555, DurationSinceBeginning: 20 * time.Millisecond, NumBytes: 1789, }, }, { Write: &modelx.WriteEvent{ - ConnID: 555, DurationSinceBeginning: 30 * time.Millisecond, NumBytes: 17714, }, @@ -852,12 +848,6 @@ func TestNewNetworkEventsListGood(t *testing.T) { if out[0].Address != "1.1.1.1:443" { t.Fatal("wrong out[0].Address") } - if out[0].ConnID != 555 { - t.Fatal("wrong out[0].ConnID") - } - if out[0].DialID != 17 { - t.Fatal("wrong out[0].DialID") - } if out[0].Failure != nil { t.Fatal("wrong out[0].Failure") } @@ -867,9 +857,6 @@ func TestNewNetworkEventsListGood(t *testing.T) { if out[0].Operation != errorx.ConnectOperation { t.Fatal("wrong out[0].Operation") } - if out[0].Proto != "tcp" { - t.Fatal("wrong out[0].Proto") - } if !floatEquals(out[0].T, 0.010) { t.Fatal("wrong out[0].T") } @@ -877,12 +864,6 @@ func TestNewNetworkEventsListGood(t *testing.T) { if out[1].Address != "" { t.Fatal("wrong out[1].Address") } - if out[1].ConnID != 555 { - t.Fatal("wrong out[1].ConnID") - } - if out[1].DialID != 0 { - t.Fatal("wrong out[1].DialID") - } if out[1].Failure != nil { t.Fatal("wrong out[1].Failure") } @@ -892,9 +873,6 @@ func TestNewNetworkEventsListGood(t *testing.T) { if out[1].Operation != errorx.ReadOperation { t.Fatal("wrong out[1].Operation") } - if out[1].Proto != "tcp" { - t.Fatal("wrong out[1].Proto") - } if !floatEquals(out[1].T, 0.020) { t.Fatal("wrong out[1].T") } @@ -902,12 +880,6 @@ func TestNewNetworkEventsListGood(t *testing.T) { if out[2].Address != "" { t.Fatal("wrong out[2].Address") } - if out[2].ConnID != 555 { - t.Fatal("wrong out[2].ConnID") - } - if out[2].DialID != 0 { - t.Fatal("wrong out[2].DialID") - } if out[2].Failure != nil { t.Fatal("wrong out[2].Failure") } @@ -917,9 +889,6 @@ func TestNewNetworkEventsListGood(t *testing.T) { if out[2].Operation != errorx.WriteOperation { t.Fatal("wrong out[2].Operation") } - if out[2].Proto != "tcp" { - t.Fatal("wrong out[2].Proto") - } if !floatEquals(out[2].T, 0.030) { t.Fatal("wrong out[2].T") } @@ -930,16 +899,13 @@ func TestNewNetworkEventsListGoodUDPAndErrors(t *testing.T) { NetworkEvents: []*modelx.Measurement{ { Connect: &modelx.ConnectEvent{ - ConnID: -555, DurationSinceBeginning: 10 * time.Millisecond, - DialID: 17, Error: errors.New("mocked error"), RemoteAddress: "1.1.1.1:443", }, }, { Read: &modelx.ReadEvent{ - ConnID: -555, DurationSinceBeginning: 20 * time.Millisecond, Error: errors.New("mocked error"), NumBytes: 1789, @@ -947,7 +913,6 @@ func TestNewNetworkEventsListGoodUDPAndErrors(t *testing.T) { }, { Write: &modelx.WriteEvent{ - ConnID: -555, DurationSinceBeginning: 30 * time.Millisecond, Error: errors.New("mocked error"), NumBytes: 17714, @@ -962,12 +927,6 @@ func TestNewNetworkEventsListGoodUDPAndErrors(t *testing.T) { if out[0].Address != "1.1.1.1:443" { t.Fatal("wrong out[0].Address") } - if out[0].ConnID != -555 { - t.Fatal("wrong out[0].ConnID") - } - if out[0].DialID != 17 { - t.Fatal("wrong out[0].DialID") - } if *out[0].Failure != "mocked error" { t.Fatal("wrong out[0].Failure") } @@ -977,9 +936,6 @@ func TestNewNetworkEventsListGoodUDPAndErrors(t *testing.T) { if out[0].Operation != errorx.ConnectOperation { t.Fatal("wrong out[0].Operation") } - if out[0].Proto != "udp" { - t.Fatal("wrong out[0].Proto") - } if !floatEquals(out[0].T, 0.010) { t.Fatal("wrong out[0].T") } @@ -987,12 +943,6 @@ func TestNewNetworkEventsListGoodUDPAndErrors(t *testing.T) { if out[1].Address != "" { t.Fatal("wrong out[1].Address") } - if out[1].ConnID != -555 { - t.Fatal("wrong out[1].ConnID") - } - if out[1].DialID != 0 { - t.Fatal("wrong out[1].DialID") - } if *out[1].Failure != "mocked error" { t.Fatal("wrong out[1].Failure") } @@ -1002,9 +952,6 @@ func TestNewNetworkEventsListGoodUDPAndErrors(t *testing.T) { if out[1].Operation != errorx.ReadOperation { t.Fatal("wrong out[1].Operation") } - if out[1].Proto != "udp" { - t.Fatal("wrong out[1].Proto") - } if !floatEquals(out[1].T, 0.020) { t.Fatal("wrong out[1].T") } @@ -1012,12 +959,6 @@ func TestNewNetworkEventsListGoodUDPAndErrors(t *testing.T) { if out[2].Address != "" { t.Fatal("wrong out[2].Address") } - if out[2].ConnID != -555 { - t.Fatal("wrong out[2].ConnID") - } - if out[2].DialID != 0 { - t.Fatal("wrong out[2].DialID") - } if *out[2].Failure != "mocked error" { t.Fatal("wrong out[2].Failure") } @@ -1027,9 +968,6 @@ func TestNewNetworkEventsListGoodUDPAndErrors(t *testing.T) { if out[2].Operation != errorx.WriteOperation { t.Fatal("wrong out[2].Operation") } - if out[2].Proto != "udp" { - t.Fatal("wrong out[2].Proto") - } if !floatEquals(out[2].T, 0.030) { t.Fatal("wrong out[2].T") } @@ -1052,8 +990,7 @@ func TestNewTLSHandshakesListSuccess(t *testing.T) { TLSHandshakes: []*modelx.TLSHandshakeDoneEvent{ {}, { - ConnID: 12345, - Error: errors.New("mocked error"), + Error: errors.New("mocked error"), }, { ConnectionState: modelx.TLSConnectionState{ @@ -1080,9 +1017,6 @@ func TestNewTLSHandshakesListSuccess(t *testing.T) { if out[0].CipherSuite != "" { t.Fatal("invalid out[0].CipherSuite") } - if out[0].ConnID != 0 { - t.Fatal("invalid out[0].ConnID") - } if out[0].Failure != nil { t.Fatal("invalid out[0].Failure") } @@ -1102,9 +1036,6 @@ func TestNewTLSHandshakesListSuccess(t *testing.T) { if out[1].CipherSuite != "" { t.Fatal("invalid out[1].CipherSuite") } - if out[1].ConnID != 12345 { - t.Fatal("invalid out[1].ConnID") - } if *out[1].Failure != "mocked error" { t.Fatal("invalid out[1].Failure") } @@ -1124,9 +1055,6 @@ func TestNewTLSHandshakesListSuccess(t *testing.T) { if out[2].CipherSuite != "TLS_AES_128_GCM_SHA256" { t.Fatal("invalid out[2].CipherSuite") } - if out[2].ConnID != 0 { - t.Fatal("invalid out[2].ConnID") - } if out[2].Failure != nil { t.Fatal("invalid out[2].Failure") } diff --git a/internal/engine/legacy/oonitemplates/oonitemplates.go b/internal/engine/legacy/oonitemplates/oonitemplates.go index 3be2291..432c014 100644 --- a/internal/engine/legacy/oonitemplates/oonitemplates.go +++ b/internal/engine/legacy/oonitemplates/oonitemplates.go @@ -71,45 +71,8 @@ type Results struct { TLSHandshakes []*modelx.TLSHandshakeDoneEvent } -type connmapper struct { - counter int64 - mu sync.Mutex - once sync.Once - table map[int64]int64 -} - -// scramble maps a ConnID to a different number to avoid emitting -// the port numbers. We preserve the sign because it's used to -// distinguish between TCP (positive) and UDP (negative). A special -// case is zero, which is always mapped to zero, since the zero -// port means "unspecified" in netx code. -func (m *connmapper) scramble(cid int64) int64 { - m.once.Do(func() { - m.table = make(map[int64]int64) - m.table[0] = 0 // means unspecified in netx - }) - // See https://stackoverflow.com/a/38140573/4354461 - m.mu.Lock() - defer m.mu.Unlock() - if value, found := m.table[cid]; found { - return value - } - var factor int64 = 1 - if cid < 0 { - factor = -1 - } - m.counter++ // we must never emit zero - value := factor * m.counter - m.table[cid] = value - return value -} - -// cm is the global connmapper -var cm connmapper - func (r *Results) onMeasurement(m modelx.Measurement, lowLevel bool) { if m.Connect != nil { - m.Connect.ConnID = cm.scramble(m.Connect.ConnID) r.Connects = append(r.Connects, m.Connect) if lowLevel { r.NetworkEvents = append(r.NetworkEvents, &m) @@ -122,17 +85,14 @@ func (r *Results) onMeasurement(m modelx.Measurement, lowLevel bool) { r.Resolves = append(r.Resolves, m.ResolveDone) } if m.TLSHandshakeDone != nil { - m.TLSHandshakeDone.ConnID = cm.scramble(m.TLSHandshakeDone.ConnID) r.TLSHandshakes = append(r.TLSHandshakes, m.TLSHandshakeDone) } if m.Read != nil { - m.Read.ConnID = cm.scramble(m.Read.ConnID) if lowLevel { r.NetworkEvents = append(r.NetworkEvents, &m) } } if m.Write != nil { - m.Write.ConnID = cm.scramble(m.Write.ConnID) if lowLevel { r.NetworkEvents = append(r.NetworkEvents, &m) } diff --git a/internal/engine/legacy/oonitemplates/oonitemplates_test.go b/internal/engine/legacy/oonitemplates/oonitemplates_test.go index ae78760..98b6136 100644 --- a/internal/engine/legacy/oonitemplates/oonitemplates_test.go +++ b/internal/engine/legacy/oonitemplates/oonitemplates_test.go @@ -387,22 +387,3 @@ func (txp *faketransport) ClientFactory(stateDir string) (obfs4base.ClientFactor func (txp *faketransport) ServerFactory(stateDir string, args *goptlib.Args) (obfs4base.ServerFactory, error) { return txp.txp.ServerFactory(stateDir, args) } - -func TestConnmapper(t *testing.T) { - var mapper connmapper - if mapper.scramble(-1) >= 0 { - t.Fatal("unexpected value for negative input") - } - if mapper.scramble(1234) != 2 { - t.Fatal("unexpected second value") - } - if mapper.scramble(12) != 3 { - t.Fatal("unexpected third value") - } - if mapper.scramble(12) != mapper.scramble(12) { - t.Fatal("not idempotent") - } - if mapper.scramble(0) != 0 { - t.Fatal("unexpected value for zero input") - } -} diff --git a/internal/engine/netx/archival/archival.go b/internal/engine/netx/archival/archival.go index 11fd776..b50a8d0 100644 --- a/internal/engine/netx/archival/archival.go +++ b/internal/engine/netx/archival/archival.go @@ -70,13 +70,10 @@ type TCPConnectStatus struct { // TCPConnectEntry contains one of the entries that are part // of the "tcp_connect" key of a OONI report. type TCPConnectEntry struct { - ConnID int64 `json:"conn_id,omitempty"` - DialID int64 `json:"dial_id,omitempty"` - IP string `json:"ip"` - Port int `json:"port"` - Status TCPConnectStatus `json:"status"` - T float64 `json:"t"` - TransactionID int64 `json:"transaction_id,omitempty"` + IP string `json:"ip"` + Port int `json:"port"` + Status TCPConnectStatus `json:"status"` + T float64 `json:"t"` } // NewTCPConnectList creates a new TCPConnectList @@ -289,11 +286,10 @@ type HTTPResponse struct { // RequestEntry is one of the entries that are part of // the "requests" key of a OONI report. type RequestEntry struct { - Failure *string `json:"failure"` - Request HTTPRequest `json:"request"` - Response HTTPResponse `json:"response"` - T float64 `json:"t"` - TransactionID int64 `json:"transaction_id,omitempty"` + Failure *string `json:"failure"` + Request HTTPRequest `json:"request"` + Response HTTPResponse `json:"response"` + T float64 `json:"t"` } func addheaders( @@ -382,7 +378,6 @@ type DNSAnswerEntry struct { // DNSQueryEntry is a DNS query with possibly an answer type DNSQueryEntry struct { Answers []DNSAnswerEntry `json:"answers"` - DialID int64 `json:"dial_id,omitempty"` Engine string `json:"engine"` Failure *string `json:"failure"` Hostname string `json:"hostname"` @@ -391,7 +386,6 @@ type DNSQueryEntry struct { ResolverPort *string `json:"resolver_port"` ResolverAddress string `json:"resolver_address"` T float64 `json:"t"` - TransactionID int64 `json:"transaction_id,omitempty"` } type dnsQueryType string @@ -467,16 +461,13 @@ func (qtype dnsQueryType) makequeryentry(begin time.Time, ev trace.Event) DNSQue // and most fields are optional. They are only added when it makes sense // for them to be there _and_ we have data to show. type NetworkEvent struct { - Address string `json:"address,omitempty"` - ConnID int64 `json:"conn_id,omitempty"` - DialID int64 `json:"dial_id,omitempty"` - Failure *string `json:"failure"` - NumBytes int64 `json:"num_bytes,omitempty"` - Operation string `json:"operation"` - Proto string `json:"proto,omitempty"` - T float64 `json:"t"` - Tags []string `json:"tags,omitempty"` - TransactionID int64 `json:"transaction_id,omitempty"` + Address string `json:"address,omitempty"` + Failure *string `json:"failure"` + NumBytes int64 `json:"num_bytes,omitempty"` + Operation string `json:"operation"` + Proto string `json:"proto,omitempty"` + T float64 `json:"t"` + Tags []string `json:"tags,omitempty"` } // NewNetworkEventsList returns a list of DNS queries. @@ -543,7 +534,6 @@ func NewNetworkEventsList(begin time.Time, events []trace.Event) []NetworkEvent // TLSHandshake contains TLS handshake data type TLSHandshake struct { CipherSuite string `json:"cipher_suite"` - ConnID int64 `json:"conn_id,omitempty"` Failure *string `json:"failure"` NegotiatedProtocol string `json:"negotiated_protocol"` NoTLSVerify bool `json:"no_tls_verify"` @@ -552,7 +542,6 @@ type TLSHandshake struct { T float64 `json:"t"` Tags []string `json:"tags"` TLSVersion string `json:"tls_version"` - TransactionID int64 `json:"transaction_id,omitempty"` } // NewTLSHandshakesList creates a new TLSHandshakesList diff --git a/internal/engine/netx/errorx/errorx.go b/internal/engine/netx/errorx/errorx.go index b2ac052..f71cc92 100644 --- a/internal/engine/netx/errorx/errorx.go +++ b/internal/engine/netx/errorx/errorx.go @@ -143,12 +143,6 @@ var ErrDNSBogon = errors.New("dns: detected bogon address") // this structure is to properly set Failure, which is also returned by // the Error() method, so be one of the OONI defined strings. type ErrWrapper struct { - // ConnID is the connection ID, or zero if not known. - ConnID int64 - - // DialID is the dial ID, or zero if not known. - DialID int64 - // Failure is the OONI failure string. The failure strings are // loosely backward compatible with Measurement Kit. // @@ -181,9 +175,6 @@ type ErrWrapper struct { // supposed to refer to the major operation that failed. Operation string - // TransactionID is the transaction ID, or zero if not known. - TransactionID int64 - // WrappedErr is the error that we're wrapping. WrappedErr error } @@ -201,12 +192,6 @@ func (e *ErrWrapper) Unwrap() error { // SafeErrWrapperBuilder contains a builder for ErrWrapper that // is safe, i.e., behaves correctly when the error is nil. type SafeErrWrapperBuilder struct { - // ConnID is the connection ID, if any - ConnID int64 - - // DialID is the dial ID, if any - DialID int64 - // Error is the error, if any Error error @@ -216,9 +201,6 @@ type SafeErrWrapperBuilder struct { // Operation is the operation that failed Operation string - - // TransactionID is the transaction ID, if any - TransactionID int64 } // MaybeBuild builds a new ErrWrapper, if b.Error is not nil, and returns @@ -230,12 +212,9 @@ func (b SafeErrWrapperBuilder) MaybeBuild() (err error) { classifier = toFailureString } err = &ErrWrapper{ - ConnID: b.ConnID, - DialID: b.DialID, - Failure: classifier(b.Error), - Operation: toOperationString(b.Error, b.Operation), - TransactionID: b.TransactionID, - WrappedErr: b.Error, + Failure: classifier(b.Error), + Operation: toOperationString(b.Error, b.Operation), + WrappedErr: b.Error, } } return diff --git a/internal/engine/netx/errorx/errorx_test.go b/internal/engine/netx/errorx/errorx_test.go index e9d37e1..08de2e0 100644 --- a/internal/engine/netx/errorx/errorx_test.go +++ b/internal/engine/netx/errorx/errorx_test.go @@ -17,27 +17,15 @@ import ( func TestMaybeBuildFactory(t *testing.T) { err := SafeErrWrapperBuilder{ - ConnID: 1, - DialID: 10, - Error: errors.New("mocked error"), - TransactionID: 100, + Error: errors.New("mocked error"), }.MaybeBuild() var target *ErrWrapper if errors.As(err, &target) == false { t.Fatal("not the expected error type") } - if target.ConnID != 1 { - t.Fatal("wrong ConnID") - } - if target.DialID != 10 { - t.Fatal("wrong DialID") - } if target.Failure != "unknown_failure: mocked error" { t.Fatal("the failure string is wrong") } - if target.TransactionID != 100 { - t.Fatal("the transactionID is wrong") - } if target.WrappedErr.Error() != "mocked error" { t.Fatal("the wrapped error is wrong") } diff --git a/internal/engine/netx/quicdialer/dns.go b/internal/engine/netx/quicdialer/dns.go index 7402ddf..5f390e2 100644 --- a/internal/engine/netx/quicdialer/dns.go +++ b/internal/engine/netx/quicdialer/dns.go @@ -6,7 +6,6 @@ import ( "net" "github.com/lucas-clemente/quic-go" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/dialid" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" ) @@ -30,7 +29,6 @@ func (d DNSDialer) DialContext( if tlsCfg.ServerName == "" { tlsCfg.ServerName = onlyhost } - ctx = dialid.WithDialID(ctx) var addrs []string addrs, err = d.LookupHost(ctx, onlyhost) if err != nil { diff --git a/internal/engine/netx/quicdialer/errorwrapper.go b/internal/engine/netx/quicdialer/errorwrapper.go index 181b4e8..0c69e4a 100644 --- a/internal/engine/netx/quicdialer/errorwrapper.go +++ b/internal/engine/netx/quicdialer/errorwrapper.go @@ -5,7 +5,6 @@ import ( "crypto/tls" "github.com/lucas-clemente/quic-go" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/dialid" "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" ) @@ -18,13 +17,9 @@ type ErrorWrapperDialer struct { func (d ErrorWrapperDialer) DialContext( ctx context.Context, network string, host string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) { - dialID := dialid.ContextDialID(ctx) sess, err := d.Dialer.DialContext(ctx, network, host, tlsCfg, cfg) err = errorx.SafeErrWrapperBuilder{ - // ConnID does not make any sense if we've failed and the error - // does not make any sense (and is nil) if we succeeded. Classifier: errorx.ClassifyQUICFailure, - DialID: dialID, Error: err, Operation: errorx.QUICHandshakeOperation, }.MaybeBuild() diff --git a/internal/engine/netx/quicdialer/errorwrapper_test.go b/internal/engine/netx/quicdialer/errorwrapper_test.go index 663492d..fe360b7 100644 --- a/internal/engine/netx/quicdialer/errorwrapper_test.go +++ b/internal/engine/netx/quicdialer/errorwrapper_test.go @@ -8,13 +8,12 @@ import ( "testing" "github.com/lucas-clemente/quic-go" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/dialid" "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" "github.com/ooni/probe-cli/v3/internal/engine/netx/quicdialer" ) func TestErrorWrapperFailure(t *testing.T) { - ctx := dialid.WithDialID(context.Background()) + ctx := context.Background() d := quicdialer.ErrorWrapperDialer{ Dialer: MockDialer{Sess: nil, Err: io.EOF}} sess, err := d.DialContext( @@ -33,9 +32,6 @@ func errorWrapperCheckErr(t *testing.T, err error, op string) { if !errors.As(err, &errWrapper) { t.Fatal("cannot cast to ErrWrapper") } - if errWrapper.DialID == 0 { - t.Fatal("unexpected DialID") - } if errWrapper.Operation != op { t.Fatal("unexpected Operation") } @@ -68,7 +64,7 @@ func TestErrorWrapperInvalidCertificate(t *testing.T) { } func TestErrorWrapperSuccess(t *testing.T) { - ctx := dialid.WithDialID(context.Background()) + ctx := context.Background() tlsConf := &tls.Config{ NextProtos: []string{"h3"}, ServerName: "www.google.com", diff --git a/internal/engine/netx/resolver/emitter.go b/internal/engine/netx/resolver/emitter.go index c4b095a..519f05d 100644 --- a/internal/engine/netx/resolver/emitter.go +++ b/internal/engine/netx/resolver/emitter.go @@ -4,9 +4,7 @@ import ( "context" "time" - "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" ) // EmitterTransport is a RoundTripper that emits events when they occur. @@ -20,8 +18,7 @@ func (txp EmitterTransport) RoundTrip(ctx context.Context, querydata []byte) ([] root.Handler.OnMeasurement(modelx.Measurement{ DNSQuery: &modelx.DNSQueryEvent{ Data: querydata, - DialID: dialid.ContextDialID(ctx), - DurationSinceBeginning: time.Now().Sub(root.Beginning), + DurationSinceBeginning: time.Since(root.Beginning), }, }) replydata, err := txp.RoundTripper.RoundTrip(ctx, querydata) @@ -31,8 +28,7 @@ func (txp EmitterTransport) RoundTrip(ctx context.Context, querydata []byte) ([] root.Handler.OnMeasurement(modelx.Measurement{ DNSReply: &modelx.DNSReplyEvent{ Data: replydata, - DialID: dialid.ContextDialID(ctx), - DurationSinceBeginning: time.Now().Sub(root.Beginning), + DurationSinceBeginning: time.Since(root.Beginning), }, }) return replydata, nil @@ -56,15 +52,11 @@ func (r EmitterResolver) LookupHost(ctx context.Context, hostname string) ([]str txp := qr.Transport() network, address = txp.Network(), txp.Address() } - dialID := dialid.ContextDialID(ctx) - txID := transactionid.ContextTransactionID(ctx) root := modelx.ContextMeasurementRootOrDefault(ctx) root.Handler.OnMeasurement(modelx.Measurement{ ResolveStart: &modelx.ResolveStartEvent{ - DialID: dialID, - DurationSinceBeginning: time.Now().Sub(root.Beginning), + DurationSinceBeginning: time.Since(root.Beginning), Hostname: hostname, - TransactionID: txID, TransportAddress: address, TransportNetwork: network, }, @@ -73,11 +65,9 @@ func (r EmitterResolver) LookupHost(ctx context.Context, hostname string) ([]str root.Handler.OnMeasurement(modelx.Measurement{ ResolveDone: &modelx.ResolveDoneEvent{ Addresses: addrs, - DialID: dialID, - DurationSinceBeginning: time.Now().Sub(root.Beginning), + DurationSinceBeginning: time.Since(root.Beginning), Error: err, Hostname: hostname, - TransactionID: txID, TransportAddress: address, TransportNetwork: network, }, diff --git a/internal/engine/netx/resolver/emitter_test.go b/internal/engine/netx/resolver/emitter_test.go index 6245d76..d275d3d 100644 --- a/internal/engine/netx/resolver/emitter_test.go +++ b/internal/engine/netx/resolver/emitter_test.go @@ -10,16 +10,13 @@ import ( "time" "github.com/miekg/dns" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/dialid" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/handlers" "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/resolver" ) func TestEmitterTransportSuccess(t *testing.T) { ctx := context.Background() - ctx = dialid.WithDialID(ctx) handler := &handlers.SavingHandler{} root := &modelx.MeasurementRoot{ Beginning: time.Now(), @@ -48,9 +45,6 @@ func TestEmitterTransportSuccess(t *testing.T) { if !bytes.Equal(events[0].DNSQuery.Data, querydata) { t.Fatal("invalid query data") } - if events[0].DNSQuery.DialID == 0 { - t.Fatal("invalid query DialID") - } if events[0].DNSQuery.DurationSinceBeginning <= 0 { t.Fatal("invalid duration since beginning") } @@ -60,9 +54,6 @@ func TestEmitterTransportSuccess(t *testing.T) { if !bytes.Equal(events[1].DNSReply.Data, replydata) { t.Fatal("missing reply data") } - if events[1].DNSReply.DialID != 1 { - t.Fatal("invalid query DialID") - } if events[1].DNSReply.DurationSinceBeginning <= 0 { t.Fatal("invalid duration since beginning") } @@ -70,7 +61,6 @@ func TestEmitterTransportSuccess(t *testing.T) { func TestEmitterTransportFailure(t *testing.T) { ctx := context.Background() - ctx = dialid.WithDialID(ctx) handler := &handlers.SavingHandler{} root := &modelx.MeasurementRoot{ Beginning: time.Now(), @@ -103,9 +93,6 @@ func TestEmitterTransportFailure(t *testing.T) { if !bytes.Equal(events[0].DNSQuery.Data, querydata) { t.Fatal("invalid query data") } - if events[0].DNSQuery.DialID == 0 { - t.Fatal("invalid query DialID") - } if events[0].DNSQuery.DurationSinceBeginning <= 0 { t.Fatal("invalid duration since beginning") } @@ -113,8 +100,6 @@ func TestEmitterTransportFailure(t *testing.T) { func TestEmitterResolverFailure(t *testing.T) { ctx := context.Background() - ctx = dialid.WithDialID(ctx) - ctx = transactionid.WithTransactionID(ctx) handler := &handlers.SavingHandler{} root := &modelx.MeasurementRoot{ Beginning: time.Now(), @@ -143,18 +128,12 @@ func TestEmitterResolverFailure(t *testing.T) { if events[0].ResolveStart == nil { t.Fatal("missing ResolveStart field") } - if events[0].ResolveStart.DialID == 0 { - t.Fatal("invalid DialID") - } if events[0].ResolveStart.DurationSinceBeginning <= 0 { t.Fatal("invalid duration since beginning") } if events[0].ResolveStart.Hostname != "www.google.com" { t.Fatal("invalid Hostname") } - if events[0].ResolveStart.TransactionID == 0 { - t.Fatal("invalid TransactionID") - } if events[0].ResolveStart.TransportAddress != "https://dns.google.com/" { t.Fatal("invalid TransportAddress") } @@ -164,9 +143,6 @@ func TestEmitterResolverFailure(t *testing.T) { if events[1].ResolveDone == nil { t.Fatal("missing ResolveDone field") } - if events[1].ResolveDone.DialID == 0 { - t.Fatal("invalid DialID") - } if events[1].ResolveDone.DurationSinceBeginning <= 0 { t.Fatal("invalid duration since beginning") } @@ -176,9 +152,6 @@ func TestEmitterResolverFailure(t *testing.T) { if events[1].ResolveDone.Hostname != "www.google.com" { t.Fatal("invalid Hostname") } - if events[1].ResolveDone.TransactionID == 0 { - t.Fatal("invalid TransactionID") - } if events[1].ResolveDone.TransportAddress != "https://dns.google.com/" { t.Fatal("invalid TransportAddress") } @@ -189,8 +162,6 @@ func TestEmitterResolverFailure(t *testing.T) { func TestEmitterResolverSuccess(t *testing.T) { ctx := context.Background() - ctx = dialid.WithDialID(ctx) - ctx = transactionid.WithTransactionID(ctx) handler := &handlers.SavingHandler{} root := &modelx.MeasurementRoot{ Beginning: time.Now(), diff --git a/internal/engine/netx/resolver/errorwrapper.go b/internal/engine/netx/resolver/errorwrapper.go index e3285b2..813c473 100644 --- a/internal/engine/netx/resolver/errorwrapper.go +++ b/internal/engine/netx/resolver/errorwrapper.go @@ -3,8 +3,6 @@ package resolver import ( "context" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/dialid" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/transactionid" "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" ) @@ -15,15 +13,11 @@ type ErrorWrapperResolver struct { // LookupHost implements Resolver.LookupHost func (r ErrorWrapperResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) { - dialID := dialid.ContextDialID(ctx) - txID := transactionid.ContextTransactionID(ctx) addrs, err := r.Resolver.LookupHost(ctx, hostname) err = errorx.SafeErrWrapperBuilder{ - Classifier: errorx.ClassifyResolveFailure, - DialID: dialID, - Error: err, - Operation: errorx.ResolveOperation, - TransactionID: txID, + Classifier: errorx.ClassifyResolveFailure, + Error: err, + Operation: errorx.ResolveOperation, }.MaybeBuild() return addrs, err } diff --git a/internal/engine/netx/resolver/errorwrapper_test.go b/internal/engine/netx/resolver/errorwrapper_test.go index 9212f42..f30692c 100644 --- a/internal/engine/netx/resolver/errorwrapper_test.go +++ b/internal/engine/netx/resolver/errorwrapper_test.go @@ -5,8 +5,6 @@ import ( "errors" "testing" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/dialid" - "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/engine/netx/resolver" ) @@ -30,8 +28,6 @@ func TestErrorWrapperFailure(t *testing.T) { Resolver: resolver.NewFakeResolverThatFails(), } ctx := context.Background() - ctx = dialid.WithDialID(ctx) - ctx = transactionid.WithTransactionID(ctx) addrs, err := r.LookupHost(ctx, "dns.google.com") if addrs != nil { t.Fatal("expected nil addr here") @@ -43,15 +39,6 @@ func TestErrorWrapperFailure(t *testing.T) { if errWrapper.Failure != errorx.FailureDNSNXDOMAINError { t.Fatal("unexpected failure") } - if errWrapper.ConnID != 0 { - t.Fatal("unexpected ConnID") - } - if errWrapper.DialID == 0 { - t.Fatal("unexpected DialID") - } - if errWrapper.TransactionID == 0 { - t.Fatal("unexpected TransactionID") - } if errWrapper.Operation != errorx.ResolveOperation { t.Fatal("unexpected Operation") } diff --git a/internal/engine/netx/tlsdialer/tls.go b/internal/engine/netx/tlsdialer/tls.go index 2171492..1fc05cf 100644 --- a/internal/engine/netx/tlsdialer/tls.go +++ b/internal/engine/netx/tlsdialer/tls.go @@ -7,7 +7,6 @@ import ( "net" "time" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/connid" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" ) @@ -68,11 +67,9 @@ type ErrorWrapperTLSHandshaker struct { func (h ErrorWrapperTLSHandshaker) Handshake( ctx context.Context, conn net.Conn, config *tls.Config, ) (net.Conn, tls.ConnectionState, error) { - connID := connid.Compute(conn.RemoteAddr().Network(), conn.RemoteAddr().String()) tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config) err = errorx.SafeErrWrapperBuilder{ Classifier: errorx.ClassifyTLSFailure, - ConnID: connID, Error: err, Operation: errorx.TLSHandshakeOperation, }.MaybeBuild() @@ -88,22 +85,19 @@ type EmitterTLSHandshaker struct { func (h EmitterTLSHandshaker) Handshake( ctx context.Context, conn net.Conn, config *tls.Config, ) (net.Conn, tls.ConnectionState, error) { - connID := connid.Compute(conn.RemoteAddr().Network(), conn.RemoteAddr().String()) root := modelx.ContextMeasurementRootOrDefault(ctx) root.Handler.OnMeasurement(modelx.Measurement{ TLSHandshakeStart: &modelx.TLSHandshakeStartEvent{ - ConnID: connID, - DurationSinceBeginning: time.Now().Sub(root.Beginning), + DurationSinceBeginning: time.Since(root.Beginning), SNI: config.ServerName, }, }) tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config) root.Handler.OnMeasurement(modelx.Measurement{ TLSHandshakeDone: &modelx.TLSHandshakeDoneEvent{ - ConnID: connID, ConnectionState: modelx.NewTLSConnectionState(state), Error: err, - DurationSinceBeginning: time.Now().Sub(root.Beginning), + DurationSinceBeginning: time.Since(root.Beginning), }, }) return tlsconn, state, err diff --git a/internal/engine/netx/tlsdialer/tls_test.go b/internal/engine/netx/tlsdialer/tls_test.go index 9e093fe..d88449a 100644 --- a/internal/engine/netx/tlsdialer/tls_test.go +++ b/internal/engine/netx/tlsdialer/tls_test.go @@ -109,9 +109,6 @@ func TestErrorWrapperTLSHandshakerFailure(t *testing.T) { if !errors.As(err, &errWrapper) { t.Fatal("cannot cast to ErrWrapper") } - if errWrapper.ConnID == 0 { - t.Fatal("unexpected ConnID") - } if errWrapper.Failure != errorx.FailureEOFError { t.Fatal("unexpected Failure") } @@ -143,9 +140,6 @@ func TestEmitterTLSHandshakerFailure(t *testing.T) { if events[0].TLSHandshakeStart == nil { t.Fatal("missing TLSHandshakeStart event") } - if events[0].TLSHandshakeStart.ConnID == 0 { - t.Fatal("expected nonzero ConnID") - } if events[0].TLSHandshakeStart.DurationSinceBeginning == 0 { t.Fatal("expected nonzero DurationSinceBeginning") } @@ -155,9 +149,6 @@ func TestEmitterTLSHandshakerFailure(t *testing.T) { if events[1].TLSHandshakeDone == nil { t.Fatal("missing TLSHandshakeDone event") } - if events[1].TLSHandshakeDone.ConnID == 0 { - t.Fatal("expected nonzero ConnID") - } if events[1].TLSHandshakeDone.DurationSinceBeginning == 0 { t.Fatal("expected nonzero DurationSinceBeginning") }