diff --git a/internal/engine/experiment/urlgetter/getter_integration_test.go b/internal/engine/experiment/urlgetter/getter_integration_test.go index b7f6717ca9..ef6846511d 100644 --- a/internal/engine/experiment/urlgetter/getter_integration_test.go +++ b/internal/engine/experiment/urlgetter/getter_integration_test.go @@ -429,9 +429,6 @@ func TestGetterIntegrationHTTPS(t *testing.T) { connect bool tlsHandshakeStart bool tlsHandshakeDone bool - httpWroteHeaders bool - httpWroteRequest bool - httpFirstResponseByte bool httpResponseMetadata bool httpResponseBodySnapshot bool httpTransactionDone bool @@ -452,12 +449,6 @@ func TestGetterIntegrationHTTPS(t *testing.T) { tlsHandshakeStart = true case "tls_handshake_done": tlsHandshakeDone = true - case "http_wrote_headers": - httpWroteHeaders = true - case "http_wrote_request": - httpWroteRequest = true - case "http_first_response_byte": - httpFirstResponseByte = true case "http_response_metadata": httpResponseMetadata = true case "http_response_body_snapshot": @@ -474,9 +465,6 @@ func TestGetterIntegrationHTTPS(t *testing.T) { ok = ok && connect ok = ok && tlsHandshakeStart ok = ok && tlsHandshakeDone - ok = ok && httpWroteHeaders - ok = ok && httpWroteRequest - ok = ok && httpFirstResponseByte ok = ok && httpResponseMetadata ok = ok && httpResponseBodySnapshot ok = ok && httpTransactionDone @@ -570,9 +558,6 @@ func TestGetterIntegrationTLSHandshake(t *testing.T) { connect bool tlsHandshakeStart bool tlsHandshakeDone bool - httpWroteHeaders bool - httpWroteRequest bool - httpFirstResponseByte bool httpResponseMetadata bool httpResponseBodySnapshot bool httpTransactionDone bool @@ -593,12 +578,6 @@ func TestGetterIntegrationTLSHandshake(t *testing.T) { tlsHandshakeStart = true case "tls_handshake_done": tlsHandshakeDone = true - case "http_wrote_headers": - httpWroteHeaders = true - case "http_wrote_request": - httpWroteRequest = true - case "http_first_response_byte": - httpFirstResponseByte = true case "http_response_metadata": httpResponseMetadata = true case "http_response_body_snapshot": @@ -615,9 +594,6 @@ func TestGetterIntegrationTLSHandshake(t *testing.T) { ok = ok && connect ok = ok && tlsHandshakeStart ok = ok && tlsHandshakeDone - ok = ok && !httpWroteHeaders - ok = ok && !httpWroteRequest - ok = ok && !httpFirstResponseByte ok = ok && !httpResponseMetadata ok = ok && !httpResponseBodySnapshot ok = ok && !httpTransactionDone @@ -688,9 +664,6 @@ func TestGetterHTTPSWithTunnel(t *testing.T) { connect bool tlsHandshakeStart bool tlsHandshakeDone bool - httpWroteHeaders bool - httpWroteRequest bool - httpFirstResponseByte bool httpResponseMetadata bool httpResponseBodySnapshot bool httpTransactionDone bool @@ -711,12 +684,6 @@ func TestGetterHTTPSWithTunnel(t *testing.T) { tlsHandshakeStart = true case "tls_handshake_done": tlsHandshakeDone = true - case "http_wrote_headers": - httpWroteHeaders = true - case "http_wrote_request": - httpWroteRequest = true - case "http_first_response_byte": - httpFirstResponseByte = true case "http_response_metadata": httpResponseMetadata = true case "http_response_body_snapshot": @@ -733,9 +700,6 @@ func TestGetterHTTPSWithTunnel(t *testing.T) { ok = ok && connect ok = ok && tlsHandshakeStart ok = ok && tlsHandshakeDone - ok = ok && httpWroteHeaders - ok = ok && httpWroteRequest - ok = ok && httpFirstResponseByte ok = ok && httpResponseMetadata ok = ok && httpResponseBodySnapshot ok = ok && httpTransactionDone diff --git a/internal/engine/netx/httptransport/saver.go b/internal/engine/netx/httptransport/saver.go index 1e23e9151d..772aac1f6b 100644 --- a/internal/engine/netx/httptransport/saver.go +++ b/internal/engine/netx/httptransport/saver.go @@ -5,7 +5,6 @@ import ( "context" "io" "net/http" - "net/http/httptrace" "time" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" @@ -13,34 +12,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/netxlite" ) -// SaverPerformanceHTTPTransport is a RoundTripper that saves -// performance events occurring during the round trip -type SaverPerformanceHTTPTransport struct { - model.HTTPTransport - Saver *trace.Saver -} - -// RoundTrip implements RoundTripper.RoundTrip -func (txp SaverPerformanceHTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) { - tracep := httptrace.ContextClientTrace(req.Context()) - if tracep == nil { - tracep = &httptrace.ClientTrace{ - WroteHeaders: func() { - txp.Saver.Write(trace.Event{Name: "http_wrote_headers", Time: time.Now()}) - }, - WroteRequest: func(httptrace.WroteRequestInfo) { - txp.Saver.Write(trace.Event{Name: "http_wrote_request", Time: time.Now()}) - }, - GotFirstResponseByte: func() { - txp.Saver.Write(trace.Event{ - Name: "http_first_response_byte", Time: time.Now()}) - }, - } - req = req.WithContext(httptrace.WithClientTrace(req.Context(), tracep)) - } - return txp.HTTPTransport.RoundTrip(req) -} - // SaverMetadataHTTPTransport is a RoundTripper that saves // events related to HTTP request and response metadata type SaverMetadataHTTPTransport struct { @@ -166,7 +137,6 @@ type saverReadCloser struct { io.Reader } -var _ model.HTTPTransport = SaverPerformanceHTTPTransport{} var _ model.HTTPTransport = SaverMetadataHTTPTransport{} var _ model.HTTPTransport = SaverBodyHTTPTransport{} var _ model.HTTPTransport = SaverTransactionHTTPTransport{} diff --git a/internal/engine/netx/httptransport/saver_test.go b/internal/engine/netx/httptransport/saver_test.go index 989bf36937..a5d45d8847 100644 --- a/internal/engine/netx/httptransport/saver_test.go +++ b/internal/engine/netx/httptransport/saver_test.go @@ -16,52 +16,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/netxlite" ) -func TestSaverPerformanceNoMultipleEvents(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - saver := &trace.Saver{} - // register twice - do we see events twice? - txp := httptransport.SaverPerformanceHTTPTransport{ - HTTPTransport: netxlite.NewHTTPTransportStdlib(model.DiscardLogger), - Saver: saver, - } - txp = httptransport.SaverPerformanceHTTPTransport{ - HTTPTransport: txp, - Saver: saver, - } - req, err := http.NewRequest("GET", "https://www.google.com", nil) - if err != nil { - t.Fatal(err) - } - resp, err := txp.RoundTrip(req) - if err != nil { - t.Fatal("not the error we expected") - } - if resp == nil { - t.Fatal("expected non nil response here") - } - ev := saver.Read() - // we should specifically see the events not attached to any - // context being submitted twice. This is fine because they are - // explicit, while the context is implicit and hence leads to - // more subtle bugs. For example, this happens when you measure - // every event and combine HTTP with DoH. - if len(ev) != 3 { - t.Fatal("expected three events") - } - expected := []string{ - "http_wrote_headers", // measured with context - "http_wrote_request", // measured with context - "http_first_response_byte", // measured with context - } - for i := 0; i < len(expected); i++ { - if ev[i].Name != expected[i] { - t.Fatal("unexpected event name") - } - } -} - func TestSaverMetadataSuccess(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index d0222c7b4f..c40d78e683 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -213,8 +213,6 @@ func NewHTTPTransport(config Config) model.HTTPTransport { HTTPTransport: txp, Saver: config.HTTPSaver} txp = httptransport.SaverBodyHTTPTransport{ HTTPTransport: txp, Saver: config.HTTPSaver} - txp = httptransport.SaverPerformanceHTTPTransport{ - HTTPTransport: txp, Saver: config.HTTPSaver} txp = httptransport.SaverTransactionHTTPTransport{ HTTPTransport: txp, Saver: config.HTTPSaver} } diff --git a/internal/engine/netx/netx_test.go b/internal/engine/netx/netx_test.go index 0fc35fe46f..9ab809ab3c 100644 --- a/internal/engine/netx/netx_test.go +++ b/internal/engine/netx/netx_test.go @@ -513,14 +513,7 @@ func TestNewWithSaver(t *testing.T) { if stxptxp.Saver != saver { t.Fatal("not the logger we expected") } - sptxp, ok := stxptxp.HTTPTransport.(httptransport.SaverPerformanceHTTPTransport) - if !ok { - t.Fatal("not the transport we expected") - } - if sptxp.Saver != saver { - t.Fatal("not the logger we expected") - } - sbtxp, ok := sptxp.HTTPTransport.(httptransport.SaverBodyHTTPTransport) + sbtxp, ok := stxptxp.HTTPTransport.(httptransport.SaverBodyHTTPTransport) if !ok { t.Fatal("not the transport we expected") }