Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[component] Add MustNewType constructor for component.Type #9414

Merged
merged 17 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
import (
"errors"
"fmt"
"regexp"

"go.uber.org/multierr"

Expand All @@ -29,10 +30,20 @@
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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call NewType and ignore return value, only return error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that that would be better, but it's not easy to do: it would mean adding a replace statement on cmd/mdatagen/go.mod for component, and that makes go install fail. See e.g. open-telemetry/opentelemetry-collector-contrib/issues/27855 for a recent instance of this issue with telemetrygen

if md.Type == "" {
return errors.New("missing type")
}

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

Check warning on line 46 in cmd/mdatagen/validate.go

View check run for this annotation

Codecov / codecov/patch

cmd/mdatagen/validate.go#L45-L46

Added lines #L45 - L46 were not covered by tests
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 @@
// 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
mx-psi marked this conversation as resolved.
Show resolved Hide resolved
// 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)

Check warning on line 149 in component/config.go

View check run for this annotation

Codecov / codecov/patch

component/config.go#L149

Added line #L149 was not covered by tests
}
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
Loading
Loading