Skip to content

Commit

Permalink
Put otel meter for http/grpc behind a FF (open-telemetry#7543)
Browse files Browse the repository at this point in the history
**Description:** Puts the grpc meter provider behind a feature flag for controlling high cardinality metrics.

**Link to tracking Issue:** open-telemetry#7517
  • Loading branch information
jaronoff97 authored and Alex Boten committed May 2, 2023
1 parent 00f3045 commit 9987a3a
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 9 deletions.
16 changes: 16 additions & 0 deletions .chloggen/7517-behind-ff.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
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.

# 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:
8 changes: 8 additions & 0 deletions internal/obsreportconfig/obsreportconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ var UseOtelForInternalMetricsfeatureGate = featuregate.GlobalRegistry().MustRegi
featuregate.StageAlpha,
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. 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."))

// AllViews returns all the OpenCensus views requires by obsreport package.
func AllViews(level configtelemetry.Level) []*view.View {
if level == configtelemetry.LevelNone {
Expand Down
3 changes: 2 additions & 1 deletion service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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)
Expand Down
58 changes: 52 additions & 6 deletions service/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,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"
Expand All @@ -46,7 +47,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"
)

Expand All @@ -57,10 +58,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 {
Expand All @@ -69,13 +89,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,
}
}

Expand Down Expand Up @@ -225,10 +247,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
Expand All @@ -247,6 +286,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 == '_' {
Expand Down
61 changes: 59 additions & 2 deletions service/telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,16 @@ 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"
)

const (
metricPrefix = "otelcol_"
otelPrefix = "otel_sdk_"
ocPrefix = "oc_sdk_"
grpcPrefix = "gprc_"
httpPrefix = "http_"
counterName = "test_counter"
)

Expand Down Expand Up @@ -97,6 +99,7 @@ func TestTelemetryInit(t *testing.T) {
for _, tc := range []struct {
name string
useOtel bool
disableHighCard bool
expectedMetrics map[string]metricValue
}{
{
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit 9987a3a

Please sign in to comment.