Skip to content

Commit

Permalink
fix(webconnectivitylte): use scope for endpoint IDs (#1507)
Browse files Browse the repository at this point in the history
We use scope for endpoint IDs. This helps to reduce churn when running
`./script/updateminipipeline.bash`.

Part of ooni/probe#2677
  • Loading branch information
bassosimone committed Feb 12, 2024
1 parent 80b1681 commit 165a9ac
Show file tree
Hide file tree
Showing 243 changed files with 12,898 additions and 13,118 deletions.
5 changes: 2 additions & 3 deletions internal/experiment/webconnectivitylte/cleartextflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"net/http"
"net/url"
"sync"
"sync/atomic"
"time"

"github.com/ooni/probe-cli/v3/internal/logx"
Expand All @@ -39,7 +38,7 @@ type CleartextFlow struct {
Depth int64

// IDGenerator is the MANDATORY atomic int64 to generate task IDs.
IDGenerator *atomic.Int64
IDGenerator *IDGenerator

// Logger is the MANDATORY logger to use.
Logger model.Logger
Expand Down Expand Up @@ -87,7 +86,7 @@ type CleartextFlow struct {
// Start starts this task in a background goroutine.
func (t *CleartextFlow) Start(ctx context.Context) {
t.WaitGroup.Add(1)
index := t.IDGenerator.Add(1)
index := t.IDGenerator.NewIDForEndpointCleartext()
go func() {
defer t.WaitGroup.Done() // synchronize with the parent
t.Run(ctx, index)
Expand Down
3 changes: 1 addition & 2 deletions internal/experiment/webconnectivitylte/cleartextflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"errors"
"net/http"
"sync"
"sync/atomic"
"testing"
"time"

Expand All @@ -16,7 +15,7 @@ func TestCleartextFlow_Run(t *testing.T) {
type fields struct {
Address string
DNSCache *DNSCache
IDGenerator *atomic.Int64
IDGenerator *IDGenerator
Logger model.Logger
NumRedirects *NumRedirects
TestKeys *TestKeys
Expand Down
9 changes: 4 additions & 5 deletions internal/experiment/webconnectivitylte/dnsresolvers.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"net/http"
"net/url"
"sync"
"sync/atomic"
"time"

"github.com/ooni/probe-cli/v3/internal/logx"
Expand All @@ -38,7 +37,7 @@ type DNSResolvers struct {
Depth int64

// IDGenerator is the MANDATORY atomic int64 to generate task IDs.
IDGenerator *atomic.Int64
IDGenerator *IDGenerator

// Logger is the MANDATORY logger to use.
Logger model.Logger
Expand Down Expand Up @@ -209,7 +208,7 @@ func (t *DNSResolvers) lookupHostSystem(parentCtx context.Context, out chan<- []
defer lookpCancel()

// create trace's index
index := t.IDGenerator.Add(1)
index := t.IDGenerator.NewIDForGetaddrinfo()

// create trace
trace := measurexlite.NewTrace(index, t.ZeroTime, fmt.Sprintf("depth=%d", t.Depth))
Expand All @@ -236,7 +235,7 @@ func (t *DNSResolvers) lookupHostUDP(parentCtx context.Context, udpAddress strin
defer lookpCancel()

// create trace's index
index := t.IDGenerator.Add(1)
index := t.IDGenerator.NewIDForDNSOverUDP()

// create trace
trace := measurexlite.NewTrace(index, t.ZeroTime, fmt.Sprintf("depth=%d", t.Depth))
Expand Down Expand Up @@ -325,7 +324,7 @@ func (t *DNSResolvers) lookupHostDNSOverHTTPS(parentCtx context.Context, out cha
defer lookpCancel()

// create trace's index
index := t.IDGenerator.Add(1)
index := t.IDGenerator.NewIDForDNSOverHTTPS()

// create trace
trace := measurexlite.NewTrace(index, t.ZeroTime, fmt.Sprintf("depth=%d", t.Depth))
Expand Down
67 changes: 67 additions & 0 deletions internal/experiment/webconnectivitylte/idgenerator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package webconnectivitylte

import "sync/atomic"

const (
idGeneratorGetaddrinfoOffset = 0
idGeneratorDNSOverUDPOffset = 10_000
idGeneratorDNSOverHTTPSOffset = 20_000
idGeneratorEndpointCleartextOffset = 30_000
idGeneratorEndpointSecureOffset = 40_000
)

// IDGenerator helps with generating IDs that neatly fall into namespaces.
//
// The zero value is invalid, please use [NewIDGenerator].
type IDGenerator struct {
// getaddrinfo generates IDs for getaddrinfo.
getaddrinfo *atomic.Int64

// dnsOverUDP generates IDs for DNS-over-UDP lookups.
dnsOverUDP *atomic.Int64

// dnsOverHTTPS generates IDs for DNS-over-HTTPS lookups.
dnsOverHTTPS *atomic.Int64

// endpointCleartext generates IDs for endpoints using HTTP.
endpointCleartext *atomic.Int64

// endpointSecure generates IDs for endpoints using HTTPS.
endpointSecure *atomic.Int64
}

// NewIDGenerator creates a new [*IDGenerator] instance.
func NewIDGenerator() *IDGenerator {
return &IDGenerator{
getaddrinfo: &atomic.Int64{},
dnsOverUDP: &atomic.Int64{},
dnsOverHTTPS: &atomic.Int64{},
endpointCleartext: &atomic.Int64{},
endpointSecure: &atomic.Int64{},
}
}

// NewIDForGetaddrinfo returns a new ID for a getaddrinfo lookup.
func (idgen *IDGenerator) NewIDForGetaddrinfo() int64 {
return idgen.getaddrinfo.Add(1) + idGeneratorGetaddrinfoOffset
}

// NewIDForDNSOverUDP returns a new ID for a DNS-over-UDP lookup.
func (idgen *IDGenerator) NewIDForDNSOverUDP() int64 {
return idgen.dnsOverUDP.Add(1) + idGeneratorDNSOverUDPOffset
}

// NewIDForDNSOverHTTPS returns a new ID for a DNS-over-HTTPS lookup.
func (idgen *IDGenerator) NewIDForDNSOverHTTPS() int64 {
return idgen.dnsOverHTTPS.Add(1) + idGeneratorDNSOverHTTPSOffset
}

// NewIDForEndpointCleartext returns a new ID for a cleartext endpoint operation.
func (idgen *IDGenerator) NewIDForEndpointCleartext() int64 {
return idgen.endpointCleartext.Add(1) + idGeneratorEndpointCleartextOffset
}

// NewIDForEndpointSecure returns a new ID for a secure endpoint operation.
func (idgen *IDGenerator) NewIDForEndpointSecure() int64 {
return idgen.endpointSecure.Add(1) + idGeneratorEndpointSecureOffset
}
4 changes: 1 addition & 3 deletions internal/experiment/webconnectivitylte/measurer.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"errors"
"net/http/cookiejar"
"sync"
"sync/atomic"

"github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity"
"github.com/ooni/probe-cli/v3/internal/inputparser"
Expand Down Expand Up @@ -84,7 +83,6 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
tk.ClientResolver = sess.ResolverIP()

// create variables required to run parallel tasks
idGenerator := &atomic.Int64{}
wg := &sync.WaitGroup{}

// create cookiejar
Expand All @@ -109,7 +107,7 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
DNSCache: NewDNSCache(),
Depth: 0,
Domain: URL.Hostname(),
IDGenerator: idGenerator,
IDGenerator: NewIDGenerator(),
Logger: sess.Logger(),
NumRedirects: NewNumRedirects(5),
TestKeys: tk,
Expand Down
5 changes: 2 additions & 3 deletions internal/experiment/webconnectivitylte/secureflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"net/http"
"net/url"
"sync"
"sync/atomic"
"time"

"github.com/ooni/probe-cli/v3/internal/logx"
Expand All @@ -40,7 +39,7 @@ type SecureFlow struct {
Depth int64

// IDGenerator is the MANDATORY atomic int64 to generate task IDs.
IDGenerator *atomic.Int64
IDGenerator *IDGenerator

// Logger is the MANDATORY logger to use.
Logger model.Logger
Expand Down Expand Up @@ -94,7 +93,7 @@ type SecureFlow struct {
// Start starts this task in a background goroutine.
func (t *SecureFlow) Start(ctx context.Context) {
t.WaitGroup.Add(1)
index := t.IDGenerator.Add(1)
index := t.IDGenerator.NewIDForEndpointSecure()
go func() {
defer t.WaitGroup.Done() // synchronize with the parent
t.Run(ctx, index)
Expand Down
3 changes: 1 addition & 2 deletions internal/experiment/webconnectivitylte/secureflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"errors"
"net/http"
"sync"
"sync/atomic"
"testing"
"time"

Expand All @@ -16,7 +15,7 @@ func TestSecureFlow_Run(t *testing.T) {
type fields struct {
Address string
DNSCache *DNSCache
IDGenerator *atomic.Int64
IDGenerator *IDGenerator
Logger model.Logger
NumRedirects *NumRedirects
TestKeys *TestKeys
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@
},
"DNSLookupSuccess": [
1,
2
10001
],
"DNSLookupSuccessWithInvalidAddresses": [],
"DNSLookupSuccessWithValidAddress": [
1,
2
10001
],
"DNSLookupSuccessWithBogonAddresses": [],
"DNSLookupSuccessWithInvalidAddressesClassic": [],
"DNSLookupSuccessWithValidAddressClassic": [
1,
2
10001
],
"DNSLookupUnexpectedFailure": [],
"DNSLookupUnexplainedFailure": [],
Expand All @@ -33,7 +33,7 @@
"TCPConnectUnexplainedFailureDuringWebFetch": [],
"TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
"TLSHandshakeExpectedFailure": [
3
40001
],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
Expand All @@ -56,7 +56,7 @@
"TagDepth": 0,
"Type": 2,
"Failure": "ssl_invalid_certificate",
"TransactionID": 3,
"TransactionID": 40001,
"TagFetchBody": true,
"DNSTransactionID": 1,
"DNSDomain": "expired.badssl.com",
Expand All @@ -70,7 +70,7 @@
"IPAddress": "104.154.89.105",
"IPAddressASN": 396982,
"IPAddressBogon": false,
"EndpointTransactionID": 3,
"EndpointTransactionID": 40001,
"EndpointProto": "tcp",
"EndpointPort": "443",
"EndpointAddress": "104.154.89.105:443",
Expand Down Expand Up @@ -103,9 +103,9 @@
"TagDepth": 0,
"Type": 0,
"Failure": "",
"TransactionID": 2,
"TransactionID": 10001,
"TagFetchBody": null,
"DNSTransactionID": 2,
"DNSTransactionID": 10001,
"DNSDomain": "expired.badssl.com",
"DNSLookupFailure": "",
"DNSQueryType": "A",
Expand Down Expand Up @@ -197,9 +197,9 @@
"TagDepth": 0,
"Type": 0,
"Failure": "dns_no_answer",
"TransactionID": 2,
"TransactionID": 10001,
"TagFetchBody": null,
"DNSTransactionID": 2,
"DNSTransactionID": 10001,
"DNSDomain": "expired.badssl.com",
"DNSLookupFailure": "dns_no_answer",
"DNSQueryType": "AAAA",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"TCPConnectUnexplainedFailureDuringWebFetch": [],
"TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
"TLSHandshakeExpectedFailure": [
3
40001
],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
Expand All @@ -53,7 +53,7 @@
"TagDepth": 0,
"Type": 2,
"Failure": "ssl_invalid_certificate",
"TransactionID": 3,
"TransactionID": 40001,
"TagFetchBody": true,
"DNSTransactionID": 1,
"DNSDomain": "expired.badssl.com",
Expand All @@ -67,7 +67,7 @@
"IPAddress": "104.154.89.105",
"IPAddressASN": 396982,
"IPAddressBogon": false,
"EndpointTransactionID": 3,
"EndpointTransactionID": 40001,
"EndpointProto": "tcp",
"EndpointPort": "443",
"EndpointAddress": "104.154.89.105:443",
Expand Down
Loading

0 comments on commit 165a9ac

Please sign in to comment.