From 037f10b5074cd7f6c5b0b5d926ea104c1c075bc2 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Mon, 16 Sep 2024 17:32:49 -0700 Subject: [PATCH] [chore] Move metrics initialization in service/telemetry Signed-off-by: Bogdan Drutu --- service/internal/promtest/server_util.go | 34 +++++++++++++++++++ service/service.go | 25 ++------------ service/service_test.go | 30 ++-------------- service/telemetry/factory.go | 23 +++++++++++++ .../{telemetry.go => telemetry/metrics.go} | 7 ++-- .../metrics_test.go} | 20 ++++++----- 6 files changed, 76 insertions(+), 63 deletions(-) create mode 100644 service/internal/promtest/server_util.go rename service/{telemetry.go => telemetry/metrics.go} (94%) rename service/{telemetry_test.go => telemetry/metrics_test.go} (95%) diff --git a/service/internal/promtest/server_util.go b/service/internal/promtest/server_util.go new file mode 100644 index 00000000000..47ed358a5bb --- /dev/null +++ b/service/internal/promtest/server_util.go @@ -0,0 +1,34 @@ +package promtest + +import ( + "net" + "strconv" + "testing" + + "go.opentelemetry.io/contrib/config" + + "go.opentelemetry.io/collector/internal/testutil" +) + +func GetAvailableLocalIPv6AddressPrometheus(t testing.TB) *config.Prometheus { + return addrToPrometheus(testutil.GetAvailableLocalIPv6Address(t)) +} + +func GetAvailableLocalAddressPrometheus(t testing.TB) *config.Prometheus { + return addrToPrometheus(testutil.GetAvailableLocalAddress(t)) +} + +func addrToPrometheus(address string) *config.Prometheus { + host, port, err := net.SplitHostPort(address) + if err != nil { + return nil + } + portInt, err := strconv.Atoi(port) + if err != nil { + return nil + } + return &config.Prometheus{ + Host: &host, + Port: &portInt, + } +} diff --git a/service/service.go b/service/service.go index 29bd3d57e92..b3371e92ae4 100644 --- a/service/service.go +++ b/service/service.go @@ -45,14 +45,6 @@ var useOtelWithSDKConfigurationForInternalTelemetryFeatureGate = featuregate.Glo featuregate.WithRegisterDescription("controls whether the collector supports extended OpenTelemetry"+ "configuration for internal telemetry")) -// disableHighCardinalityMetricsfeatureGate is the feature gate that controls whether the collector should enable -// potentially high cardinality metrics. The gate will be removed when the collector allows for view configuration. -var disableHighCardinalityMetricsfeatureGate = featuregate.GlobalRegistry().MustRegister( - "telemetry.disableHighCardinalityMetrics", - featuregate.StageAlpha, - featuregate.WithRegisterDescription("controls whether the collector should enable potentially high"+ - "cardinality metrics. The gate will be removed when the collector allows for view configuration.")) - // Settings holds configuration for building a new Service. type Settings struct { // BuildInfo provides collector start information. @@ -104,8 +96,6 @@ type Service struct { // New creates a new Service, its telemetry, and Components. func New(ctx context.Context, set Settings, cfg Config) (*Service, error) { - disableHighCard := disableHighCardinalityMetricsfeatureGate.IsEnabled() - srv := &Service{ buildInfo: set.BuildInfo, host: &graph.Host{ @@ -144,14 +134,7 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) { logger.Info("Setting up own telemetry...") - mp, err := newMeterProvider( - meterProviderSettings{ - res: res, - cfg: cfg.Telemetry.Metrics, - asyncErrorChannel: set.AsyncErrorChannel, - }, - disableHighCard, - ) + mp, err := telFactory.CreateMeterProvider(ctx, telset, &cfg.Telemetry) if err != nil { return nil, fmt.Errorf("failed to create metric provider: %w", err) } @@ -198,11 +181,7 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) { func logsAboutMeterProvider(logger *zap.Logger, cfg telemetry.MetricsConfig, mp metric.MeterProvider) { if cfg.Level == configtelemetry.LevelNone || (cfg.Address == "" && len(cfg.Readers) == 0) { - logger.Info( - "Skipped telemetry setup.", - zap.String(zapKeyTelemetryAddress, cfg.Address), - zap.Stringer(zapKeyTelemetryLevel, cfg.Level), - ) + logger.Info("Skipped telemetry setup.") return } diff --git a/service/service_test.go b/service/service_test.go index 8ca370b00da..bff18959569 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -8,9 +8,7 @@ import ( "context" "errors" "fmt" - "net" "net/http" - "strconv" "strings" "sync" "testing" @@ -35,6 +33,7 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/service/extensions" "go.opentelemetry.io/collector/service/internal/builders" + "go.opentelemetry.io/collector/service/internal/promtest" "go.opentelemetry.io/collector/service/pipelines" "go.opentelemetry.io/collector/service/telemetry" ) @@ -355,10 +354,10 @@ func testCollectorStartHelperWithReaders(t *testing.T, tc ownMetricsTestCase, ne ) switch network { case "tcp", "tcp4": - metricsAddr = getAvailableLocalAddressPrometheus(t) + metricsAddr = promtest.GetAvailableLocalAddressPrometheus(t) zpagesAddr = testutil.GetAvailableLocalAddress(t) case "tcp6": - metricsAddr = getAvailableLocalIPv6AddressPrometheus(t) + metricsAddr = promtest.GetAvailableLocalIPv6AddressPrometheus(t) zpagesAddr = testutil.GetAvailableLocalIPv6Address(t) } require.NotZero(t, metricsAddr, "network must be either of tcp, tcp4 or tcp6") @@ -745,26 +744,3 @@ func newConfigWatcherExtensionFactory(name component.Type) extension.Factory { component.StabilityLevelDevelopment, ) } - -func getAvailableLocalIPv6AddressPrometheus(t testing.TB) *config.Prometheus { - return addrToPrometheus(testutil.GetAvailableLocalIPv6Address(t)) -} - -func getAvailableLocalAddressPrometheus(t testing.TB) *config.Prometheus { - return addrToPrometheus(testutil.GetAvailableLocalAddress(t)) -} - -func addrToPrometheus(address string) *config.Prometheus { - host, port, err := net.SplitHostPort(address) - if err != nil { - return nil - } - portInt, err := strconv.Atoi(port) - if err != nil { - return nil - } - return &config.Prometheus{ - Host: &host, - Port: &portInt, - } -} diff --git a/service/telemetry/factory.go b/service/telemetry/factory.go index edcca9e373f..0fe91eef676 100644 --- a/service/telemetry/factory.go +++ b/service/telemetry/factory.go @@ -7,15 +7,26 @@ import ( "context" "time" + "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" "go.uber.org/zap/zapcore" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configtelemetry" + "go.opentelemetry.io/collector/featuregate" + "go.opentelemetry.io/collector/service/internal/resource" "go.opentelemetry.io/collector/service/telemetry/internal" ) +// disableHighCardinalityMetricsfeatureGate is the feature gate that controls whether the collector should enable +// potentially high cardinality metrics. The gate will be removed when the collector allows for view configuration. +var disableHighCardinalityMetricsfeatureGate = featuregate.GlobalRegistry().MustRegister( + "telemetry.disableHighCardinalityMetrics", + featuregate.StageAlpha, + featuregate.WithRegisterDescription("controls whether the collector should enable potentially high"+ + "cardinality metrics. The gate will be removed when the collector allows for view configuration.")) + func createDefaultConfig() component.Config { return &Config{ Logs: LogsConfig{ @@ -55,5 +66,17 @@ func NewFactory() Factory { c := *cfg.(*Config) return newTracerProvider(ctx, set, c) }), + internal.WithMeterProvider(func(ctx context.Context, set Settings, cfg component.Config) (metric.MeterProvider, error) { + c := *cfg.(*Config) + disableHighCard := disableHighCardinalityMetricsfeatureGate.IsEnabled() + return newMeterProvider( + meterProviderSettings{ + res: resource.New(set.BuildInfo, c.Resource), + cfg: c.Metrics, + asyncErrorChannel: set.AsyncErrorChannel, + }, + disableHighCard, + ) + }), ) } diff --git a/service/telemetry.go b/service/telemetry/metrics.go similarity index 94% rename from service/telemetry.go rename to service/telemetry/metrics.go index 7d3427e77bf..6c738bb46f8 100644 --- a/service/telemetry.go +++ b/service/telemetry/metrics.go @@ -1,7 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -package service // import "go.opentelemetry.io/collector/service" +package telemetry // import "go.opentelemetry.io/collector/service" import ( "context" @@ -20,7 +20,6 @@ import ( "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/service/internal/proctelemetry" - "go.opentelemetry.io/collector/service/telemetry" ) const ( @@ -36,7 +35,7 @@ type meterProvider struct { type meterProviderSettings struct { res *resource.Resource - cfg telemetry.MetricsConfig + cfg MetricsConfig asyncErrorChannel chan error } @@ -93,7 +92,7 @@ func newMeterProvider(set meterProviderSettings, disableHighCardinality bool) (m } // LogAboutServers logs about the servers that are serving metrics. -func (mp *meterProvider) LogAboutServers(logger *zap.Logger, cfg telemetry.MetricsConfig) { +func (mp *meterProvider) LogAboutServers(logger *zap.Logger, cfg MetricsConfig) { for _, server := range mp.servers { logger.Info( "Serving metrics", diff --git a/service/telemetry_test.go b/service/telemetry/metrics_test.go similarity index 95% rename from service/telemetry_test.go rename to service/telemetry/metrics_test.go index 7c2cb6541d7..8ff26a5f3dc 100644 --- a/service/telemetry_test.go +++ b/service/telemetry/metrics_test.go @@ -1,7 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -package service +package telemetry import ( "context" @@ -20,8 +20,8 @@ import ( "go.opentelemetry.io/collector/internal/testutil" semconv "go.opentelemetry.io/collector/semconv/v1.18.0" "go.opentelemetry.io/collector/service/internal/proctelemetry" + "go.opentelemetry.io/collector/service/internal/promtest" "go.opentelemetry.io/collector/service/internal/resource" - "go.opentelemetry.io/collector/service/telemetry" ) const ( @@ -32,6 +32,8 @@ const ( counterName = "test_counter" ) +var testInstanceID = "test_instance_id" + func TestTelemetryInit(t *testing.T) { type metricValue struct { value float64 @@ -43,7 +45,7 @@ func TestTelemetryInit(t *testing.T) { disableHighCard bool expectedMetrics map[string]metricValue extendedConfig bool - cfg *telemetry.Config + cfg *Config }{ { name: "UseOpenTelemetryForInternalMetrics", @@ -128,11 +130,11 @@ func TestTelemetryInit(t *testing.T) { { name: "UseOTelWithSDKConfiguration", extendedConfig: true, - cfg: &telemetry.Config{ - Metrics: telemetry.MetricsConfig{ + cfg: &Config{ + Metrics: MetricsConfig{ Level: configtelemetry.LevelDetailed, }, - Traces: telemetry.TracesConfig{ + Traces: TracesConfig{ Processors: []config.SpanProcessor{ { Batch: &config.BatchSpanProcessor{ @@ -194,18 +196,18 @@ func TestTelemetryInit(t *testing.T) { { Pull: &config.PullMetricReader{ Exporter: config.MetricExporter{ - Prometheus: getAvailableLocalAddressPrometheus(t), + Prometheus: promtest.GetAvailableLocalAddressPrometheus(t), }, }, }, } } if tc.cfg == nil { - tc.cfg = &telemetry.Config{ + tc.cfg = &Config{ Resource: map[string]*string{ semconv.AttributeServiceInstanceID: &testInstanceID, }, - Metrics: telemetry.MetricsConfig{ + Metrics: MetricsConfig{ Level: configtelemetry.LevelDetailed, Address: testutil.GetAvailableLocalAddress(t), },