From 01a48d24363e7b1076bd10a92d69afbe3c3544e3 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Wed, 10 Sep 2025 00:02:48 +0800 Subject: [PATCH] [chore] service: inject default views via settings (#13775) #### Description Instead of updating the telemetry config, pass the default views in via settings and have the telemetry implementation use it as appropriate. This is necessary to decouple the service package from the otelconftelemetry config. #### Link to tracking issue Part of https://github.com/open-telemetry/opentelemetry-collector/issues/4970 #### Testing N/A, non-functional change. #### Documentation N/A --------- Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- service/service.go | 9 +- .../otelconftelemetry/metrics_test.go | 92 +++++++++++++++++++ .../telemetry/otelconftelemetry/telemetry.go | 4 +- service/telemetry/telemetry.go | 12 +++ 4 files changed, 110 insertions(+), 7 deletions(-) diff --git a/service/service.go b/service/service.go index aa89d26c33..f5dc034f19 100644 --- a/service/service.go +++ b/service/service.go @@ -124,14 +124,11 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) { } mpConfig := &cfg.Telemetry.Metrics.MeterProvider - if mpConfig.Views == nil { - mpConfig.Views = configureViews(cfg.Telemetry.Metrics.Level) - } - telFactory := otelconftelemetry.NewFactory() telset := telemetry.Settings{ - BuildInfo: set.BuildInfo, - ZapOptions: set.LoggingOptions, + BuildInfo: set.BuildInfo, + ZapOptions: set.LoggingOptions, + DefaultViews: configureViews, } telProviders, err := telFactory.CreateProviders(ctx, telset, &cfg.Telemetry) diff --git a/service/telemetry/otelconftelemetry/metrics_test.go b/service/telemetry/otelconftelemetry/metrics_test.go index 55ba81ee40..e933ae6468 100644 --- a/service/telemetry/otelconftelemetry/metrics_test.go +++ b/service/telemetry/otelconftelemetry/metrics_test.go @@ -6,7 +6,9 @@ package otelconftelemetry import ( "context" "fmt" + "io" "net/http" + "net/http/httptest" "testing" "time" @@ -20,6 +22,8 @@ import ( semconv "go.opentelemetry.io/otel/semconv/v1.26.0" "go.opentelemetry.io/collector/config/configtelemetry" + "go.opentelemetry.io/collector/pdata/pmetric" + "go.opentelemetry.io/collector/pdata/pmetric/pmetricotlp" "go.opentelemetry.io/collector/service/internal/promtest" "go.opentelemetry.io/collector/service/telemetry" ) @@ -255,3 +259,91 @@ func TestInstrumentEnabled(t *testing.T) { require.NoError(t, err) assert.Implements(t, new(enabledInstrument), floatGauge) } + +func TestTelemetryMetrics_DefaultViews(t *testing.T) { + type testcase struct { + configuredViews []config.View + expectedMeterNames []string + } + + defaultViews := func(level configtelemetry.Level) []config.View { + assert.Equal(t, configtelemetry.LevelDetailed, level) + return []config.View{{ + Selector: &config.ViewSelector{ + MeterName: ptr("a"), + }, + Stream: &config.ViewStream{ + Aggregation: &config.ViewStreamAggregation{ + Drop: config.ViewStreamAggregationDrop{}, + }, + }, + }} + } + + for name, tc := range map[string]testcase{ + "no_configured_views": { + expectedMeterNames: []string{"b", "c"}, + }, + "configured_views": { + configuredViews: []config.View{{ + Selector: &config.ViewSelector{ + MeterName: ptr("b"), + }, + Stream: &config.ViewStream{ + Aggregation: &config.ViewStreamAggregation{ + Drop: config.ViewStreamAggregationDrop{}, + }, + }, + }}, + expectedMeterNames: []string{"a", "c"}, + }, + } { + t.Run(name, func(t *testing.T) { + var metrics pmetric.Metrics + srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, req *http.Request) { + body, err := io.ReadAll(req.Body) + assert.NoError(t, err) + + exportRequest := pmetricotlp.NewExportRequest() + assert.NoError(t, exportRequest.UnmarshalProto(body)) + metrics = exportRequest.Metrics() + })) + defer srv.Close() + + cfg := createDefaultConfig().(*Config) + cfg.Metrics.Level = configtelemetry.LevelDetailed + cfg.Metrics.Views = tc.configuredViews + cfg.Metrics.Readers = []config.MetricReader{{ + Periodic: &config.PeriodicMetricReader{ + Exporter: config.PushMetricExporter{ + OTLP: &config.OTLPMetric{ + Endpoint: ptr(srv.URL), + Protocol: ptr("http/protobuf"), + Insecure: ptr(true), + }, + }, + }, + }} + + factory := NewFactory() + settings := telemetry.Settings{DefaultViews: defaultViews} + providers, err := factory.CreateProviders(t.Context(), settings, cfg) + require.NoError(t, err) + + mp := providers.MeterProvider() + for _, meterName := range []string{"a", "b", "c"} { + counter, _ := mp.Meter(meterName).Int64Counter(meterName + ".counter") + counter.Add(t.Context(), 1) + } + require.NoError(t, providers.Shutdown(t.Context())) // should flush metrics + + var scopes []string + for _, rm := range metrics.ResourceMetrics().All() { + for _, sm := range rm.ScopeMetrics().All() { + scopes = append(scopes, sm.Scope().Name()) + } + } + assert.ElementsMatch(t, tc.expectedMeterNames, scopes) + }) + } +} diff --git a/service/telemetry/otelconftelemetry/telemetry.go b/service/telemetry/otelconftelemetry/telemetry.go index c9802872c3..e1cf32ecde 100644 --- a/service/telemetry/otelconftelemetry/telemetry.go +++ b/service/telemetry/otelconftelemetry/telemetry.go @@ -121,10 +121,12 @@ func (t *otelconfTelemetry) Shutdown(ctx context.Context) error { return nil } -func newSDK(ctx context.Context, _ telemetry.Settings, cfg *Config, res *sdkresource.Resource) (*config.SDK, error) { +func newSDK(ctx context.Context, set telemetry.Settings, cfg *Config, res *sdkresource.Resource) (*config.SDK, error) { mpConfig := cfg.Metrics.MeterProvider if cfg.Metrics.Level == configtelemetry.LevelNone { mpConfig.Readers = nil + } else if mpConfig.Views == nil && set.DefaultViews != nil { + mpConfig.Views = set.DefaultViews(cfg.Metrics.Level) } // Merge the BuildInfo-based resource attributes with the user-defined resource attributes. diff --git a/service/telemetry/telemetry.go b/service/telemetry/telemetry.go index 34664586d6..8187672d7a 100644 --- a/service/telemetry/telemetry.go +++ b/service/telemetry/telemetry.go @@ -6,6 +6,7 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry" import ( "context" + otelconf "go.opentelemetry.io/contrib/otelconf/v0.3.0" "go.opentelemetry.io/otel/log" nooplog "go.opentelemetry.io/otel/log/noop" "go.opentelemetry.io/otel/metric" @@ -15,6 +16,7 @@ import ( "go.uber.org/zap" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/service/telemetry/internal/migration" ) @@ -67,6 +69,16 @@ type Settings struct { // ZapOptions contains options for creating the zap logger. ZapOptions []zap.Option + + // DefaultViews holds a function that returns default metric + // views for the given internal telemetry metrics level. + // + // The meter provider is expected to use this if no user-provided + // view configuration is supplied. + // + // TODO we should not use otelconf.View directly here, change + // to something independent of otelconf. + DefaultViews func(configtelemetry.Level) []otelconf.View } // Factory is a factory interface for internal telemetry.