diff --git a/.chloggen/fix_40198_simple.yaml b/.chloggen/fix_40198_simple.yaml new file mode 100644 index 0000000000000..3ed683a65dd8b --- /dev/null +++ b/.chloggen/fix_40198_simple.yaml @@ -0,0 +1,28 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog) +component: pkg/ottl + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Return errors when OTTL context setters receive values of the wrong type + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [40198] + +# (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: | + Introduces `ctxutil.ExpectType` and updates log, metric, and scope setters to surface type assertion failures. + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# 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, user] diff --git a/pkg/ottl/contexts/internal/ctxlog/log.go b/pkg/ottl/contexts/internal/ctxlog/log.go index 6ed7db836efbe..76634cf146833 100644 --- a/pkg/ottl/contexts/internal/ctxlog/log.go +++ b/pkg/ottl/contexts/internal/ctxlog/log.go @@ -88,9 +88,11 @@ func accessTimeUnixNano[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetLogRecord().Timestamp().AsTime().UnixNano(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if i, ok := val.(int64); ok { - tCtx.GetLogRecord().SetTimestamp(pcommon.NewTimestampFromTime(time.Unix(0, i))) + i, err := ctxutil.ExpectType[int64](val) + if err != nil { + return err } + tCtx.GetLogRecord().SetTimestamp(pcommon.NewTimestampFromTime(time.Unix(0, i))) return nil }, } @@ -102,9 +104,11 @@ func accessObservedTimeUnixNano[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetLogRecord().ObservedTimestamp().AsTime().UnixNano(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if i, ok := val.(int64); ok { - tCtx.GetLogRecord().SetObservedTimestamp(pcommon.NewTimestampFromTime(time.Unix(0, i))) + i, err := ctxutil.ExpectType[int64](val) + if err != nil { + return err } + tCtx.GetLogRecord().SetObservedTimestamp(pcommon.NewTimestampFromTime(time.Unix(0, i))) return nil }, } @@ -116,9 +120,11 @@ func accessTime[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetLogRecord().Timestamp().AsTime(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if i, ok := val.(time.Time); ok { - tCtx.GetLogRecord().SetTimestamp(pcommon.NewTimestampFromTime(i)) + i, err := ctxutil.ExpectType[time.Time](val) + if err != nil { + return err } + tCtx.GetLogRecord().SetTimestamp(pcommon.NewTimestampFromTime(i)) return nil }, } @@ -130,9 +136,11 @@ func accessObservedTime[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetLogRecord().ObservedTimestamp().AsTime(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if i, ok := val.(time.Time); ok { - tCtx.GetLogRecord().SetObservedTimestamp(pcommon.NewTimestampFromTime(i)) + i, err := ctxutil.ExpectType[time.Time](val) + if err != nil { + return err } + tCtx.GetLogRecord().SetObservedTimestamp(pcommon.NewTimestampFromTime(i)) return nil }, } @@ -144,9 +152,11 @@ func accessSeverityNumber[K Context]() ottl.StandardGetSetter[K] { return int64(tCtx.GetLogRecord().SeverityNumber()), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if i, ok := val.(int64); ok { - tCtx.GetLogRecord().SetSeverityNumber(plog.SeverityNumber(i)) + i, err := ctxutil.ExpectType[int64](val) + if err != nil { + return err } + tCtx.GetLogRecord().SetSeverityNumber(plog.SeverityNumber(i)) return nil }, } @@ -158,9 +168,11 @@ func accessSeverityText[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetLogRecord().SeverityText(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if s, ok := val.(string); ok { - tCtx.GetLogRecord().SetSeverityText(s) + s, err := ctxutil.ExpectType[string](val) + if err != nil { + return err } + tCtx.GetLogRecord().SetSeverityText(s) return nil }, } @@ -210,9 +222,11 @@ func accessStringBody[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetLogRecord().Body().AsString(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if str, ok := val.(string); ok { - tCtx.GetLogRecord().Body().SetStr(str) + str, err := ctxutil.ExpectType[string](val) + if err != nil { + return err } + tCtx.GetLogRecord().Body().SetStr(str) return nil }, } @@ -246,9 +260,11 @@ func accessDroppedAttributesCount[K Context]() ottl.StandardGetSetter[K] { return int64(tCtx.GetLogRecord().DroppedAttributesCount()), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if i, ok := val.(int64); ok { - tCtx.GetLogRecord().SetDroppedAttributesCount(uint32(i)) + i, err := ctxutil.ExpectType[int64](val) + if err != nil { + return err } + tCtx.GetLogRecord().SetDroppedAttributesCount(uint32(i)) return nil }, } @@ -260,9 +276,11 @@ func accessFlags[K Context]() ottl.StandardGetSetter[K] { return int64(tCtx.GetLogRecord().Flags()), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if i, ok := val.(int64); ok { - tCtx.GetLogRecord().SetFlags(plog.LogRecordFlags(i)) + i, err := ctxutil.ExpectType[int64](val) + if err != nil { + return err } + tCtx.GetLogRecord().SetFlags(plog.LogRecordFlags(i)) return nil }, } @@ -274,9 +292,11 @@ func accessTraceID[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetLogRecord().TraceID(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if newTraceID, ok := val.(pcommon.TraceID); ok { - tCtx.GetLogRecord().SetTraceID(newTraceID) + newTraceID, err := ctxutil.ExpectType[pcommon.TraceID](val) + if err != nil { + return err } + tCtx.GetLogRecord().SetTraceID(newTraceID) return nil }, } @@ -289,13 +309,15 @@ func accessStringTraceID[K Context]() ottl.StandardGetSetter[K] { return hex.EncodeToString(id[:]), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if str, ok := val.(string); ok { - id, err := ctxcommon.ParseTraceID(str) - if err != nil { - return err - } - tCtx.GetLogRecord().SetTraceID(id) + str, err := ctxutil.ExpectType[string](val) + if err != nil { + return err } + id, err := ctxcommon.ParseTraceID(str) + if err != nil { + return err + } + tCtx.GetLogRecord().SetTraceID(id) return nil }, } @@ -307,9 +329,11 @@ func accessSpanID[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetLogRecord().SpanID(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if newSpanID, ok := val.(pcommon.SpanID); ok { - tCtx.GetLogRecord().SetSpanID(newSpanID) + newSpanID, err := ctxutil.ExpectType[pcommon.SpanID](val) + if err != nil { + return err } + tCtx.GetLogRecord().SetSpanID(newSpanID) return nil }, } @@ -322,13 +346,15 @@ func accessStringSpanID[K Context]() ottl.StandardGetSetter[K] { return hex.EncodeToString(id[:]), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if str, ok := val.(string); ok { - id, err := ctxcommon.ParseSpanID(str) - if err != nil { - return err - } - tCtx.GetLogRecord().SetSpanID(id) + str, err := ctxutil.ExpectType[string](val) + if err != nil { + return err + } + id, err := ctxcommon.ParseSpanID(str) + if err != nil { + return err } + tCtx.GetLogRecord().SetSpanID(id) return nil }, } @@ -340,9 +366,11 @@ func accessEventName[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetLogRecord().EventName(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if v, ok := val.(string); ok { - tCtx.GetLogRecord().SetEventName(v) + v, err := ctxutil.ExpectType[string](val) + if err != nil { + return err } + tCtx.GetLogRecord().SetEventName(v) return nil }, } diff --git a/pkg/ottl/contexts/internal/ctxmetric/metric.go b/pkg/ottl/contexts/internal/ctxmetric/metric.go index 6b4e916655add..9b4ed55aa3879 100644 --- a/pkg/ottl/contexts/internal/ctxmetric/metric.go +++ b/pkg/ottl/contexts/internal/ctxmetric/metric.go @@ -48,9 +48,11 @@ func accessName[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetMetric().Name(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if str, ok := val.(string); ok { - tCtx.GetMetric().SetName(str) + str, err := ctxutil.ExpectType[string](val) + if err != nil { + return err } + tCtx.GetMetric().SetName(str) return nil }, } @@ -62,9 +64,11 @@ func accessDescription[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetMetric().Description(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if str, ok := val.(string); ok { - tCtx.GetMetric().SetDescription(str) + str, err := ctxutil.ExpectType[string](val) + if err != nil { + return err } + tCtx.GetMetric().SetDescription(str) return nil }, } @@ -76,9 +80,11 @@ func accessUnit[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetMetric().Unit(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if str, ok := val.(string); ok { - tCtx.GetMetric().SetUnit(str) + str, err := ctxutil.ExpectType[string](val) + if err != nil { + return err } + tCtx.GetMetric().SetUnit(str) return nil }, } @@ -112,16 +118,18 @@ func accessAggTemporality[K Context]() ottl.StandardGetSetter[K] { return nil, nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if newAggTemporality, ok := val.(int64); ok { - metric := tCtx.GetMetric() - switch metric.Type() { - case pmetric.MetricTypeSum: - metric.Sum().SetAggregationTemporality(pmetric.AggregationTemporality(newAggTemporality)) - case pmetric.MetricTypeHistogram: - metric.Histogram().SetAggregationTemporality(pmetric.AggregationTemporality(newAggTemporality)) - case pmetric.MetricTypeExponentialHistogram: - metric.ExponentialHistogram().SetAggregationTemporality(pmetric.AggregationTemporality(newAggTemporality)) - } + newAggTemporality, err := ctxutil.ExpectType[int64](val) + if err != nil { + return err + } + metric := tCtx.GetMetric() + switch metric.Type() { + case pmetric.MetricTypeSum: + metric.Sum().SetAggregationTemporality(pmetric.AggregationTemporality(newAggTemporality)) + case pmetric.MetricTypeHistogram: + metric.Histogram().SetAggregationTemporality(pmetric.AggregationTemporality(newAggTemporality)) + case pmetric.MetricTypeExponentialHistogram: + metric.ExponentialHistogram().SetAggregationTemporality(pmetric.AggregationTemporality(newAggTemporality)) } return nil }, @@ -138,11 +146,13 @@ func accessIsMonotonic[K Context]() ottl.StandardGetSetter[K] { return nil, nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if newIsMonotonic, ok := val.(bool); ok { - metric := tCtx.GetMetric() - if metric.Type() == pmetric.MetricTypeSum { - metric.Sum().SetIsMonotonic(newIsMonotonic) - } + newIsMonotonic, err := ctxutil.ExpectType[bool](val) + if err != nil { + return err + } + metric := tCtx.GetMetric() + if metric.Type() == pmetric.MetricTypeSum { + metric.Sum().SetIsMonotonic(newIsMonotonic) } return nil }, @@ -171,25 +181,35 @@ func accessDataPoints[K Context]() ottl.StandardGetSetter[K] { metric := tCtx.GetMetric() switch metric.Type() { case pmetric.MetricTypeSum: - if newDataPoints, ok := val.(pmetric.NumberDataPointSlice); ok { - newDataPoints.CopyTo(metric.Sum().DataPoints()) + newDataPoints, err := ctxutil.ExpectType[pmetric.NumberDataPointSlice](val) + if err != nil { + return err } + newDataPoints.CopyTo(metric.Sum().DataPoints()) case pmetric.MetricTypeGauge: - if newDataPoints, ok := val.(pmetric.NumberDataPointSlice); ok { - newDataPoints.CopyTo(metric.Gauge().DataPoints()) + newDataPoints, err := ctxutil.ExpectType[pmetric.NumberDataPointSlice](val) + if err != nil { + return err } + newDataPoints.CopyTo(metric.Gauge().DataPoints()) case pmetric.MetricTypeHistogram: - if newDataPoints, ok := val.(pmetric.HistogramDataPointSlice); ok { - newDataPoints.CopyTo(metric.Histogram().DataPoints()) + newDataPoints, err := ctxutil.ExpectType[pmetric.HistogramDataPointSlice](val) + if err != nil { + return err } + newDataPoints.CopyTo(metric.Histogram().DataPoints()) case pmetric.MetricTypeExponentialHistogram: - if newDataPoints, ok := val.(pmetric.ExponentialHistogramDataPointSlice); ok { - newDataPoints.CopyTo(metric.ExponentialHistogram().DataPoints()) + newDataPoints, err := ctxutil.ExpectType[pmetric.ExponentialHistogramDataPointSlice](val) + if err != nil { + return err } + newDataPoints.CopyTo(metric.ExponentialHistogram().DataPoints()) case pmetric.MetricTypeSummary: - if newDataPoints, ok := val.(pmetric.SummaryDataPointSlice); ok { - newDataPoints.CopyTo(metric.Summary().DataPoints()) + newDataPoints, err := ctxutil.ExpectType[pmetric.SummaryDataPointSlice](val) + if err != nil { + return err } + newDataPoints.CopyTo(metric.Summary().DataPoints()) } return nil }, diff --git a/pkg/ottl/contexts/internal/ctxscope/scope.go b/pkg/ottl/contexts/internal/ctxscope/scope.go index 240d4cbc3b73a..0fa9f54cb25da 100644 --- a/pkg/ottl/contexts/internal/ctxscope/scope.go +++ b/pkg/ottl/contexts/internal/ctxscope/scope.go @@ -63,9 +63,11 @@ func accessInstrumentationScopeName[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetInstrumentationScope().Name(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if str, ok := val.(string); ok { - tCtx.GetInstrumentationScope().SetName(str) + str, err := ctxutil.ExpectType[string](val) + if err != nil { + return err } + tCtx.GetInstrumentationScope().SetName(str) return nil }, } @@ -77,9 +79,11 @@ func accessInstrumentationScopeVersion[K Context]() ottl.StandardGetSetter[K] { return tCtx.GetInstrumentationScope().Version(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if str, ok := val.(string); ok { - tCtx.GetInstrumentationScope().SetVersion(str) + str, err := ctxutil.ExpectType[string](val) + if err != nil { + return err } + tCtx.GetInstrumentationScope().SetVersion(str) return nil }, } @@ -91,9 +95,11 @@ func accessInstrumentationScopeDroppedAttributesCount[K Context]() ottl.Standard return int64(tCtx.GetInstrumentationScope().DroppedAttributesCount()), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if i, ok := val.(int64); ok { - tCtx.GetInstrumentationScope().SetDroppedAttributesCount(uint32(i)) + i, err := ctxutil.ExpectType[int64](val) + if err != nil { + return err } + tCtx.GetInstrumentationScope().SetDroppedAttributesCount(uint32(i)) return nil }, } @@ -105,9 +111,11 @@ func accessInstrumentationScopeSchemaURLItem[K Context]() ottl.StandardGetSetter return tCtx.GetScopeSchemaURLItem().SchemaUrl(), nil }, Setter: func(_ context.Context, tCtx K, val any) error { - if schemaURL, ok := val.(string); ok { - tCtx.GetScopeSchemaURLItem().SetSchemaUrl(schemaURL) + schemaURL, err := ctxutil.ExpectType[string](val) + if err != nil { + return err } + tCtx.GetScopeSchemaURLItem().SetSchemaUrl(schemaURL) return nil }, } diff --git a/pkg/ottl/contexts/internal/ctxutil/typecheck.go b/pkg/ottl/contexts/internal/ctxutil/typecheck.go new file mode 100644 index 0000000000000..597a901e5cde1 --- /dev/null +++ b/pkg/ottl/contexts/internal/ctxutil/typecheck.go @@ -0,0 +1,16 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package ctxutil // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxutil" + +import "fmt" + +// ExpectType ensures val can be asserted to T, returning a descriptive error when it cannot. +func ExpectType[T any](val any) (T, error) { + var zero T + typed, ok := val.(T) + if !ok { + return zero, fmt.Errorf("expects %T but got %T", zero, val) + } + return typed, nil +} diff --git a/pkg/ottl/contexts/internal/ctxutil/typecheck_test.go b/pkg/ottl/contexts/internal/ctxutil/typecheck_test.go new file mode 100644 index 0000000000000..3b5bffc44229c --- /dev/null +++ b/pkg/ottl/contexts/internal/ctxutil/typecheck_test.go @@ -0,0 +1,23 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package ctxutil + +import "testing" + +func TestExpectTypeSuccess(t *testing.T) { + got, err := ExpectType[int](42) + if err != nil { + t.Fatalf("expect success, got error %v", err) + } + if got != 42 { + t.Fatalf("expected 42, got %d", got) + } +} + +func TestExpectTypeError(t *testing.T) { + _, err := ExpectType[string](123) + if err == nil { + t.Fatalf("expected error, got nil") + } +}