[exporterhelper] Make panics into errors

Author: mx-psi

.chloggen/mx-psi_fallible-options-exporterhelper.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: exporterhelper
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Invalid exporterhelper options now make the exporter creation error out instead of panicking.
+
+# One or more tracking issues or pull requests related to the change
+issues: [9717]
+
+# (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: [api]

exporter/exporterhelper/common.go

@@ -5,6 +5,7 @@ package exporterhelper // import "go.opentelemetry.io/collector/exporter/exporte
 
 import (
 	"context"
+	"fmt"
 
 	"go.uber.org/multierr"
 	"go.uber.org/zap"
@@ -42,55 +43,74 @@ func (b *baseRequestSender) setNextSender(nextSender requestSender) {
 type obsrepSenderFactory func(obsrep *ObsReport) requestSender
 
 // Option apply changes to baseExporter.
-type Option func(*baseExporter)
+type Option interface {
+	apply(*baseExporter) error
+}
+
+var _ Option = (optionFuncNoErr)(nil)
+
+type optionFuncNoErr func(*baseExporter)
+
+func (o optionFuncNoErr) apply(be *baseExporter) error {
+	o(be)
+	return nil
+}
+
+var _ Option = (optionFunc)(nil)
+
+type optionFunc func(*baseExporter) error
+
+func (o optionFunc) apply(be *baseExporter) error {
+	return o(be)
+}
 
 // WithStart overrides the default Start function for an exporter.
 // The default start function does nothing and always returns nil.
 func WithStart(start component.StartFunc) Option {
-	return func(o *baseExporter) {
+	return optionFuncNoErr(func(o *baseExporter) {
 		o.StartFunc = start
-	}
+	})
 }
 
 // WithShutdown overrides the default Shutdown function for an exporter.
 // The default shutdown function does nothing and always returns nil.
 func WithShutdown(shutdown component.ShutdownFunc) Option {
-	return func(o *baseExporter) {
+	return optionFuncNoErr(func(o *baseExporter) {
 		o.ShutdownFunc = shutdown
-	}
+	})
 }
 
 // WithTimeout overrides the default TimeoutSettings for an exporter.
 // The default TimeoutSettings is 5 seconds.
 func WithTimeout(timeoutSettings TimeoutSettings) Option {
-	return func(o *baseExporter) {
+	return optionFuncNoErr(func(o *baseExporter) {
 		o.timeoutSender.cfg = timeoutSettings
-	}
+	})
 }
 
 // WithRetry overrides the default configretry.BackOffConfig for an exporter.
 // The default configretry.BackOffConfig is to disable retries.
 func WithRetry(config configretry.BackOffConfig) Option {
-	return func(o *baseExporter) {
+	return optionFuncNoErr(func(o *baseExporter) {
 		if !config.Enabled {
 			o.exportFailureMessage += " Try enabling retry_on_failure config option to retry on retryable errors."
 			return
 		}
 		o.retrySender = newRetrySender(config, o.set)
-	}
+	})
 }
 
 // WithQueue overrides the default QueueSettings for an exporter.
 // The default QueueSettings is to disable queueing.
 // This option cannot be used with the new exporter helpers New[Traces|Metrics|Logs]RequestExporter.
 func WithQueue(config QueueSettings) Option {
-	return func(o *baseExporter) {
+	return optionFunc(func(o *baseExporter) error {
 		if o.marshaler == nil || o.unmarshaler == nil {
-			panic("WithQueue option is not available for the new request exporters, use WithRequestQueue instead")
+			return fmt.Errorf("WithQueue option is not available for the new request exporters, use WithRequestQueue instead")
 		}
 		if !config.Enabled {
 			o.exportFailureMessage += " Try enabling sending_queue to survive temporary failures."
-			return
+			return nil
 		}
 		qf := exporterqueue.NewPersistentQueueFactory[Request](config.StorageID, exporterqueue.PersistentQueueSettings[Request]{
 			Marshaler:   o.marshaler,
@@ -105,53 +125,55 @@ func WithQueue(config QueueSettings) Option {
 			QueueSize:    config.QueueSize,
 		})
 		o.queueSender = newQueueSender(q, o.set, config.NumConsumers, o.exportFailureMessage)
-	}
+		return nil
+	})
 }
 
 // WithRequestQueue enables queueing for an exporter.
 // This option should be used with the new exporter helpers New[Traces|Metrics|Logs]RequestExporter.
 // This API is at the early stage of development and may change without backward compatibility
 // until https://github.com/open-telemetry/opentelemetry-collector/issues/8122 is resolved.
 func WithRequestQueue(cfg exporterqueue.Config, queueFactory exporterqueue.Factory[Request]) Option {
-	return func(o *baseExporter) {
+	return optionFunc(func(o *baseExporter) error {
 		if o.marshaler != nil || o.unmarshaler != nil {
-			panic("WithRequestQueue option must be used with the new request exporters only, use WithQueue instead")
+			return fmt.Errorf("WithRequestQueue option must be used with the new request exporters only, use WithQueue instead")
 		}
 		if !cfg.Enabled {
 			o.exportFailureMessage += " Try enabling sending_queue to survive temporary failures."
-			return
+			return nil
 		}
 		set := exporterqueue.Settings{
 			DataType:         o.signal,
 			ExporterSettings: o.set,
 		}
 		o.queueSender = newQueueSender(queueFactory(context.Background(), set, cfg), o.set, cfg.NumConsumers, o.exportFailureMessage)
-	}
+		return nil
+	})
 }
 
 // WithCapabilities overrides the default Capabilities() function for a Consumer.
 // The default is non-mutable data.
 // TODO: Verify if we can change the default to be mutable as we do for processors.
 func WithCapabilities(capabilities consumer.Capabilities) Option {
-	return func(o *baseExporter) {
+	return optionFuncNoErr(func(o *baseExporter) {
 		o.consumerOptions = append(o.consumerOptions, consumer.WithCapabilities(capabilities))
-	}
+	})
 }
 
 // withMarshaler is used to set the request marshaler for the new exporter helper.
 // It must be provided as the first option when creating a new exporter helper.
 func withMarshaler(marshaler exporterqueue.Marshaler[Request]) Option {
-	return func(o *baseExporter) {
+	return optionFuncNoErr(func(o *baseExporter) {
 		o.marshaler = marshaler
-	}
+	})
 }
 
 // withUnmarshaler is used to set the request unmarshaler for the new exporter helper.
 // It must be provided as the first option when creating a new exporter helper.
 func withUnmarshaler(unmarshaler exporterqueue.Unmarshaler[Request]) Option {
-	return func(o *baseExporter) {
+	return optionFuncNoErr(func(o *baseExporter) {
 		o.unmarshaler = unmarshaler
-	}
+	})
 }
 
 // baseExporter contains common fields between different exporter types.
@@ -199,8 +221,12 @@ func newBaseExporter(set exporter.CreateSettings, signal component.DataType, osf
 	}
 
 	for _, op := range options {
-		op(be)
+		err = multierr.Append(err, op.apply(be))
+	}
+	if err != nil {
+		return nil, err
 	}
+
 	be.connectSenders()
 
 	return be, nil

exporter/exporterhelper/common_test.go

@@ -71,16 +71,15 @@ func TestQueueOptionsWithRequestExporter(t *testing.T) {
 	require.Nil(t, err)
 	require.Nil(t, bs.marshaler)
 	require.Nil(t, bs.unmarshaler)
-	require.Panics(t, func() {
-		_, _ = newBaseExporter(exportertest.NewNopCreateSettings(), defaultType, newNoopObsrepSender,
-			WithRetry(configretry.NewDefaultBackOffConfig()), WithQueue(NewDefaultQueueSettings()))
-	})
-	require.Panics(t, func() {
-		_, _ = newBaseExporter(exportertest.NewNopCreateSettings(), defaultType, newNoopObsrepSender,
-			withMarshaler(mockRequestMarshaler), withUnmarshaler(mockRequestUnmarshaler(&mockRequest{})),
-			WithRetry(configretry.NewDefaultBackOffConfig()),
-			WithRequestQueue(exporterqueue.NewDefaultConfig(), exporterqueue.NewMemoryQueueFactory[Request]()))
-	})
+	_, err = newBaseExporter(exportertest.NewNopCreateSettings(), defaultType, newNoopObsrepSender,
+		WithRetry(configretry.NewDefaultBackOffConfig()), WithQueue(NewDefaultQueueSettings()))
+	require.Error(t, err)
+
+	_, err = newBaseExporter(exportertest.NewNopCreateSettings(), defaultType, newNoopObsrepSender,
+		withMarshaler(mockRequestMarshaler), withUnmarshaler(mockRequestUnmarshaler(&mockRequest{})),
+		WithRetry(configretry.NewDefaultBackOffConfig()),
+		WithRequestQueue(exporterqueue.NewDefaultConfig(), exporterqueue.NewMemoryQueueFactory[Request]()))
+	require.Error(t, err)
 }
 
 func TestBaseExporterLogging(t *testing.T) {