From b654e2b1b476dde768b1dac3ff9d1e0265848d8f Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 18 Apr 2023 10:30:03 +0200 Subject: [PATCH 1/6] put otel meter for http/grpc behind a FF (#202) --- config/configgrpc/configgrpc.go | 6 +++++- config/confighttp/confighttp.go | 12 +++++++++--- internal/obsreportconfig/obsreportconfig.go | 8 ++++++++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 68e79942a25..20469d705bb 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -45,6 +45,7 @@ import ( "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/config/internal" "go.opentelemetry.io/collector/extension/auth" + "go.opentelemetry.io/collector/internal/obsreportconfig" ) var errMetadataNotFound = errors.New("no request metadata found") @@ -260,10 +261,13 @@ func (gcs *GRPCClientSettings) toDialOptions(host component.Host, settings compo otelOpts := []otelgrpc.Option{ otelgrpc.WithTracerProvider(settings.TracerProvider), - otelgrpc.WithMeterProvider(settings.MeterProvider), otelgrpc.WithPropagators(otel.GetTextMapPropagator()), } + if obsreportconfig.EnableHighCardinalityMetricsfeatureGate.IsEnabled() { + otelOpts = append(otelOpts, otelgrpc.WithMeterProvider(settings.MeterProvider)) + } + // Enable OpenTelemetry observability plugin. opts = append(opts, grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor(otelOpts...))) opts = append(opts, grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor(otelOpts...))) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 99a86d6bd64..88128e84881 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -33,6 +33,7 @@ import ( "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/config/internal" "go.opentelemetry.io/collector/extension/auth" + "go.opentelemetry.io/collector/internal/obsreportconfig" ) const headerContentEncoding = "Content-Encoding" @@ -143,11 +144,16 @@ func (hcs *HTTPClientSettings) ToClient(host component.Host, settings component. } // wrapping http transport with otelhttp transport to enable otel instrumenetation if settings.TracerProvider != nil && settings.MeterProvider != nil { - clientTransport = otelhttp.NewTransport( - clientTransport, + otelOpts := []otelhttp.Option{ otelhttp.WithTracerProvider(settings.TracerProvider), - otelhttp.WithMeterProvider(settings.MeterProvider), otelhttp.WithPropagators(otel.GetTextMapPropagator()), + } + if obsreportconfig.EnableHighCardinalityMetricsfeatureGate.IsEnabled() { + otelOpts = append(otelOpts, otelhttp.WithMeterProvider(settings.MeterProvider)) + } + clientTransport = otelhttp.NewTransport( + clientTransport, + otelOpts..., ) } diff --git a/internal/obsreportconfig/obsreportconfig.go b/internal/obsreportconfig/obsreportconfig.go index c4e03edf0bb..f090fac131d 100644 --- a/internal/obsreportconfig/obsreportconfig.go +++ b/internal/obsreportconfig/obsreportconfig.go @@ -31,6 +31,14 @@ var UseOtelForInternalMetricsfeatureGate = featuregate.GlobalRegistry().MustRegi featuregate.StageAlpha, featuregate.WithRegisterDescription("controls whether the collector uses OpenTelemetry for internal metrics")) +// EnableHighCardinalityMetricsfeatureGate is the feature gate that controls whether the collecor should enable +// potentially high cardinality metrics. +var EnableHighCardinalityMetricsfeatureGate = featuregate.GlobalRegistry().MustRegister( + "telemetry.enableHighCardinalityMetrics", + featuregate.StageAlpha, + featuregate.WithRegisterDescription("controls whether the collecor should enable potentially high"+ + "cardinality metrics.")) + // AllViews returns all the OpenCensus views requires by obsreport package. func AllViews(level configtelemetry.Level) []*view.View { if level == configtelemetry.LevelNone { From d427f84641ab79157b23a5f6fd4db559063a56ea Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 18 Apr 2023 10:37:31 +0200 Subject: [PATCH 2/6] Chloggen --- .chloggen/7517-behind-ff.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100755 .chloggen/7517-behind-ff.yaml diff --git a/.chloggen/7517-behind-ff.yaml b/.chloggen/7517-behind-ff.yaml new file mode 100755 index 00000000000..8cf505c63b2 --- /dev/null +++ b/.chloggen/7517-behind-ff.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: observavibility + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Puts the meter provider for otlp grpc and otlp http receivers behind a feature flag due to high cardinality issues. + +# One or more tracking issues or pull requests related to the change +issues: [7517] + +# (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: From 82fd8b0bc5d070bcddc354d6667ab1382cbb52fa Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 25 Apr 2023 17:48:37 -0400 Subject: [PATCH 3/6] Switched to use a view, added a test --- config/configgrpc/configgrpc.go | 6 +- config/confighttp/confighttp.go | 12 +--- internal/obsreportconfig/obsreportconfig.go | 8 +-- service/service.go | 3 +- service/telemetry.go | 59 ++++++++++++++++++-- service/telemetry_test.go | 61 ++++++++++++++++++++- 6 files changed, 122 insertions(+), 27 deletions(-) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 20469d705bb..68e79942a25 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -45,7 +45,6 @@ import ( "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/config/internal" "go.opentelemetry.io/collector/extension/auth" - "go.opentelemetry.io/collector/internal/obsreportconfig" ) var errMetadataNotFound = errors.New("no request metadata found") @@ -261,13 +260,10 @@ func (gcs *GRPCClientSettings) toDialOptions(host component.Host, settings compo otelOpts := []otelgrpc.Option{ otelgrpc.WithTracerProvider(settings.TracerProvider), + otelgrpc.WithMeterProvider(settings.MeterProvider), otelgrpc.WithPropagators(otel.GetTextMapPropagator()), } - if obsreportconfig.EnableHighCardinalityMetricsfeatureGate.IsEnabled() { - otelOpts = append(otelOpts, otelgrpc.WithMeterProvider(settings.MeterProvider)) - } - // Enable OpenTelemetry observability plugin. opts = append(opts, grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor(otelOpts...))) opts = append(opts, grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor(otelOpts...))) diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 88128e84881..99a86d6bd64 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -33,7 +33,6 @@ import ( "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/config/internal" "go.opentelemetry.io/collector/extension/auth" - "go.opentelemetry.io/collector/internal/obsreportconfig" ) const headerContentEncoding = "Content-Encoding" @@ -144,16 +143,11 @@ func (hcs *HTTPClientSettings) ToClient(host component.Host, settings component. } // wrapping http transport with otelhttp transport to enable otel instrumenetation if settings.TracerProvider != nil && settings.MeterProvider != nil { - otelOpts := []otelhttp.Option{ - otelhttp.WithTracerProvider(settings.TracerProvider), - otelhttp.WithPropagators(otel.GetTextMapPropagator()), - } - if obsreportconfig.EnableHighCardinalityMetricsfeatureGate.IsEnabled() { - otelOpts = append(otelOpts, otelhttp.WithMeterProvider(settings.MeterProvider)) - } clientTransport = otelhttp.NewTransport( clientTransport, - otelOpts..., + otelhttp.WithTracerProvider(settings.TracerProvider), + otelhttp.WithMeterProvider(settings.MeterProvider), + otelhttp.WithPropagators(otel.GetTextMapPropagator()), ) } diff --git a/internal/obsreportconfig/obsreportconfig.go b/internal/obsreportconfig/obsreportconfig.go index f090fac131d..459406bfccb 100644 --- a/internal/obsreportconfig/obsreportconfig.go +++ b/internal/obsreportconfig/obsreportconfig.go @@ -31,12 +31,12 @@ var UseOtelForInternalMetricsfeatureGate = featuregate.GlobalRegistry().MustRegi featuregate.StageAlpha, featuregate.WithRegisterDescription("controls whether the collector uses OpenTelemetry for internal metrics")) -// EnableHighCardinalityMetricsfeatureGate is the feature gate that controls whether the collecor should enable +// DisableHighCardinalityMetricsfeatureGate is the feature gate that controls whether the collector should enable // potentially high cardinality metrics. -var EnableHighCardinalityMetricsfeatureGate = featuregate.GlobalRegistry().MustRegister( - "telemetry.enableHighCardinalityMetrics", +var DisableHighCardinalityMetricsfeatureGate = featuregate.GlobalRegistry().MustRegister( + "telemetry.disableHighCardinalityMetrics", featuregate.StageAlpha, - featuregate.WithRegisterDescription("controls whether the collecor should enable potentially high"+ + featuregate.WithRegisterDescription("controls whether the collector should enable potentially high"+ "cardinality metrics.")) // AllViews returns all the OpenCensus views requires by obsreport package. diff --git a/service/service.go b/service/service.go index e3f3f2999ca..075b01d5d89 100644 --- a/service/service.go +++ b/service/service.go @@ -81,6 +81,7 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) { if set.useOtel != nil { useOtel = *set.useOtel } + disableHighCard := obsreportconfig.DisableHighCardinalityMetricsfeatureGate.IsEnabled() srv := &Service{ buildInfo: set.BuildInfo, host: &serviceHost{ @@ -92,7 +93,7 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) { buildInfo: set.BuildInfo, asyncErrorChannel: set.AsyncErrorChannel, }, - telemetryInitializer: newColTelemetry(useOtel), + telemetryInitializer: newColTelemetry(useOtel, disableHighCard), } var err error srv.telemetry, err = telemetry.New(ctx, telemetry.Settings{ZapOptions: set.LoggingOptions}, cfg.Telemetry) diff --git a/service/telemetry.go b/service/telemetry.go index 96c8859a93e..ce4df650719 100644 --- a/service/telemetry.go +++ b/service/telemetry.go @@ -22,6 +22,8 @@ import ( "strings" "unicode" + "go.opentelemetry.io/otel/sdk/instrumentation" + ocprom "contrib.go.opencensus.io/exporter/prometheus" "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" @@ -46,7 +48,7 @@ import ( "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/internal/obsreportconfig" "go.opentelemetry.io/collector/obsreport" - semconv "go.opentelemetry.io/collector/semconv/v1.5.0" + semconv "go.opentelemetry.io/collector/semconv/v1.18.0" "go.opentelemetry.io/collector/service/telemetry" ) @@ -57,10 +59,29 @@ const ( // supported trace propagators traceContextPropagator = "tracecontext" b3Propagator = "b3" + + // gRPC Instrumentation Name + grpcInstrumentation = "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" + + // http Instrumentation Name + httpInstrumentation = "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" ) var ( errUnsupportedPropagator = errors.New("unsupported trace propagator") + + // grpcUnacceptableKeyValues is a list of high cardinality grpc attributes that should be filtered out. + grpcUnacceptableKeyValues = []attribute.KeyValue{ + attribute.String(semconv.AttributeNetSockPeerAddr, ""), + attribute.String(semconv.AttributeNetSockPeerPort, ""), + attribute.String(semconv.AttributeNetSockPeerName, ""), + } + + // httpUnacceptableKeyValues is a list of high cardinality http attributes that should be filtered out. + httpUnacceptableKeyValues = []attribute.KeyValue{ + attribute.String(semconv.AttributeNetHostName, ""), + attribute.String(semconv.AttributeNetHostPort, ""), + } ) type telemetryInitializer struct { @@ -69,13 +90,15 @@ type telemetryInitializer struct { mp metric.MeterProvider servers []*http.Server - useOtel bool + useOtel bool + disableHighCardinality bool } -func newColTelemetry(useOtel bool) *telemetryInitializer { +func newColTelemetry(useOtel bool, disableHighCardinality bool) *telemetryInitializer { return &telemetryInitializer{ - mp: metric.NewNoopMeterProvider(), - useOtel: useOtel, + mp: metric.NewNoopMeterProvider(), + useOtel: useOtel, + disableHighCardinality: disableHighCardinality, } } @@ -225,10 +248,27 @@ func (tel *telemetryInitializer) initOpenTelemetry(attrs map[string]string, prom return fmt.Errorf("error creating otel prometheus exporter: %w", err) } exporter.RegisterProducer(opencensus.NewMetricProducer()) + views := batchViews() + if tel.disableHighCardinality { + views = append(views, sdkmetric.NewView(sdkmetric.Instrument{ + Scope: instrumentation.Scope{ + Name: grpcInstrumentation, + }, + }, sdkmetric.Stream{ + AttributeFilter: cardinalityFilter(grpcUnacceptableKeyValues...), + })) + views = append(views, sdkmetric.NewView(sdkmetric.Instrument{ + Scope: instrumentation.Scope{ + Name: httpInstrumentation, + }, + }, sdkmetric.Stream{ + AttributeFilter: cardinalityFilter(httpUnacceptableKeyValues...), + })) + } tel.mp = sdkmetric.NewMeterProvider( sdkmetric.WithResource(res), sdkmetric.WithReader(exporter), - sdkmetric.WithView(batchViews()...), + sdkmetric.WithView(views...), ) return nil @@ -247,6 +287,13 @@ func (tel *telemetryInitializer) shutdown() error { return errs } +func cardinalityFilter(kvs ...attribute.KeyValue) attribute.Filter { + filter := attribute.NewSet(kvs...) + return func(kv attribute.KeyValue) bool { + return !filter.HasValue(kv.Key) + } +} + func sanitizePrometheusKey(str string) string { runeFilterMap := func(r rune) rune { if unicode.IsDigit(r) || unicode.IsLetter(r) || r == '_' { diff --git a/service/telemetry_test.go b/service/telemetry_test.go index 67049628cf0..dd2484f4198 100644 --- a/service/telemetry_test.go +++ b/service/telemetry_test.go @@ -33,7 +33,7 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/internal/testutil" - semconv "go.opentelemetry.io/collector/semconv/v1.5.0" + semconv "go.opentelemetry.io/collector/semconv/v1.18.0" "go.opentelemetry.io/collector/service/telemetry" ) @@ -41,6 +41,8 @@ const ( metricPrefix = "otelcol_" otelPrefix = "otel_sdk_" ocPrefix = "oc_sdk_" + grpcPrefix = "gprc_" + httpPrefix = "http_" counterName = "test_counter" ) @@ -97,6 +99,7 @@ func TestTelemetryInit(t *testing.T) { for _, tc := range []struct { name string useOtel bool + disableHighCard bool expectedMetrics map[string]metricValue }{ { @@ -125,6 +128,52 @@ func TestTelemetryInit(t *testing.T) { value: 13, labels: map[string]string{}, }, + metricPrefix + grpcPrefix + counterName + "_total": { + value: 11, + labels: map[string]string{ + "net_sock_peer_addr": "", + "net_sock_peer_name": "", + "net_sock_peer_port": "", + }, + }, + metricPrefix + httpPrefix + counterName + "_total": { + value: 10, + labels: map[string]string{ + "net_host_name": "", + "net_host_port": "", + }, + }, + metricPrefix + "target_info": { + value: 0, + labels: map[string]string{ + "service_name": "otelcol", + "service_version": "latest", + "service_instance_id": testInstanceID, + }, + }, + }, + }, + { + name: "DisableHighCardinalityWithOtel", + useOtel: true, + disableHighCard: true, + expectedMetrics: map[string]metricValue{ + metricPrefix + ocPrefix + counterName + "_total": { + value: 13, + labels: map[string]string{}, + }, + metricPrefix + otelPrefix + counterName + "_total": { + value: 13, + labels: map[string]string{}, + }, + metricPrefix + grpcPrefix + counterName + "_total": { + value: 11, + labels: map[string]string{}, + }, + metricPrefix + httpPrefix + counterName + "_total": { + value: 10, + labels: map[string]string{}, + }, metricPrefix + "target_info": { value: 0, labels: map[string]string{ @@ -137,7 +186,7 @@ func TestTelemetryInit(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - tel := newColTelemetry(tc.useOtel) + tel := newColTelemetry(tc.useOtel, tc.disableHighCard) buildInfo := component.NewDefaultBuildInfo() cfg := telemetry.Config{ Resource: map[string]*string{ @@ -187,6 +236,14 @@ func createTestMetrics(t *testing.T, mp metric.MeterProvider) *view.View { require.NoError(t, err) counter.Add(context.Background(), 13) + grpcExampleCounter, err := mp.Meter(grpcInstrumentation).Int64Counter(grpcPrefix + counterName) + require.NoError(t, err) + grpcExampleCounter.Add(context.Background(), 11, grpcUnacceptableKeyValues...) + + httpExampleCounter, err := mp.Meter(httpInstrumentation).Int64Counter(httpPrefix + counterName) + require.NoError(t, err) + httpExampleCounter.Add(context.Background(), 10, httpUnacceptableKeyValues...) + // Creates a OpenCensus measure ocCounter := stats.Int64(ocPrefix+counterName, counterName, stats.UnitDimensionless) v := &view.View{ From 7afe7adc6ffac5f338b13f9662a69fecdb65f6ca Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 25 Apr 2023 17:52:12 -0400 Subject: [PATCH 4/6] Update the chloggen, deps --- .chloggen/7517-behind-ff.yaml | 4 ++-- service/telemetry.go | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.chloggen/7517-behind-ff.yaml b/.chloggen/7517-behind-ff.yaml index 8cf505c63b2..237c1c0fcbf 100755 --- a/.chloggen/7517-behind-ff.yaml +++ b/.chloggen/7517-behind-ff.yaml @@ -1,11 +1,11 @@ # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' -change_type: breaking +change_type: enhancement # The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) component: observavibility # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: Puts the meter provider for otlp grpc and otlp http receivers behind a feature flag due to high cardinality issues. +note: Allows users to disable high cardinality OTLP attributes behind a feature flag. # One or more tracking issues or pull requests related to the change issues: [7517] diff --git a/service/telemetry.go b/service/telemetry.go index ce4df650719..39e391757d9 100644 --- a/service/telemetry.go +++ b/service/telemetry.go @@ -22,8 +22,6 @@ import ( "strings" "unicode" - "go.opentelemetry.io/otel/sdk/instrumentation" - ocprom "contrib.go.opencensus.io/exporter/prometheus" "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" @@ -38,6 +36,7 @@ import ( otelprom "go.opentelemetry.io/otel/exporters/prometheus" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/propagation" + "go.opentelemetry.io/otel/sdk/instrumentation" sdkmetric "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/resource" From c37aad7b01ea5510fed8cb8684c494033f944cad Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 26 Apr 2023 15:06:12 -0400 Subject: [PATCH 5/6] Docstring --- internal/obsreportconfig/obsreportconfig.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/obsreportconfig/obsreportconfig.go b/internal/obsreportconfig/obsreportconfig.go index 459406bfccb..21501d6e8d3 100644 --- a/internal/obsreportconfig/obsreportconfig.go +++ b/internal/obsreportconfig/obsreportconfig.go @@ -32,12 +32,12 @@ var UseOtelForInternalMetricsfeatureGate = featuregate.GlobalRegistry().MustRegi featuregate.WithRegisterDescription("controls whether the collector uses OpenTelemetry for internal metrics")) // DisableHighCardinalityMetricsfeatureGate is the feature gate that controls whether the collector should enable -// potentially high cardinality metrics. +// 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.")) + "cardinality metrics. The gate will be removed when the collector allows for view configuration.")) // AllViews returns all the OpenCensus views requires by obsreport package. func AllViews(level configtelemetry.Level) []*view.View { From cf2ff3625bf8c2910b30786d8ae883cf5956e27a Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 26 Apr 2023 16:41:17 -0400 Subject: [PATCH 6/6] fix chloggen --- .chloggen/7517-behind-ff.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/7517-behind-ff.yaml b/.chloggen/7517-behind-ff.yaml index 237c1c0fcbf..d302806105c 100755 --- a/.chloggen/7517-behind-ff.yaml +++ b/.chloggen/7517-behind-ff.yaml @@ -2,7 +2,7 @@ change_type: enhancement # The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) -component: observavibility +component: service # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). note: Allows users to disable high cardinality OTLP attributes behind a feature flag.