Skip to content

Commit

Permalink
[processor/deltacumulative] fix meter panic (open-telemetry#35685)
Browse files Browse the repository at this point in the history
#### Description

As an oversight,
open-telemetry#35048
creates two `metadata.TelemetryBuilder` instances.
It also introduces an async metric, but one `TelemetryBuilder` sets no
callback for that, leading to a panic on `Collect()`.
Fixes that by using the same `TelemetryBuilder` for both, properly
setting the callback.

#### Testing

Test was added in first commit that passes after adding second commit
  • Loading branch information
sh0rez authored and AkhigbeEromo committed Oct 9, 2024
1 parent 811d0a5 commit 086c267
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 12 deletions.
27 changes: 27 additions & 0 deletions .chloggen/deltatocumulative-bug-meter.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# 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. filelogreceiver)
component: deltatocumulative

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: fix meter panic on startup

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [35685]

# (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: properly constructs the TelemetryBuilder, so it does not panic on startup, rendering the entire component unusable

# 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: [user]
8 changes: 2 additions & 6 deletions processor/deltatocumulativeprocessor/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,12 @@ func createMetricsProcessor(_ context.Context, set processor.Settings, cfg compo
return nil, fmt.Errorf("configuration parsing error")
}

telb, err := metadata.NewTelemetryBuilder(set.TelemetrySettings)
if err != nil {
return nil, err
}
proc := newProcessor(pcfg, set.Logger, telb, next)

ltel, err := ltel.New(set.TelemetrySettings)
if err != nil {
return nil, err
}

proc := newProcessor(pcfg, set.Logger, &ltel.TelemetryBuilder, next)
linear := newLinear(pcfg, ltel, proc)

return Chain{linear, proc}, nil
Expand Down
32 changes: 26 additions & 6 deletions processor/deltatocumulativeprocessor/processor_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package deltatocumulativeprocessor_test
package deltatocumulativeprocessor

import (
"context"
Expand All @@ -19,26 +19,26 @@ import (
"go.opentelemetry.io/collector/pdata/pmetric"
"go.opentelemetry.io/collector/processor"
"go.opentelemetry.io/collector/processor/processortest"
"go.opentelemetry.io/otel/sdk/metric/metricdata"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/exp/metrics/identity"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden"
self "github.com/open-telemetry/opentelemetry-collector-contrib/processor/deltatocumulativeprocessor"
"github.com/open-telemetry/opentelemetry-collector-contrib/processor/deltatocumulativeprocessor/internal/data"
"github.com/open-telemetry/opentelemetry-collector-contrib/processor/deltatocumulativeprocessor/internal/data/datatest/compare"
"github.com/open-telemetry/opentelemetry-collector-contrib/processor/deltatocumulativeprocessor/internal/metrics"
"github.com/open-telemetry/opentelemetry-collector-contrib/processor/deltatocumulativeprocessor/internal/streams"
"github.com/open-telemetry/opentelemetry-collector-contrib/processor/deltatocumulativeprocessor/internal/testdata/random"
)

func setup(t *testing.T, cfg *self.Config) (processor.Metrics, *consumertest.MetricsSink) {
func setup(t *testing.T, cfg *Config) (processor.Metrics, *consumertest.MetricsSink) {
t.Helper()

next := &consumertest.MetricsSink{}
if cfg == nil {
cfg = &self.Config{MaxStale: 0, MaxStreams: math.MaxInt}
cfg = &Config{MaxStale: 0, MaxStreams: math.MaxInt}
}

proc, err := self.NewFactory().CreateMetrics(
proc, err := NewFactory().CreateMetrics(
context.Background(),
processortest.NewNopSettings(),
cfg,
Expand Down Expand Up @@ -173,7 +173,7 @@ func TestTimestamps(t *testing.T) {
}

func TestStreamLimit(t *testing.T) {
proc, sink := setup(t, &self.Config{MaxStale: 5 * time.Minute, MaxStreams: 10})
proc, sink := setup(t, &Config{MaxStale: 5 * time.Minute, MaxStreams: 10})

good := make([]SumBuilder, 10)
for i := range good {
Expand Down Expand Up @@ -308,3 +308,23 @@ func TestIgnore(t *testing.T) {
t.Fatal(diff)
}
}

func TestTelemetry(t *testing.T) {
tt := setupTestTelemetry()

next := &consumertest.MetricsSink{}
cfg := createDefaultConfig()

_, err := NewFactory().CreateMetrics(
context.Background(),
tt.NewSettings(),
cfg,
next,
)
require.NoError(t, err)

var rm metricdata.ResourceMetrics
if err := tt.reader.Collect(context.Background(), &rm); err != nil {
t.Fatal(err)
}
}

0 comments on commit 086c267

Please sign in to comment.