Validate instrument names when creating them

CHANGELOG.md

@@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - Starting from `v1.21.0` of semantic conventions, `go.opentelemetry.io/otel/semconv/{version}/httpconv` and `go.opentelemetry.io/otel/semconv/{version}/netconv` packages will no longer be published. (#4145)
 - Log duplicate instrument conflict at a warning level instead of info in `go.opentelemetry.io/otel/sdk/metric`. (#4202)
+- Reject the creation of new instruments if their name doesn't pass regexp validation. (#4210)
 
 ## [1.16.0/0.39.0] 2023-05-18
 

exporters/prometheus/exporter_test.go

@@ -154,7 +154,7 @@ func TestPrometheusExporter(t *testing.T) {
 				gauge.Add(ctx, 100, opt)
 
 				counter, err := meter.Float64Counter("0invalid.counter.name", otelmetric.WithDescription("a counter with an invalid name"))
-				require.NoError(t, err)
+				require.Equal(t, err, metric.ErrInvalidInstrumentName)
 				counter.Add(ctx, 100, opt)
 
 				histogram, err := meter.Float64Histogram("invalid.hist.name", otelmetric.WithDescription("a histogram with an invalid name"))

sdk/metric/meter.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"regexp"
 
 	"go.opentelemetry.io/otel/internal/global"
 	"go.opentelemetry.io/otel/metric"
@@ -26,6 +27,14 @@ import (
 	"go.opentelemetry.io/otel/sdk/metric/internal"
 )
 
+var (
+	instrumentNameRe = regexp.MustCompile(`^([A-Za-z]){1}([A-Za-z0-9\_\-\.]){0,62}$`)
+
+	// ErrInvalidInstrumentName indicates the created instrument has an invalid name.
+	// Valid names must consist of 63 or fewer characters including alphanumeric, _, ., -, and start with a letter.
+	ErrInvalidInstrumentName = errors.New("invalid instrument name")
+)
+
 // meter handles the creation and coordination of all metric instruments. A
 // meter represents a single instrumentation scope; all metric telemetry
 // produced by an instrumentation scope will use metric instruments from a
@@ -91,11 +100,11 @@ func (m *meter) Int64ObservableCounter(name string, options ...metric.Int64Obser
 	const kind = InstrumentKindObservableCounter
 	p := int64ObservProvider{m.int64IP}
 	inst, err := p.lookup(kind, name, cfg.Description(), cfg.Unit())
-	if err != nil {
+	if err != nil && err != ErrInvalidInstrumentName {
 		return nil, err
 	}
 	p.registerCallbacks(inst, cfg.Callbacks())
-	return inst, nil
+	return inst, err
 }
 
 // Int64ObservableUpDownCounter returns a new instrument identified by name and
@@ -106,11 +115,11 @@ func (m *meter) Int64ObservableUpDownCounter(name string, options ...metric.Int6
 	const kind = InstrumentKindObservableUpDownCounter
 	p := int64ObservProvider{m.int64IP}
 	inst, err := p.lookup(kind, name, cfg.Description(), cfg.Unit())
-	if err != nil {
+	if err != nil && err != ErrInvalidInstrumentName {
 		return nil, err
 	}
 	p.registerCallbacks(inst, cfg.Callbacks())
-	return inst, nil
+	return inst, err
 }
 
 // Int64ObservableGauge returns a new instrument identified by name and
@@ -121,11 +130,11 @@ func (m *meter) Int64ObservableGauge(name string, options ...metric.Int64Observa
 	const kind = InstrumentKindObservableGauge
 	p := int64ObservProvider{m.int64IP}
 	inst, err := p.lookup(kind, name, cfg.Description(), cfg.Unit())
-	if err != nil {
+	if err != nil && err != ErrInvalidInstrumentName {
 		return nil, err
 	}
 	p.registerCallbacks(inst, cfg.Callbacks())
-	return inst, nil
+	return inst, err
 }
 
 // Float64Counter returns a new instrument identified by name and configured
@@ -163,11 +172,11 @@ func (m *meter) Float64ObservableCounter(name string, options ...metric.Float64O
 	const kind = InstrumentKindObservableCounter
 	p := float64ObservProvider{m.float64IP}
 	inst, err := p.lookup(kind, name, cfg.Description(), cfg.Unit())
-	if err != nil {
+	if err != nil && err != ErrInvalidInstrumentName {
 		return nil, err
 	}
 	p.registerCallbacks(inst, cfg.Callbacks())
-	return inst, nil
+	return inst, err
 }
 
 // Float64ObservableUpDownCounter returns a new instrument identified by name
@@ -178,11 +187,11 @@ func (m *meter) Float64ObservableUpDownCounter(name string, options ...metric.Fl
 	const kind = InstrumentKindObservableUpDownCounter
 	p := float64ObservProvider{m.float64IP}
 	inst, err := p.lookup(kind, name, cfg.Description(), cfg.Unit())
-	if err != nil {
+	if err != nil && err != ErrInvalidInstrumentName {
 		return nil, err
 	}
 	p.registerCallbacks(inst, cfg.Callbacks())
-	return inst, nil
+	return inst, err
 }
 
 // Float64ObservableGauge returns a new instrument identified by name and
@@ -193,11 +202,18 @@ func (m *meter) Float64ObservableGauge(name string, options ...metric.Float64Obs
 	const kind = InstrumentKindObservableGauge
 	p := float64ObservProvider{m.float64IP}
 	inst, err := p.lookup(kind, name, cfg.Description(), cfg.Unit())
-	if err != nil {
+	if err != nil && err != ErrInvalidInstrumentName {
 		return nil, err
 	}
 	p.registerCallbacks(inst, cfg.Callbacks())
-	return inst, nil
+	return inst, err
+}
+
+func validateInstrumentName(name string) error {
+	if !instrumentNameRe.MatchString(name) {
+		return ErrInvalidInstrumentName
+	}
+	return nil
 }
 
 // RegisterCallback registers f to be called each collection cycle so it will
@@ -392,7 +408,12 @@ func (p *int64InstProvider) aggs(kind InstrumentKind, name, desc, u string) ([]i
 // lookup returns the resolved instrumentImpl.
 func (p *int64InstProvider) lookup(kind InstrumentKind, name, desc, u string) (*int64Inst, error) {
 	aggs, err := p.aggs(kind, name, desc, u)
-	return &int64Inst{aggregators: aggs}, err
+	inst := &int64Inst{aggregators: aggs}
+	if err != nil {
+		return inst, err
+	}
+
+	return inst, validateInstrumentName(name)
 }
 
 // float64InstProvider provides float64 OpenTelemetry instruments.
@@ -420,14 +441,24 @@ func (p *float64InstProvider) aggs(kind InstrumentKind, name, desc, u string) ([
 // lookup returns the resolved instrumentImpl.
 func (p *float64InstProvider) lookup(kind InstrumentKind, name, desc, u string) (*float64Inst, error) {
 	aggs, err := p.aggs(kind, name, desc, u)
-	return &float64Inst{aggregators: aggs}, err
+	inst := &float64Inst{aggregators: aggs}
+	if err != nil {
+		return inst, err
+	}
+
+	return inst, validateInstrumentName(name)
 }
 
 type int64ObservProvider struct{ *int64InstProvider }
 
 func (p int64ObservProvider) lookup(kind InstrumentKind, name, desc, u string) (int64Observable, error) {
 	aggs, err := p.aggs(kind, name, desc, u)
-	return newInt64Observable(p.scope, kind, name, desc, u, aggs), err
+	inst := newInt64Observable(p.scope, kind, name, desc, u, aggs)
+	if err != nil {
+		return inst, err
+	}
+
+	return inst, validateInstrumentName(name)
 }
 
 func (p int64ObservProvider) registerCallbacks(inst int64Observable, cBacks []metric.Int64Callback) {
@@ -460,7 +491,12 @@ type float64ObservProvider struct{ *float64InstProvider }
 
 func (p float64ObservProvider) lookup(kind InstrumentKind, name, desc, u string) (float64Observable, error) {
 	aggs, err := p.aggs(kind, name, desc, u)
-	return newFloat64Observable(p.scope, kind, name, desc, u, aggs), err
+	inst := newFloat64Observable(p.scope, kind, name, desc, u, aggs)
+	if err != nil {
+		return inst, err
+	}
+
+	return inst, validateInstrumentName(name)
 }
 
 func (p float64ObservProvider) registerCallbacks(inst float64Observable, cBacks []metric.Float64Callback) {