Skip to content

Commit

Permalink
[component] Add MustNewType constructor for component.Type (#9414)
Browse files Browse the repository at this point in the history
**Description:** 

- Adds `component.MustNewType` to create a type. This function panics if
the type has invalid characters. Add similar functions
`component.MustNewID` and `component.MustNewIDWithName`.
- Adds `component.Type.String` to recover the string
- Use `component.MustNewType`, `component.MustNewID`,
`component.MustNewIDWithName` and `component.Type.String` everywhere in
this codebase. To do this I changed `component.Type` into an opaque
struct and checked for compile-time errors.

Some notes:

1. All components currently on core and contrib follow this rule. This
is still breaking for other components.
2. A future PR will change this into a struct, to actually validate this
(right now you can just do `component.Type("anything")` to bypass
validation). I want to do this in two steps to avoid breaking contrib
tests: we first introduce this function, and after that we change into a
struct.

**Link to tracking Issue:** Updates #9208
  • Loading branch information
mx-psi committed Feb 2, 2024
1 parent 3665732 commit 26c157e
Show file tree
Hide file tree
Showing 96 changed files with 1,606 additions and 1,205 deletions.
26 changes: 26 additions & 0 deletions .chloggen/mx-psi_component-type-validation.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: component

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Validate component.Type at creation and unmarshaling time.

# One or more tracking issues or pull requests related to the change
issues: [9208]

# (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: |
- A component.Type must start with an ASCII alphabetic character and can only contain ASCII alphanumeric characters and '_'.
# 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]
5 changes: 4 additions & 1 deletion cmd/mdatagen/internal/metadata/generated_status.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions cmd/mdatagen/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,11 @@ import (
"go.opentelemetry.io/otel/trace"
)
var (
Type = component.MustNewType("foo")
)
const (
Type = "foo"
MetricsStability = component.StabilityLevelBeta
)
Expand Down Expand Up @@ -399,8 +402,11 @@ import (
"go.opentelemetry.io/otel/trace"
)
var (
Type = component.MustNewType("foo")
)
const (
Type = "foo"
MetricsStability = component.StabilityLevelAlpha
)
Expand Down
7 changes: 5 additions & 2 deletions cmd/mdatagen/templates/status.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ import (
"go.opentelemetry.io/otel/trace"
)

var (
Type = component.MustNewType("{{ .Type }}")
)

const (
Type = "{{ .Type }}"
{{- range $stability, $signals := .Status.Stability }}
{{- range $signal := $signals }}
{{ toCamelCase $signal }}Stability = component.StabilityLevel{{ casesTitle $stability }}
Expand All @@ -23,4 +26,4 @@ func Meter(settings component.TelemetrySettings) metric.Meter {

func Tracer(settings component.TelemetrySettings) trace.Tracer {
return settings.TracerProvider.Tracer("{{ .ScopeName }}")
}
}
11 changes: 11 additions & 0 deletions cmd/mdatagen/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package main
import (
"errors"
"fmt"
"regexp"

"go.uber.org/multierr"

Expand All @@ -29,10 +30,20 @@ func (md *metadata) Validate() error {
return errs
}

// typeRegexp is used to validate the type of a component.
// A type must start with an ASCII alphabetic character and
// can only contain ASCII alphanumeric characters and '_'.
// This must be kept in sync with the regex in component/config.go.
var typeRegexp = regexp.MustCompile(`^[a-zA-Z][0-9a-zA-Z_]*$`)

func (md *metadata) validateType() error {
if md.Type == "" {
return errors.New("missing type")
}

if !typeRegexp.MatchString(md.Type) {
return fmt.Errorf("invalid character(s) in type %q", md.Type)
}
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion component/componenttest/nop_host_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ func TestNewNopHost(t *testing.T) {
nh.ReportFatalError(errors.New("TestError"))
assert.Nil(t, nh.GetExporters()) // nolint: staticcheck
assert.Nil(t, nh.GetExtensions())
assert.Nil(t, nh.GetFactory(component.KindReceiver, "test"))
assert.Nil(t, nh.GetFactory(component.KindReceiver, component.MustNewType("test")))
}
8 changes: 4 additions & 4 deletions component/componenttest/otelprometheuschecker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ func TestPromChecker(t *testing.T) {
pc, err := newStubPromChecker()
require.NoError(t, err)

scraper := component.NewID("fakeScraper")
receiver := component.NewID("fakeReceiver")
processor := component.NewID("fakeProcessor")
exporter := component.NewID("fakeExporter")
scraper := component.MustNewID("fakeScraper")
receiver := component.MustNewID("fakeReceiver")
processor := component.MustNewID("fakeProcessor")
exporter := component.MustNewID("fakeExporter")
transport := "fakeTransport"

assert.NoError(t,
Expand Down
41 changes: 41 additions & 0 deletions component/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
package component // import "go.opentelemetry.io/collector/component"

import (
"fmt"
"reflect"
"regexp"

"go.uber.org/multierr"

Expand Down Expand Up @@ -110,6 +112,45 @@ func callValidateIfPossible(v reflect.Value) error {
// Type is the component type as it is used in the config.
type Type string

// String returns the string representation of the type.
func (t Type) String() string {
return string(t)
}

// typeRegexp is used to validate the type of a component.
// A type must start with an ASCII alphabetic character and
// can only contain ASCII alphanumeric characters and '_'.
// This must be kept in sync with the regex in cmd/mdatagen/validate.go.
var typeRegexp = regexp.MustCompile(`^[a-zA-Z][0-9a-zA-Z_]*$`)

// NewType creates a type. It returns an error if the type is invalid.
// A type must
// - have at least one character,
// - start with an ASCII alphabetic character and
// - can only contain ASCII alphanumeric characters and '_'.
func NewType(ty string) (Type, error) {
if len(ty) == 0 {
return Type(""), fmt.Errorf("id must not be empty")
}
if !typeRegexp.MatchString(ty) {
return Type(""), fmt.Errorf("invalid character(s) in type %q", ty)
}
return Type(ty), nil
}

// MustNewType creates a type. It panics if the type is invalid.
// A type must
// - have at least one character,
// - start with an ASCII alphabetic character and
// - can only contain ASCII alphanumeric characters and '_'.
func MustNewType(strType string) Type {
ty, err := NewType(strType)
if err != nil {
panic(err)
}
return ty
}

// DataType is a special Type that represents the data types supported by the collector. We currently support
// collecting metrics, traces and logs, this can expand in the future.
type DataType = Type
Expand Down

0 comments on commit 26c157e

Please sign in to comment.