From 28967fbc03d8e2e761cc193b450e6578dd734c96 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Mon, 11 Dec 2023 11:30:12 -0700 Subject: [PATCH 1/8] Add DialerTimeout config option to confignet --- config/confignet/README.md | 1 + config/confignet/confignet.go | 13 ++++++++++-- config/confignet/confignet_test.go | 33 ++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/config/confignet/README.md b/config/confignet/README.md index 579564cbff4..cd8150e6303 100644 --- a/config/confignet/README.md +++ b/config/confignet/README.md @@ -13,6 +13,7 @@ leverage network configuration to set connection and transport information. - `transport`: Known protocols are "tcp", "tcp4" (IPv4-only), "tcp6" (IPv6-only), "udp", "udp4" (IPv4-only), "udp6" (IPv6-only), "ip", "ip4" (IPv4-only), "ip6" (IPv6-only), "unix", "unixgram" and "unixpacket". +- `dialer_timeout`: DialerTimeout is the maximum amount of time a dial will wait for a connect to complete. The default is no timeout. Note that for TCP receivers only the `endpoint` configuration setting is required. diff --git a/config/confignet/confignet.go b/config/confignet/confignet.go index 6695b6035b8..833756e4ecc 100644 --- a/config/confignet/confignet.go +++ b/config/confignet/confignet.go @@ -5,6 +5,7 @@ package confignet // import "go.opentelemetry.io/collector/config/confignet" import ( "net" + "time" ) // NetAddr represents a network endpoint address. @@ -19,11 +20,15 @@ type NetAddr struct { // Transport to use. Known protocols are "tcp", "tcp4" (IPv4-only), "tcp6" (IPv6-only), "udp", "udp4" (IPv4-only), // "udp6" (IPv6-only), "ip", "ip4" (IPv4-only), "ip6" (IPv6-only), "unix", "unixgram" and "unixpacket". Transport string `mapstructure:"transport"` + + // DialerTimeout is the maximum amount of time a dial will wait for + // a connect to complete. The default is no timeout. + DialerTimeout time.Duration `mapstructure:"dialer_timeout"` } // Dial equivalent with net.Dial for this address. func (na *NetAddr) Dial() (net.Conn, error) { - return net.Dial(na.Transport, na.Endpoint) + return net.DialTimeout(na.Transport, na.Endpoint, na.DialerTimeout) } // Listen equivalent with net.Listen for this address. @@ -39,11 +44,15 @@ type TCPAddr struct { // If the host is a literal IPv6 address it must be enclosed in square brackets, as in "[2001:db8::1]:80" or // "[fe80::1%zone]:80". The zone specifies the scope of the literal IPv6 address as defined in RFC 4007. Endpoint string `mapstructure:"endpoint"` + + // DialerTimeout is the maximum amount of time a dial will wait for + // a connect to complete. The default is no timeout. + DialerTimeout time.Duration `mapstructure:"dialer_timeout"` } // Dial equivalent with net.Dial for this address. func (na *TCPAddr) Dial() (net.Conn, error) { - return net.Dial("tcp", na.Endpoint) + return net.DialTimeout("tcp", na.Endpoint, na.DialerTimeout) } // Listen equivalent with net.Listen for this address. diff --git a/config/confignet/confignet_test.go b/config/confignet/confignet_test.go index 7ba5e442ed5..88273bb4bd5 100644 --- a/config/confignet/confignet_test.go +++ b/config/confignet/confignet_test.go @@ -4,8 +4,10 @@ package confignet import ( + "errors" "net" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -45,6 +47,22 @@ func TestNetAddr(t *testing.T) { assert.NoError(t, ln.Close()) } +func TestNetAddrTimeout(t *testing.T) { + nac := &NetAddr{ + Endpoint: "localhost:0", + Transport: "tcp", + DialerTimeout: time.Millisecond, + } + _, err := nac.Dial() + assert.Error(t, err) + var netErr net.Error + if errors.As(err, &netErr) { + assert.True(t, netErr.Timeout()) + } else { + assert.Fail(t, "error should be a net.Error") + } +} + func TestTcpAddr(t *testing.T) { nas := &TCPAddr{ Endpoint: "localhost:0", @@ -77,3 +95,18 @@ func TestTcpAddr(t *testing.T) { <-done assert.NoError(t, ln.Close()) } + +func TestTcpAddrTimeout(t *testing.T) { + nac := &TCPAddr{ + Endpoint: "localhost:0", + DialerTimeout: time.Millisecond, + } + _, err := nac.Dial() + assert.Error(t, err) + var netErr net.Error + if errors.As(err, &netErr) { + assert.True(t, netErr.Timeout()) + } else { + assert.Fail(t, "error should be a net.Error") + } +} From 8fd08f4d9f38c6836f50155e219ad50974036807 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Mon, 11 Dec 2023 11:33:31 -0700 Subject: [PATCH 2/8] changelog --- .chloggen/confignet-add-dialertimeout.yaml | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100755 .chloggen/confignet-add-dialertimeout.yaml diff --git a/.chloggen/confignet-add-dialertimeout.yaml b/.chloggen/confignet-add-dialertimeout.yaml new file mode 100755 index 00000000000..915be3fe66f --- /dev/null +++ b/.chloggen/confignet-add-dialertimeout.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: confignet + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add `dialer_timeout` config option. + +# One or more tracking issues or pull requests related to the change +issues: [9066] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] \ No newline at end of file From db66c0e36a050defb45abfb2bb0d570eec875458 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Mon, 11 Dec 2023 12:20:36 -0700 Subject: [PATCH 3/8] Fix flaky test by fixing the order --- config/confignet/confignet_test.go | 73 ++++++++++++++++-------------- 1 file changed, 40 insertions(+), 33 deletions(-) diff --git a/config/confignet/confignet_test.go b/config/confignet/confignet_test.go index 88273bb4bd5..de524024339 100644 --- a/config/confignet/confignet_test.go +++ b/config/confignet/confignet_test.go @@ -12,7 +12,45 @@ import ( "github.com/stretchr/testify/assert" ) -func TestNetAddr(t *testing.T) { +func Test_SetOrder(t *testing.T) { + t.Run("TestNetAddrTimeout", testNetAddrTimeout) + t.Run("TestTcpAddrTimeout", testTcpAddrTimeout) + t.Run("TestNetAddr", testNetAddr) + t.Run("testTcpAddr", testTcpAddr) +} + +func testNetAddrTimeout(t *testing.T) { + nac := &NetAddr{ + Endpoint: "localhost:0", + Transport: "tcp", + DialerTimeout: time.Millisecond, + } + _, err := nac.Dial() + assert.Error(t, err) + var netErr net.Error + if errors.As(err, &netErr) { + assert.True(t, netErr.Timeout()) + } else { + assert.Fail(t, "error should be a net.Error") + } +} + +func testTcpAddrTimeout(t *testing.T) { + nac := &TCPAddr{ + Endpoint: "localhost:0", + DialerTimeout: time.Millisecond, + } + _, err := nac.Dial() + assert.Error(t, err) + var netErr net.Error + if errors.As(err, &netErr) { + assert.True(t, netErr.Timeout()) + } else { + assert.Fail(t, "error should be a net.Error") + } +} + +func testNetAddr(t *testing.T) { nas := &NetAddr{ Endpoint: "localhost:0", Transport: "tcp", @@ -47,23 +85,7 @@ func TestNetAddr(t *testing.T) { assert.NoError(t, ln.Close()) } -func TestNetAddrTimeout(t *testing.T) { - nac := &NetAddr{ - Endpoint: "localhost:0", - Transport: "tcp", - DialerTimeout: time.Millisecond, - } - _, err := nac.Dial() - assert.Error(t, err) - var netErr net.Error - if errors.As(err, &netErr) { - assert.True(t, netErr.Timeout()) - } else { - assert.Fail(t, "error should be a net.Error") - } -} - -func TestTcpAddr(t *testing.T) { +func testTcpAddr(t *testing.T) { nas := &TCPAddr{ Endpoint: "localhost:0", } @@ -95,18 +117,3 @@ func TestTcpAddr(t *testing.T) { <-done assert.NoError(t, ln.Close()) } - -func TestTcpAddrTimeout(t *testing.T) { - nac := &TCPAddr{ - Endpoint: "localhost:0", - DialerTimeout: time.Millisecond, - } - _, err := nac.Dial() - assert.Error(t, err) - var netErr net.Error - if errors.As(err, &netErr) { - assert.True(t, netErr.Timeout()) - } else { - assert.Fail(t, "error should be a net.Error") - } -} From 5bde6f1dc7d1a81668147f4545988b362617dce6 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Mon, 11 Dec 2023 12:31:46 -0700 Subject: [PATCH 4/8] Fix lint --- config/confignet/confignet_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/config/confignet/confignet_test.go b/config/confignet/confignet_test.go index de524024339..042254e666d 100644 --- a/config/confignet/confignet_test.go +++ b/config/confignet/confignet_test.go @@ -14,9 +14,9 @@ import ( func Test_SetOrder(t *testing.T) { t.Run("TestNetAddrTimeout", testNetAddrTimeout) - t.Run("TestTcpAddrTimeout", testTcpAddrTimeout) + t.Run("TestTcpAddrTimeout", testTCPAddrTimeout) t.Run("TestNetAddr", testNetAddr) - t.Run("testTcpAddr", testTcpAddr) + t.Run("testTcpAddr", testTCPAddr) } func testNetAddrTimeout(t *testing.T) { @@ -35,7 +35,7 @@ func testNetAddrTimeout(t *testing.T) { } } -func testTcpAddrTimeout(t *testing.T) { +func testTCPAddrTimeout(t *testing.T) { nac := &TCPAddr{ Endpoint: "localhost:0", DialerTimeout: time.Millisecond, @@ -85,7 +85,7 @@ func testNetAddr(t *testing.T) { assert.NoError(t, ln.Close()) } -func testTcpAddr(t *testing.T) { +func testTCPAddr(t *testing.T) { nas := &TCPAddr{ Endpoint: "localhost:0", } From 992d3fc7f405d39eb97a0b6bc91c8dde2b06be4a Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Mon, 11 Dec 2023 12:34:29 -0700 Subject: [PATCH 5/8] Keep tweaking tests --- config/confignet/confignet_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/confignet/confignet_test.go b/config/confignet/confignet_test.go index 042254e666d..3a0e4bb71a9 100644 --- a/config/confignet/confignet_test.go +++ b/config/confignet/confignet_test.go @@ -23,7 +23,7 @@ func testNetAddrTimeout(t *testing.T) { nac := &NetAddr{ Endpoint: "localhost:0", Transport: "tcp", - DialerTimeout: time.Millisecond, + DialerTimeout: -1 * time.Second, } _, err := nac.Dial() assert.Error(t, err) @@ -38,7 +38,7 @@ func testNetAddrTimeout(t *testing.T) { func testTCPAddrTimeout(t *testing.T) { nac := &TCPAddr{ Endpoint: "localhost:0", - DialerTimeout: time.Millisecond, + DialerTimeout: -1 * time.Second, } _, err := nac.Dial() assert.Error(t, err) From 18b703df2f0befea80016eaa8c43087aee5493e0 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Mon, 11 Dec 2023 12:44:25 -0700 Subject: [PATCH 6/8] Keep tweaking tests --- config/confignet/confignet_test.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/config/confignet/confignet_test.go b/config/confignet/confignet_test.go index 3a0e4bb71a9..d8373995f52 100644 --- a/config/confignet/confignet_test.go +++ b/config/confignet/confignet_test.go @@ -12,14 +12,7 @@ import ( "github.com/stretchr/testify/assert" ) -func Test_SetOrder(t *testing.T) { - t.Run("TestNetAddrTimeout", testNetAddrTimeout) - t.Run("TestTcpAddrTimeout", testTCPAddrTimeout) - t.Run("TestNetAddr", testNetAddr) - t.Run("testTcpAddr", testTCPAddr) -} - -func testNetAddrTimeout(t *testing.T) { +func TestNetAddrTimeout(t *testing.T) { nac := &NetAddr{ Endpoint: "localhost:0", Transport: "tcp", @@ -35,7 +28,7 @@ func testNetAddrTimeout(t *testing.T) { } } -func testTCPAddrTimeout(t *testing.T) { +func TestTCPAddrTimeout(t *testing.T) { nac := &TCPAddr{ Endpoint: "localhost:0", DialerTimeout: -1 * time.Second, @@ -50,7 +43,7 @@ func testTCPAddrTimeout(t *testing.T) { } } -func testNetAddr(t *testing.T) { +func TestNetAddr(t *testing.T) { nas := &NetAddr{ Endpoint: "localhost:0", Transport: "tcp", @@ -85,7 +78,7 @@ func testNetAddr(t *testing.T) { assert.NoError(t, ln.Close()) } -func testTCPAddr(t *testing.T) { +func TestTCPAddr(t *testing.T) { nas := &TCPAddr{ Endpoint: "localhost:0", } From 7cbb9a0c1229bc26c95057230589b785dcfe5e32 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Wed, 13 Dec 2023 11:25:31 -0700 Subject: [PATCH 7/8] Move Timeout into a Dialer struct --- config/confignet/confignet.go | 21 +++++++++++++-------- config/confignet/confignet_test.go | 14 +++++++++----- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/config/confignet/confignet.go b/config/confignet/confignet.go index 833756e4ecc..f2a4ce5aaf0 100644 --- a/config/confignet/confignet.go +++ b/config/confignet/confignet.go @@ -8,6 +8,13 @@ import ( "time" ) +// Dialer contains options for connecting to an address. +type Dialer struct { + // Timeout is the maximum amount of time a dial will wait for + // a connect to complete. The default is no timeout. + Timeout time.Duration `mapstructure:"timeout"` +} + // NetAddr represents a network endpoint address. type NetAddr struct { // Endpoint configures the address for this network connection. @@ -21,14 +28,13 @@ type NetAddr struct { // "udp6" (IPv6-only), "ip", "ip4" (IPv4-only), "ip6" (IPv6-only), "unix", "unixgram" and "unixpacket". Transport string `mapstructure:"transport"` - // DialerTimeout is the maximum amount of time a dial will wait for - // a connect to complete. The default is no timeout. - DialerTimeout time.Duration `mapstructure:"dialer_timeout"` + // Dialer contains options for connecting to an address. + Dialer Dialer `mapstructure:"dialer"` } // Dial equivalent with net.Dial for this address. func (na *NetAddr) Dial() (net.Conn, error) { - return net.DialTimeout(na.Transport, na.Endpoint, na.DialerTimeout) + return net.DialTimeout(na.Transport, na.Endpoint, na.Dialer.Timeout) } // Listen equivalent with net.Listen for this address. @@ -45,14 +51,13 @@ type TCPAddr struct { // "[fe80::1%zone]:80". The zone specifies the scope of the literal IPv6 address as defined in RFC 4007. Endpoint string `mapstructure:"endpoint"` - // DialerTimeout is the maximum amount of time a dial will wait for - // a connect to complete. The default is no timeout. - DialerTimeout time.Duration `mapstructure:"dialer_timeout"` + // Dialer contains options for connecting to an address. + Dialer Dialer `mapstructure:"dialer"` } // Dial equivalent with net.Dial for this address. func (na *TCPAddr) Dial() (net.Conn, error) { - return net.DialTimeout("tcp", na.Endpoint, na.DialerTimeout) + return net.DialTimeout("tcp", na.Endpoint, na.Dialer.Timeout) } // Listen equivalent with net.Listen for this address. diff --git a/config/confignet/confignet_test.go b/config/confignet/confignet_test.go index d8373995f52..270848ad3e1 100644 --- a/config/confignet/confignet_test.go +++ b/config/confignet/confignet_test.go @@ -14,9 +14,11 @@ import ( func TestNetAddrTimeout(t *testing.T) { nac := &NetAddr{ - Endpoint: "localhost:0", - Transport: "tcp", - DialerTimeout: -1 * time.Second, + Endpoint: "localhost:0", + Transport: "tcp", + Dialer: Dialer{ + Timeout: -1 * time.Second, + }, } _, err := nac.Dial() assert.Error(t, err) @@ -30,8 +32,10 @@ func TestNetAddrTimeout(t *testing.T) { func TestTCPAddrTimeout(t *testing.T) { nac := &TCPAddr{ - Endpoint: "localhost:0", - DialerTimeout: -1 * time.Second, + Endpoint: "localhost:0", + Dialer: Dialer{ + Timeout: -1 * time.Second, + }, } _, err := nac.Dial() assert.Error(t, err) From 78991d1d39419534927d999fe032998d1d3b573e Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Tue, 19 Dec 2023 12:34:41 -0700 Subject: [PATCH 8/8] Rename to DialerConfig --- config/confignet/confignet.go | 16 ++++++++-------- config/confignet/confignet_test.go | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/config/confignet/confignet.go b/config/confignet/confignet.go index f2a4ce5aaf0..d3adf14209d 100644 --- a/config/confignet/confignet.go +++ b/config/confignet/confignet.go @@ -8,8 +8,8 @@ import ( "time" ) -// Dialer contains options for connecting to an address. -type Dialer struct { +// DialerConfig contains options for connecting to an address. +type DialerConfig struct { // Timeout is the maximum amount of time a dial will wait for // a connect to complete. The default is no timeout. Timeout time.Duration `mapstructure:"timeout"` @@ -28,13 +28,13 @@ type NetAddr struct { // "udp6" (IPv6-only), "ip", "ip4" (IPv4-only), "ip6" (IPv6-only), "unix", "unixgram" and "unixpacket". Transport string `mapstructure:"transport"` - // Dialer contains options for connecting to an address. - Dialer Dialer `mapstructure:"dialer"` + // DialerConfig contains options for connecting to an address. + DialerConfig DialerConfig `mapstructure:"dialer"` } // Dial equivalent with net.Dial for this address. func (na *NetAddr) Dial() (net.Conn, error) { - return net.DialTimeout(na.Transport, na.Endpoint, na.Dialer.Timeout) + return net.DialTimeout(na.Transport, na.Endpoint, na.DialerConfig.Timeout) } // Listen equivalent with net.Listen for this address. @@ -51,13 +51,13 @@ type TCPAddr struct { // "[fe80::1%zone]:80". The zone specifies the scope of the literal IPv6 address as defined in RFC 4007. Endpoint string `mapstructure:"endpoint"` - // Dialer contains options for connecting to an address. - Dialer Dialer `mapstructure:"dialer"` + // DialerConfig contains options for connecting to an address. + DialerConfig DialerConfig `mapstructure:"dialer"` } // Dial equivalent with net.Dial for this address. func (na *TCPAddr) Dial() (net.Conn, error) { - return net.DialTimeout("tcp", na.Endpoint, na.Dialer.Timeout) + return net.DialTimeout("tcp", na.Endpoint, na.DialerConfig.Timeout) } // Listen equivalent with net.Listen for this address. diff --git a/config/confignet/confignet_test.go b/config/confignet/confignet_test.go index 270848ad3e1..3b8c3fcbbe2 100644 --- a/config/confignet/confignet_test.go +++ b/config/confignet/confignet_test.go @@ -16,7 +16,7 @@ func TestNetAddrTimeout(t *testing.T) { nac := &NetAddr{ Endpoint: "localhost:0", Transport: "tcp", - Dialer: Dialer{ + DialerConfig: DialerConfig{ Timeout: -1 * time.Second, }, } @@ -33,7 +33,7 @@ func TestNetAddrTimeout(t *testing.T) { func TestTCPAddrTimeout(t *testing.T) { nac := &TCPAddr{ Endpoint: "localhost:0", - Dialer: Dialer{ + DialerConfig: DialerConfig{ Timeout: -1 * time.Second, }, }