From 8f65a8fee9f15f26e9aeb799f68cf1238ff69757 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 | 37 +++++++++++++++++++ service/service.go | 25 +------------ service/service_test.go | 30 ++------------- service/telemetry/factory.go | 23 ++++++++++++ .../internal/otelinit}/config.go | 2 +- .../internal/otelinit}/config_test.go | 2 +- .../{telemetry.go => telemetry/metrics.go} | 13 +++---- .../metrics_test.go} | 30 ++++++++------- 8 files changed, 89 insertions(+), 73 deletions(-) create mode 100644 service/internal/promtest/server_util.go rename service/{internal/proctelemetry => telemetry/internal/otelinit}/config.go (99%) rename service/{internal/proctelemetry => telemetry/internal/otelinit}/config_test.go (99%) rename service/{telemetry.go => telemetry/metrics.go} (85%) rename service/{telemetry_test.go => telemetry/metrics_test.go} (90%) diff --git a/service/internal/promtest/server_util.go b/service/internal/promtest/server_util.go new file mode 100644 index 00000000000..b0122d9f71e --- /dev/null +++ b/service/internal/promtest/server_util.go @@ -0,0 +1,37 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package promtest // import "go.opentelemetry.io/collector/service/internal/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/internal/proctelemetry/config.go b/service/telemetry/internal/otelinit/config.go similarity index 99% rename from service/internal/proctelemetry/config.go rename to service/telemetry/internal/otelinit/config.go index f36f9eb15c9..f56f60df44a 100644 --- a/service/internal/proctelemetry/config.go +++ b/service/telemetry/internal/otelinit/config.go @@ -1,7 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -package proctelemetry // import "go.opentelemetry.io/collector/service/internal/proctelemetry" +package otelinit // import "go.opentelemetry.io/collector/service/telemetry/internal/otelinit" import ( "context" diff --git a/service/internal/proctelemetry/config_test.go b/service/telemetry/internal/otelinit/config_test.go similarity index 99% rename from service/internal/proctelemetry/config_test.go rename to service/telemetry/internal/otelinit/config_test.go index efb081be0d4..c91e6f667e5 100644 --- a/service/internal/proctelemetry/config_test.go +++ b/service/telemetry/internal/otelinit/config_test.go @@ -1,7 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -package proctelemetry +package otelinit import ( "context" diff --git a/service/telemetry.go b/service/telemetry/metrics.go similarity index 85% rename from service/telemetry.go rename to service/telemetry/metrics.go index 7d3427e77bf..76b0a758b5a 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/telemetry" import ( "context" @@ -19,8 +19,7 @@ import ( "go.uber.org/zap" "go.opentelemetry.io/collector/config/configtelemetry" - "go.opentelemetry.io/collector/service/internal/proctelemetry" - "go.opentelemetry.io/collector/service/telemetry" + "go.opentelemetry.io/collector/service/telemetry/internal/otelinit" ) const ( @@ -36,7 +35,7 @@ type meterProvider struct { type meterProviderSettings struct { res *resource.Resource - cfg telemetry.MetricsConfig + cfg MetricsConfig asyncErrorChannel chan error } @@ -73,7 +72,7 @@ func newMeterProvider(set meterProviderSettings, disableHighCardinality bool) (m var opts []sdkmetric.Option for _, reader := range set.cfg.Readers { // https://github.com/open-telemetry/opentelemetry-collector/issues/8045 - r, server, err := proctelemetry.InitMetricReader(context.Background(), reader, set.asyncErrorChannel, &mp.serverWG) + r, server, err := otelinit.InitMetricReader(context.Background(), reader, set.asyncErrorChannel, &mp.serverWG) if err != nil { return nil, err } @@ -85,7 +84,7 @@ func newMeterProvider(set meterProviderSettings, disableHighCardinality bool) (m } var err error - mp.MeterProvider, err = proctelemetry.InitOpenTelemetry(set.res, opts, disableHighCardinality) + mp.MeterProvider, err = otelinit.InitOpenTelemetry(set.res, opts, disableHighCardinality) if err != nil { return nil, err } @@ -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 90% rename from service/telemetry_test.go rename to service/telemetry/metrics_test.go index 7c2cb6541d7..b4e0c24be70 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" @@ -19,9 +19,9 @@ import ( "go.opentelemetry.io/collector/config/configtelemetry" "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" + "go.opentelemetry.io/collector/service/telemetry/internal/otelinit" ) 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), }, @@ -253,13 +255,13 @@ func createTestMetrics(t *testing.T, mp metric.MeterProvider) { require.NoError(t, err) counter.Add(context.Background(), 13) - grpcExampleCounter, err := mp.Meter(proctelemetry.GRPCInstrumentation).Int64Counter(metricPrefix + grpcPrefix + counterName) + grpcExampleCounter, err := mp.Meter(otelinit.GRPCInstrumentation).Int64Counter(metricPrefix + grpcPrefix + counterName) require.NoError(t, err) - grpcExampleCounter.Add(context.Background(), 11, metric.WithAttributes(proctelemetry.GRPCUnacceptableKeyValues...)) + grpcExampleCounter.Add(context.Background(), 11, metric.WithAttributes(otelinit.GRPCUnacceptableKeyValues...)) - httpExampleCounter, err := mp.Meter(proctelemetry.HTTPInstrumentation).Int64Counter(metricPrefix + httpPrefix + counterName) + httpExampleCounter, err := mp.Meter(otelinit.HTTPInstrumentation).Int64Counter(metricPrefix + httpPrefix + counterName) require.NoError(t, err) - httpExampleCounter.Add(context.Background(), 10, metric.WithAttributes(proctelemetry.HTTPUnacceptableKeyValues...)) + httpExampleCounter.Add(context.Background(), 10, metric.WithAttributes(otelinit.HTTPUnacceptableKeyValues...)) } func getMetricsFromPrometheus(t *testing.T, handler http.Handler) map[string]*io_prometheus_client.MetricFamily {