diff --git a/internal/experiment/webconnectivitylte/cleartextflow.go b/internal/experiment/webconnectivitylte/cleartextflow.go index 7c94d8f34d..cbff7e4143 100644 --- a/internal/experiment/webconnectivitylte/cleartextflow.go +++ b/internal/experiment/webconnectivitylte/cleartextflow.go @@ -100,6 +100,10 @@ func (t *CleartextFlow) Run(parentCtx context.Context, index int64) error { // create trace trace := measurexlite.NewTrace(index, t.ZeroTime) + // TODO(bassosimone): the DSL starts measuring for throttling when we start + // fetching the body while here we start immediately. We should come up with + // a consistent policy otherwise data won't be comparable. + // start measuring throttling sampler := throttling.NewSampler(trace) defer func() { @@ -119,7 +123,7 @@ func (t *CleartextFlow) Run(parentCtx context.Context, index int64) error { tcpDialer := trace.NewDialerWithoutResolver(t.Logger) tcpConn, err := tcpDialer.DialContext(tcpCtx, "tcp", t.Address) t.TestKeys.AppendTCPConnectResults(trace.TCPConnects()...) - defer t.TestKeys.AppendNetworkEvents(trace.NetworkEvents()...) // here to include connect events + defer t.TestKeys.AppendNetworkEvents(trace.NetworkEvents()...) // here to include "connect" events if err != nil { ol.Stop(err) return err @@ -241,6 +245,12 @@ func (t *CleartextFlow) httpTransaction(ctx context.Context, network, address, a txp model.HTTPTransport, req *http.Request, trace *measurexlite.Trace) (*http.Response, []byte, error) { const maxbody = 1 << 19 started := trace.TimeSince(trace.ZeroTime()) + // TODO(bassosimone): I am wondering whether we should have the HTTP transaction + // start at the beginning of the flow rather than here. If we start it at the + // beginning this is nicer, but, at the same time, starting it at the beginning + // of the flow means we're not collecting information about DNS. So, I am a + // bit torn about what is the best approach to follow here. Maybe it does not + // even matter to emit transaction_start/end events given that we have transaction ID. t.TestKeys.AppendNetworkEvents(measurexlite.NewAnnotationArchivalNetworkEvent( trace.Index(), started, "http_transaction_start", )) @@ -287,6 +297,8 @@ func (t *CleartextFlow) maybeFollowRedirects(ctx context.Context, resp *http.Res if err != nil { return // broken response from server } + // TODO(https://github.com/ooni/probe/issues/2628): we need to handle + // the case where the redirect URL is incomplete t.Logger.Infof("redirect to: %s", location.String()) resolvers := &DNSResolvers{ CookieJar: t.CookieJar, diff --git a/internal/experiment/webconnectivitylte/secureflow.go b/internal/experiment/webconnectivitylte/secureflow.go index ee71654084..8b4f129790 100644 --- a/internal/experiment/webconnectivitylte/secureflow.go +++ b/internal/experiment/webconnectivitylte/secureflow.go @@ -107,6 +107,10 @@ func (t *SecureFlow) Run(parentCtx context.Context, index int64) error { // create trace trace := measurexlite.NewTrace(index, t.ZeroTime) + // TODO(bassosimone): the DSL starts measuring for throttling when we start + // fetching the body while here we start immediately. We should come up with + // a consistent policy otherwise data won't be comparable. + // start measuring throttling sampler := throttling.NewSampler(trace) defer func() { @@ -296,6 +300,12 @@ func (t *SecureFlow) httpTransaction(ctx context.Context, network, address, alpn txp model.HTTPTransport, req *http.Request, trace *measurexlite.Trace) (*http.Response, []byte, error) { const maxbody = 1 << 19 started := trace.TimeSince(trace.ZeroTime()) + // TODO(bassosimone): I am wondering whether we should have the HTTP transaction + // start at the beginning of the flow rather than here. If we start it at the + // beginning this is nicer, but, at the same time, starting it at the beginning + // of the flow means we're not collecting information about DNS. So, I am a + // bit torn about what is the best approach to follow here. Maybe it does not + // even matter to emit transaction_start/end events given that we have transaction ID. t.TestKeys.AppendNetworkEvents(measurexlite.NewAnnotationArchivalNetworkEvent( trace.Index(), started, "http_transaction_start", )) @@ -342,6 +352,8 @@ func (t *SecureFlow) maybeFollowRedirects(ctx context.Context, resp *http.Respon if err != nil { return // broken response from server } + // TODO(https://github.com/ooni/probe/issues/2628): we need to handle + // the case where the redirect URL is incomplete t.Logger.Infof("redirect to: %s", location.String()) resolvers := &DNSResolvers{ CookieJar: t.CookieJar,