[pipeline] Dreprecate MustNewID and MustNewIDWithName

Signed-off-by: Bogdan Drutu <[email protected]>

Author: bogdandrutu

pipeline/internal/globalsignal/signal.go

@@ -4,9 +4,18 @@
 package globalsignal // import "go.opentelemetry.io/collector/pipeline/internal/globalsignal"
 
 import (
-	"errors"
+	"encoding"
 	"fmt"
-	"regexp"
+)
+
+var (
+	SignalProfiles = Signal{name: "profiles"}
+	SignalTraces   = Signal{name: "traces"}
+	SignalMetrics  = Signal{name: "metrics"}
+	SignalLogs     = Signal{name: "logs"}
+
+	_ encoding.TextMarshaler   = (*Signal)(nil)
+	_ encoding.TextUnmarshaler = (*Signal)(nil)
 )
 
 // Signal represents the signals supported by the collector.
@@ -15,37 +24,28 @@ type Signal struct {
 }
 
 // String returns the string representation of the signal.
-func (s Signal) String() string {
+func (s *Signal) String() string {
 	return s.name
 }
 
 // MarshalText marshals the Signal.
-func (s Signal) MarshalText() (text []byte, err error) {
+func (s *Signal) MarshalText() ([]byte, error) {
 	return []byte(s.name), nil
 }
 
-// signalRegex is used to validate the signal.
-// A signal must consist of 1 to 62 lowercase ASCII alphabetic characters.
-var signalRegex = regexp.MustCompile(`^[a-z]{1,62}$`)
-
-// NewSignal creates a Signal. It returns an error if the Signal is invalid.
-// A Signal must consist of 1 to 62 lowercase ASCII alphabetic characters.
-func NewSignal(signal string) (Signal, error) {
-	if len(signal) == 0 {
-		return Signal{}, errors.New("signal must not be empty")
-	}
-	if !signalRegex.MatchString(signal) {
-		return Signal{}, fmt.Errorf("invalid character(s) in type %q", signal)
-	}
-	return Signal{name: signal}, nil
-}
-
-// MustNewSignal creates a Signal. It panics if the Signal is invalid.
-// A signal must consist of 1 to 62 lowercase ASCII alphabetic characters.
-func MustNewSignal(signal string) Signal {
-	s, err := NewSignal(signal)
-	if err != nil {
-		panic(err)
+// UnmarshalText marshals the Signal.
+func (s *Signal) UnmarshalText(text []byte) error {
+	switch string(text) {
+	case SignalProfiles.name:
+		*s = SignalProfiles
+	case SignalTraces.name:
+		*s = SignalTraces
+	case SignalMetrics.name:
+		*s = SignalMetrics
+	case SignalLogs.name:
+		*s = SignalLogs
+	default:
+		return fmt.Errorf("unknown pipeline signal: %q", string(text))
 	}
-	return s
+	return nil
 }

pipeline/internal/globalsignal/signal_test.go

@@ -10,32 +10,49 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func Test_NewSignal(t *testing.T) {
-	s, err := NewSignal("traces")
-	require.NoError(t, err)
-	assert.Equal(t, Signal{name: "traces"}, s)
+func TestSignal_String(t *testing.T) {
+	assert.Equal(t, "traces", SignalTraces.String())
+	assert.Equal(t, "metrics", SignalMetrics.String())
+	assert.Equal(t, "logs", SignalLogs.String())
+	assert.Equal(t, "profiles", SignalProfiles.String())
 }
 
-func Test_NewSignal_Invalid(t *testing.T) {
-	_, err := NewSignal("")
-	require.Error(t, err)
-	_, err = NewSignal("TRACES")
-	require.Error(t, err)
-}
+func TestSignal_MarshalText(t *testing.T) {
+	b, err := SignalTraces.MarshalText()
+	require.NoError(t, err)
+	assert.Equal(t, []byte("traces"), b)
 
-func Test_MustNewSignal(t *testing.T) {
-	s := MustNewSignal("traces")
-	assert.Equal(t, Signal{name: "traces"}, s)
-}
+	b, err = SignalMetrics.MarshalText()
+	require.NoError(t, err)
+	assert.Equal(t, []byte("metrics"), b)
 
-func Test_Signal_String(t *testing.T) {
-	s := MustNewSignal("traces")
-	assert.Equal(t, "traces", s.String())
-}
+	b, err = SignalLogs.MarshalText()
+	require.NoError(t, err)
+	assert.Equal(t, []byte("logs"), b)
 
-func Test_Signal_MarshalText(t *testing.T) {
-	s := MustNewSignal("traces")
-	b, err := s.MarshalText()
+	b, err = SignalProfiles.MarshalText()
 	require.NoError(t, err)
-	assert.Equal(t, []byte("traces"), b)
+	assert.Equal(t, []byte("profiles"), b)
+
+	var s Signal
+	b, err = s.MarshalText()
+	require.NoError(t, err)
+	assert.Equal(t, []byte(""), b)
+}
+
+func TestSignal_UnmarshalText(t *testing.T) {
+	var s Signal
+	require.NoError(t, s.UnmarshalText([]byte("traces")))
+	assert.Equal(t, SignalTraces, s)
+
+	require.NoError(t, s.UnmarshalText([]byte("metrics")))
+	assert.Equal(t, SignalMetrics, s)
+
+	require.NoError(t, s.UnmarshalText([]byte("logs")))
+	assert.Equal(t, SignalLogs, s)
+
+	require.NoError(t, s.UnmarshalText([]byte("profiles")))
+	assert.Equal(t, SignalProfiles, s)
+
+	require.Error(t, s.UnmarshalText([]byte("unknown")))
 }

pipeline/pipeline.go

@@ -7,8 +7,6 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
-
-	"go.opentelemetry.io/collector/pipeline/internal/globalsignal"
 )
 
 // typeAndNameSeparator is the separator that is used between type and name in type/name composite keys.
@@ -27,24 +25,24 @@ func NewID(signal Signal) ID {
 	return ID{signal: signal}
 }
 
-// MustNewID builds a Signal and returns a new ID with the given Signal and empty name.
-// It panics if the Signal is invalid.
-// A signal must consist of 1 to 62 lowercase ASCII alphabetic characters.
+// Deprecated: [v0.124.0] use NewIDWithName.
 func MustNewID(signal string) ID {
-	return ID{signal: globalsignal.MustNewSignal(signal)}
+	id := ID{}
+	err := id.signal.UnmarshalText([]byte(signal))
+	if err != nil {
+		panic(err)
+	}
+	return id
 }
 
 // NewIDWithName returns a new ID with the given Signal and name.
 func NewIDWithName(signal Signal, name string) ID {
 	return ID{signal: signal, name: name}
 }
 
-// MustNewIDWithName builds a Signal and returns a new ID with the given Signal and name.
-// It panics if the Signal is invalid or name is invalid.
-// A signal must consist of 1 to 62 lowercase ASCII alphabetic characters.
-// A name must consist of 1 to 1024 unicode characters excluding whitespace, control characters, and symbols.
+// Deprecated: [v0.124.0] use NewIDWithName.
 func MustNewIDWithName(signal string, name string) ID {
-	id := ID{signal: globalsignal.MustNewSignal(signal)}
+	id := MustNewID(signal)
 	err := validateName(name)
 	if err != nil {
 		panic(err)
@@ -54,18 +52,18 @@ func MustNewIDWithName(signal string, name string) ID {
 }
 
 // Signal returns the Signal of the ID.
-func (i ID) Signal() Signal {
+func (i *ID) Signal() Signal {
 	return i.signal
 }
 
 // Name returns the name of the ID.
-func (i ID) Name() string {
+func (i *ID) Name() string {
 	return i.name
 }
 
 // MarshalText implements the encoding.TextMarshaler interface.
 // This marshals the Signal and name as one string in the config.
-func (i ID) MarshalText() (text []byte, err error) {
+func (i *ID) MarshalText() (text []byte, err error) {
 	return []byte(i.String()), nil
 }
 
@@ -93,8 +91,7 @@ func (i *ID) UnmarshalText(text []byte) error {
 		}
 	}
 
-	var err error
-	if i.signal, err = globalsignal.NewSignal(signalStr); err != nil {
+	if err := i.signal.UnmarshalText([]byte(signalStr)); err != nil {
 		return fmt.Errorf("in %q id: %w", idStr, err)
 	}
 	i.name = nameStr
@@ -103,7 +100,7 @@ func (i *ID) UnmarshalText(text []byte) error {
 }
 
 // String returns the ID string representation as "signal[/name]" format.
-func (i ID) String() string {
+func (i *ID) String() string {
 	if i.name == "" {
 		return i.signal.String()
 	}

pipeline/pipeline_test.go

@@ -41,40 +41,39 @@ func TestMarshalText(t *testing.T) {
 }
 
 func TestUnmarshalText(t *testing.T) {
-	validSignal := globalsignal.MustNewSignal("valid")
 	testCases := []struct {
 		idStr       string
 		expectedErr bool
 		expectedID  ID
 	}{
 		{
-			idStr:      "valid",
-			expectedID: ID{signal: validSignal, name: ""},
+			idStr:      "traces",
+			expectedID: ID{signal: globalsignal.SignalTraces, name: ""},
 		},
 		{
-			idStr:      "valid/valid_name",
-			expectedID: ID{signal: validSignal, name: "valid_name"},
+			idStr:      "traces/valid_name",
+			expectedID: ID{signal: globalsignal.SignalTraces, name: "valid_name"},
 		},
 		{
-			idStr:      "   valid   /   valid_name  ",
-			expectedID: ID{signal: validSignal, name: "valid_name"},
+			idStr:      "   traces   /   valid_name  ",
+			expectedID: ID{signal: globalsignal.SignalTraces, name: "valid_name"},
 		},
 		{
-			idStr:      "valid/中文好",
-			expectedID: ID{signal: validSignal, name: "中文好"},
+			idStr:      "traces/中文好",
+			expectedID: ID{signal: globalsignal.SignalTraces, name: "中文好"},
 		},
 		{
-			idStr:      "valid/name-with-dashes",
-			expectedID: ID{signal: validSignal, name: "name-with-dashes"},
+			idStr:      "traces/name-with-dashes",
+			expectedID: ID{signal: globalsignal.SignalTraces, name: "name-with-dashes"},
 		},
 		// issue 10816
 		{
-			idStr:      "valid/Linux-Messages-File_01J49HCH3SWFXRVASWFZFRT3J2__processor0__logs",
-			expectedID: ID{signal: validSignal, name: "Linux-Messages-File_01J49HCH3SWFXRVASWFZFRT3J2__processor0__logs"},
+			idStr:      "traces/Linux-Messages-File_01J49HCH3SWFXRVASWFZFRT3J2__processor0__logs",
+			expectedID: ID{signal: globalsignal.SignalTraces, name: "Linux-Messages-File_01J49HCH3SWFXRVASWFZFRT3J2__processor0__logs"},
 		},
 		{
-			idStr:      "valid/1",
-			expectedID: ID{signal: validSignal, name: "1"},
+			idStr:      "traces/1",
+			expectedID: ID{signal: globalsignal.SignalTraces, name: "1"},
 		},
 		{
 			idStr:       "/valid_name",
@@ -85,23 +84,23 @@ func TestUnmarshalText(t *testing.T) {
 			expectedErr: true,
 		},
 		{
-			idStr:       "valid/",
+			idStr:       "traces/",
 			expectedErr: true,
 		},
 		{
-			idStr:       "valid/      ",
+			idStr:       "traces/      ",
 			expectedErr: true,
 		},
 		{
 			idStr:       "      ",
 			expectedErr: true,
 		},
 		{
-			idStr:       "valid/invalid name",
+			idStr:       "traces/invalid name",
 			expectedErr: true,
 		},
 		{
-			idStr:       "valid/" + strings.Repeat("a", 1025),
+			idStr:       "traces/" + strings.Repeat("a", 1025),
 			expectedErr: true,
 		},
 		{

pipeline/signal.go

@@ -16,7 +16,7 @@ type Signal = globalsignal.Signal
 var ErrSignalNotSupported = errors.New("telemetry type is not supported")
 
 var (
-	SignalTraces  = globalsignal.MustNewSignal("traces")
-	SignalMetrics = globalsignal.MustNewSignal("metrics")
-	SignalLogs    = globalsignal.MustNewSignal("logs")
+	SignalTraces  = globalsignal.SignalTraces
+	SignalMetrics = globalsignal.SignalMetrics
+	SignalLogs    = globalsignal.SignalLogs
 )

pipeline/signal_test.go

@@ -5,4 +5,4 @@ package xpipeline // import "go.opentelemetry.io/collector/pipeline/xpipeline"
 
 import "go.opentelemetry.io/collector/pipeline/internal/globalsignal"
 
-var SignalProfiles = globalsignal.MustNewSignal("profiles")
+var SignalProfiles = globalsignal.SignalProfiles

pipeline/xpipeline/config.go

@@ -53,19 +53,6 @@ func TestConfigValidate(t *testing.T) {
 			},
 			expected: errors.New(`references processor "nop" multiple times`),
 		},
-		{
-			name: "invalid-service-pipeline-type",
-			cfgFn: func() *Config {
-				cfg := generateConfig()
-				cfg.Pipelines[pipeline.MustNewID("wrongtype")] = &pipelines.PipelineConfig{
-					Receivers:  []component.ID{component.MustNewID("nop")},
-					Processors: []component.ID{component.MustNewID("nop")},
-					Exporters:  []component.ID{component.MustNewID("nop")},
-				}
-				return cfg
-			},
-			expected: errors.New(`pipeline "wrongtype": unknown signal "wrongtype"`),
-		},
 		{
 			name: "invalid-telemetry-metric-config",
 			cfgFn: func() *Config {