From 6db952623df797ed06fd90f3bd002098fa9be7da Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Tue, 28 May 2024 10:19:32 -0700 Subject: [PATCH] [batchprocessor] ensure attributes are set on metadata metric Fixing this bug required a change in mdatagen to ensure attributes could be passed in to the telemetry builder. Fixes #9674 Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- .chloggen/codeboten_fix-9674-2.yaml | 25 +++++++++++++++++++ .chloggen/codeboten_fix-9674.yaml | 25 +++++++++++++++++++ .../internal/metadata/generated_telemetry.go | 11 +++++++- cmd/mdatagen/main.go | 8 ++++++ cmd/mdatagen/templates/telemetry.go.tmpl | 14 ++++++++++- .../batchprocessor/batch_processor_test.go | 3 ++- .../internal/metadata/generated_telemetry.go | 11 +++++++- processor/batchprocessor/metrics.go | 15 ++++++----- 8 files changed, 102 insertions(+), 10 deletions(-) create mode 100644 .chloggen/codeboten_fix-9674-2.yaml create mode 100644 .chloggen/codeboten_fix-9674.yaml diff --git a/.chloggen/codeboten_fix-9674-2.yaml b/.chloggen/codeboten_fix-9674-2.yaml new file mode 100644 index 00000000000..c51b1ad7b24 --- /dev/null +++ b/.chloggen/codeboten_fix-9674-2.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: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: mdatagen + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: support setting an AttributeSet for async instruments + +# One or more tracking issues or pull requests related to the change +issues: [9674] + +# (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: [] diff --git a/.chloggen/codeboten_fix-9674.yaml b/.chloggen/codeboten_fix-9674.yaml new file mode 100644 index 00000000000..9243a82621a --- /dev/null +++ b/.chloggen/codeboten_fix-9674.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: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: batchprocessor + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: ensure attributes are set on cardinality metadata metric + +# One or more tracking issues or pull requests related to the change +issues: [9674] + +# (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: [] diff --git a/cmd/mdatagen/internal/samplereceiver/internal/metadata/generated_telemetry.go b/cmd/mdatagen/internal/samplereceiver/internal/metadata/generated_telemetry.go index f40369d8c55..e6530a97a9a 100644 --- a/cmd/mdatagen/internal/samplereceiver/internal/metadata/generated_telemetry.go +++ b/cmd/mdatagen/internal/samplereceiver/internal/metadata/generated_telemetry.go @@ -6,6 +6,7 @@ import ( "context" "errors" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/noop" "go.opentelemetry.io/otel/trace" @@ -30,6 +31,7 @@ type TelemetryBuilder struct { observeProcessRuntimeTotalAllocBytes func() int64 RequestDuration metric.Float64Histogram level configtelemetry.Level + attributeSet attribute.Set } // telemetryBuilderOption applies changes to default builder. @@ -42,6 +44,13 @@ func WithLevel(lvl configtelemetry.Level) telemetryBuilderOption { } } +// WithAttributeSet applies a set of attributes for asynchronous instruments. +func WithAttributeSet(set attribute.Set) telemetryBuilderOption { + return func(builder *TelemetryBuilder) { + builder.attributeSet = set + } +} + // WithProcessRuntimeTotalAllocBytesCallback sets callback for observable ProcessRuntimeTotalAllocBytes metric. func WithProcessRuntimeTotalAllocBytesCallback(cb func() int64) telemetryBuilderOption { return func(builder *TelemetryBuilder) { @@ -76,7 +85,7 @@ func NewTelemetryBuilder(settings component.TelemetrySettings, options ...teleme metric.WithDescription("Cumulative bytes allocated for heap objects (see 'go doc runtime.MemStats.TotalAlloc')"), metric.WithUnit("By"), metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { - o.Observe(builder.observeProcessRuntimeTotalAllocBytes()) + o.Observe(builder.observeProcessRuntimeTotalAllocBytes(), metric.WithAttributeSet(builder.attributeSet)) return nil }), ) diff --git a/cmd/mdatagen/main.go b/cmd/mdatagen/main.go index cc325376826..304e12241cd 100644 --- a/cmd/mdatagen/main.go +++ b/cmd/mdatagen/main.go @@ -189,6 +189,14 @@ func templatize(tmplFile string, md metadata) *template.Template { } return result }, + "hasAsync": func(t telemetry) bool { + for _, m := range t.Metrics { + if m.Data().IsAsync() { + return true + } + } + return false + }, "inc": func(i int) int { return i + 1 }, "distroURL": func(name string) string { return distros[name] diff --git a/cmd/mdatagen/templates/telemetry.go.tmpl b/cmd/mdatagen/templates/telemetry.go.tmpl index 21c4e374849..fc976c91b0a 100644 --- a/cmd/mdatagen/templates/telemetry.go.tmpl +++ b/cmd/mdatagen/templates/telemetry.go.tmpl @@ -8,6 +8,7 @@ import ( "errors" {{- end }} + {{ if hasAsync .Telemetry }}"go.opentelemetry.io/otel/attribute"{{- end }} "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/noop" "go.opentelemetry.io/otel/trace" @@ -35,6 +36,7 @@ type TelemetryBuilder struct { {{- end }} {{- end }} level configtelemetry.Level + {{ if hasAsync .Telemetry }}attributeSet attribute.Set{{- end }} } // telemetryBuilderOption applies changes to default builder. @@ -47,6 +49,15 @@ func WithLevel(lvl configtelemetry.Level) telemetryBuilderOption { } } +{{- if hasAsync .Telemetry }} +// WithAttributeSet applies a set of attributes for asynchronous instruments. +func WithAttributeSet(set attribute.Set) telemetryBuilderOption { + return func(builder *TelemetryBuilder) { + builder.attributeSet = set + } +} +{{- end }} + {{- range $name, $metric := .Telemetry.Metrics }} {{ if $metric.Data.Async -}} // With{{ $name.Render }}Callback sets callback for observable {{ $name.Render }} metric. @@ -58,6 +69,7 @@ func With{{ $name.Render }}Callback(cb func() {{ $metric.Data.BasicType }}) tele {{- end }} {{- end }} + // NewTelemetryBuilder provides a struct with methods to update all internal telemetry // for a component func NewTelemetryBuilder(settings component.TelemetrySettings, options ...telemetryBuilderOption) (*TelemetryBuilder, error) { @@ -85,7 +97,7 @@ func NewTelemetryBuilder(settings component.TelemetrySettings, options ...teleme {{- end }} {{ if $metric.Data.Async -}} metric.With{{ casesTitle $metric.Data.BasicType }}Callback(func(_ context.Context, o metric.{{ casesTitle $metric.Data.BasicType }}Observer) error { - o.Observe(builder.observe{{ $name.Render }}()) + o.Observe(builder.observe{{ $name.Render }}(), metric.WithAttributeSet(builder.attributeSet)) return nil }), {{- end }} diff --git a/processor/batchprocessor/batch_processor_test.go b/processor/batchprocessor/batch_processor_test.go index 20c1270e5e5..5d3666a71c9 100644 --- a/processor/batchprocessor/batch_processor_test.go +++ b/processor/batchprocessor/batch_processor_test.go @@ -275,7 +275,8 @@ func TestBatchProcessorSentBySize(t *testing.T) { IsMonotonic: false, DataPoints: []metricdata.DataPoint[int64]{ { - Value: 1, + Value: 1, + Attributes: attribute.NewSet(attribute.String("processor", "batch")), }, }, }, diff --git a/processor/batchprocessor/internal/metadata/generated_telemetry.go b/processor/batchprocessor/internal/metadata/generated_telemetry.go index 2289368ab0b..76356cdc162 100644 --- a/processor/batchprocessor/internal/metadata/generated_telemetry.go +++ b/processor/batchprocessor/internal/metadata/generated_telemetry.go @@ -6,6 +6,7 @@ import ( "context" "errors" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/noop" "go.opentelemetry.io/otel/trace" @@ -32,6 +33,7 @@ type TelemetryBuilder struct { observeProcessorBatchMetadataCardinality func() int64 ProcessorBatchTimeoutTriggerSend metric.Int64Counter level configtelemetry.Level + attributeSet attribute.Set } // telemetryBuilderOption applies changes to default builder. @@ -44,6 +46,13 @@ func WithLevel(lvl configtelemetry.Level) telemetryBuilderOption { } } +// WithAttributeSet applies a set of attributes for asynchronous instruments. +func WithAttributeSet(set attribute.Set) telemetryBuilderOption { + return func(builder *TelemetryBuilder) { + builder.attributeSet = set + } +} + // WithProcessorBatchMetadataCardinalityCallback sets callback for observable ProcessorBatchMetadataCardinality metric. func WithProcessorBatchMetadataCardinalityCallback(cb func() int64) telemetryBuilderOption { return func(builder *TelemetryBuilder) { @@ -90,7 +99,7 @@ func NewTelemetryBuilder(settings component.TelemetrySettings, options ...teleme metric.WithDescription("Number of distinct metadata value combinations being processed"), metric.WithUnit("1"), metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { - o.Observe(builder.observeProcessorBatchMetadataCardinality()) + o.Observe(builder.observeProcessorBatchMetadataCardinality(), metric.WithAttributeSet(builder.attributeSet)) return nil }), ) diff --git a/processor/batchprocessor/metrics.go b/processor/batchprocessor/metrics.go index 4e0df628405..2aecaeeddc7 100644 --- a/processor/batchprocessor/metrics.go +++ b/processor/batchprocessor/metrics.go @@ -28,14 +28,17 @@ type batchProcessorTelemetry struct { exportCtx context.Context - processorAttr []attribute.KeyValue + processorAttr attribute.Set telemetryBuilder *metadata.TelemetryBuilder } func newBatchProcessorTelemetry(set processor.CreateSettings, currentMetadataCardinality func() int) (*batchProcessorTelemetry, error) { + attrs := attribute.NewSet(attribute.String(obsmetrics.ProcessorKey, set.ID.String())) + telemetryBuilder, err := metadata.NewTelemetryBuilder(set.TelemetrySettings, metadata.WithLevel(set.MetricsLevel), metadata.WithProcessorBatchMetadataCardinalityCallback(func() int64 { return int64(currentMetadataCardinality()) }), + metadata.WithAttributeSet(attrs), ) if err != nil { @@ -43,23 +46,23 @@ func newBatchProcessorTelemetry(set processor.CreateSettings, currentMetadataCar } return &batchProcessorTelemetry{ - processorAttr: []attribute.KeyValue{attribute.String(obsmetrics.ProcessorKey, set.ID.String())}, exportCtx: context.Background(), detailed: set.MetricsLevel == configtelemetry.LevelDetailed, telemetryBuilder: telemetryBuilder, + processorAttr: attrs, }, nil } func (bpt *batchProcessorTelemetry) record(trigger trigger, sent, bytes int64) { switch trigger { case triggerBatchSize: - bpt.telemetryBuilder.ProcessorBatchBatchSizeTriggerSend.Add(bpt.exportCtx, 1, metric.WithAttributes(bpt.processorAttr...)) + bpt.telemetryBuilder.ProcessorBatchBatchSizeTriggerSend.Add(bpt.exportCtx, 1, metric.WithAttributeSet(bpt.processorAttr)) case triggerTimeout: - bpt.telemetryBuilder.ProcessorBatchTimeoutTriggerSend.Add(bpt.exportCtx, 1, metric.WithAttributes(bpt.processorAttr...)) + bpt.telemetryBuilder.ProcessorBatchTimeoutTriggerSend.Add(bpt.exportCtx, 1, metric.WithAttributeSet(bpt.processorAttr)) } - bpt.telemetryBuilder.ProcessorBatchBatchSendSize.Record(bpt.exportCtx, sent, metric.WithAttributes(bpt.processorAttr...)) + bpt.telemetryBuilder.ProcessorBatchBatchSendSize.Record(bpt.exportCtx, sent, metric.WithAttributeSet(bpt.processorAttr)) if bpt.detailed { - bpt.telemetryBuilder.ProcessorBatchBatchSendSizeBytes.Record(bpt.exportCtx, bytes, metric.WithAttributes(bpt.processorAttr...)) + bpt.telemetryBuilder.ProcessorBatchBatchSendSizeBytes.Record(bpt.exportCtx, bytes, metric.WithAttributeSet(bpt.processorAttr)) } }