[pipeline] Dreprecate MustNewID and MustNewIDWithName (open-telemetry#12835)

Updates
open-telemetry#12831

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

Author: bogdandrutu

.chloggen/deprecate-pipeline-must.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: deprecation
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: pipeline
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Deprecate MustNewID and MustNewIDWithName
+
+# One or more tracking issues or pull requests related to the change
+issues: [12831]
+
+# (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]

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.
@@ -20,32 +29,23 @@ func (s Signal) String() string {
 }
 
 // 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.
@@ -24,27 +22,27 @@ type ID struct {
 
 // NewID returns a new ID with the given Signal and empty name.
 func NewID(signal Signal) ID {
-	return ID{signal: signal}
+	return NewIDWithName(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.127.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.127.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)
@@ -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

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 {

service/config_test.go

@@ -30,14 +30,14 @@ var (
 	}
 
 	pIDs = []pipeline.ID{
-		pipeline.MustNewID("traces"),
-		pipeline.MustNewIDWithName("traces", "2"),
-		pipeline.MustNewID("metrics"),
-		pipeline.MustNewIDWithName("metrics", "2"),
-		pipeline.MustNewID("logs"),
-		pipeline.MustNewIDWithName("logs", "2"),
-		pipeline.MustNewID("profiles"),
-		pipeline.MustNewIDWithName("profiles", "2"),
+		pipeline.NewID(pipeline.SignalTraces),
+		pipeline.NewIDWithName(pipeline.SignalTraces, "2"),
+		pipeline.NewID(pipeline.SignalMetrics),
+		pipeline.NewIDWithName(pipeline.SignalMetrics, "2"),
+		pipeline.NewID(pipeline.SignalLogs),
+		pipeline.NewIDWithName(pipeline.SignalLogs, "2"),
+		pipeline.NewID(xpipeline.SignalProfiles),
+		pipeline.NewIDWithName(xpipeline.SignalProfiles, "2"),
 	}
 )