From 977922a6b5f62eb9a3068581d42d7083f79740fd Mon Sep 17 00:00:00 2001
From: Bogdan Drutu <bogdandrutu@gmail.com>
Date: Wed, 13 Dec 2023 10:19:54 -0800
Subject: [PATCH] Add config Validate for exporterhelper.TimeoutSettings

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
---
 .chloggen/validatetimeout.yaml                | 13 ++++++++++
 exporter/exporterhelper/timeout_sender.go     | 23 +++++++++++++-----
 .../exporterhelper/timeout_sender_test.go     | 24 +++++++++++++++++++
 3 files changed, 54 insertions(+), 6 deletions(-)
 create mode 100755 .chloggen/validatetimeout.yaml
 create mode 100644 exporter/exporterhelper/timeout_sender_test.go

diff --git a/.chloggen/validatetimeout.yaml b/.chloggen/validatetimeout.yaml
new file mode 100755
index 00000000000..6f1bc9a2651
--- /dev/null
+++ b/.chloggen/validatetimeout.yaml
@@ -0,0 +1,13 @@
+# 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: "exporterhelper"
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: "Add config Validate for TimeoutSettings"
+
+# One or more tracking issues or pull requests related to the change
+issues: [9104]
diff --git a/exporter/exporterhelper/timeout_sender.go b/exporter/exporterhelper/timeout_sender.go
index 0ba11490efa..2a6364a2aaf 100644
--- a/exporter/exporterhelper/timeout_sender.go
+++ b/exporter/exporterhelper/timeout_sender.go
@@ -5,15 +5,25 @@ package exporterhelper // import "go.opentelemetry.io/collector/exporter/exporte
 
 import (
 	"context"
+	"errors"
 	"time"
 )
 
 // TimeoutSettings for timeout. The timeout applies to individual attempts to send data to the backend.
 type TimeoutSettings struct {
 	// Timeout is the timeout for every attempt to send data to the backend.
+	// A zero timeout means no timeout.
 	Timeout time.Duration `mapstructure:"timeout"`
 }
 
+func (ts *TimeoutSettings) Validate() error {
+	// Negative timeouts are not acceptable, since all sends will fail.
+	if ts.Timeout < 0 {
+		return errors.New("'timeout' must be non-negative")
+	}
+	return nil
+}
+
 // NewDefaultTimeoutSettings returns the default settings for TimeoutSettings.
 func NewDefaultTimeoutSettings() TimeoutSettings {
 	return TimeoutSettings{
@@ -28,12 +38,13 @@ type timeoutSender struct {
 }
 
 func (ts *timeoutSender) send(ctx context.Context, req Request) error {
+	// TODO: Remove this by avoiding to create the timeout sender if timeout is 0.
+	if ts.cfg.Timeout == 0 {
+		return req.Export(ctx)
+	}
 	// Intentionally don't overwrite the context inside the request, because in case of retries deadline will not be
 	// updated because this deadline most likely is before the next one.
-	if ts.cfg.Timeout > 0 {
-		var cancelFunc func()
-		ctx, cancelFunc = context.WithTimeout(ctx, ts.cfg.Timeout)
-		defer cancelFunc()
-	}
-	return req.Export(ctx)
+	tCtx, cancelFunc := context.WithTimeout(ctx, ts.cfg.Timeout)
+	defer cancelFunc()
+	return req.Export(tCtx)
 }
diff --git a/exporter/exporterhelper/timeout_sender_test.go b/exporter/exporterhelper/timeout_sender_test.go
new file mode 100644
index 00000000000..205ac094c07
--- /dev/null
+++ b/exporter/exporterhelper/timeout_sender_test.go
@@ -0,0 +1,24 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package exporterhelper
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewDefaultTimeoutSettings(t *testing.T) {
+	cfg := NewDefaultTimeoutSettings()
+	assert.NoError(t, cfg.Validate())
+	assert.Equal(t, TimeoutSettings{Timeout: 5 * time.Second}, cfg)
+}
+
+func TestInvalidTimeout(t *testing.T) {
+	cfg := NewDefaultTimeoutSettings()
+	assert.NoError(t, cfg.Validate())
+	cfg.Timeout = -1
+	assert.Error(t, cfg.Validate())
+}