Add config settings for component telemetry, move the flag (#2148)

Next steps will be to use the new settings in the processors.

Signed-off-by: Bogdan Drutu <[email protected]>

Author: bogdandrutu

config/configtelemetry/configtelemetry.go

@@ -15,38 +15,112 @@
 package configtelemetry
 
 import (
+	"flag"
 	"fmt"
 	"strings"
 )
 
 const (
 	// LevelNone indicates that no telemetry data should be collected.
 	LevelNone Level = iota - 1
-	// LevelBasic is the default and covers the basics of the service telemetry.
+	// LevelBasic is the recommended and covers the basics of the service telemetry.
 	LevelBasic
 	// LevelNormal adds some other indicators on top of basic.
 	LevelNormal
 	// LevelDetailed adds dimensions and views to the previous levels.
 	LevelDetailed
+
+	levelNoneStr     = "none"
+	levelBasicStr    = "basic"
+	levelNormalStr   = "normal"
+	levelDetailedStr = "detailed"
+
+	metricsLevelCfg = "metrics-level"
 )
 
+var metricsLevelPtr = new(Level)
+
+// Flags is a helper func, to add the telemetry config flags to the service that exposes
+// the application flags.
+func Flags(flags *flag.FlagSet) {
+	flags.Var(
+		metricsLevelPtr,
+		metricsLevelCfg,
+		"Output level of telemetry metrics (none, basic, normal, detailed)")
+}
+
 // Level is the level of internal telemetry (metrics, logs, traces about the component itself)
 // that every component should generate.
 type Level int8
 
+var _ flag.Value = (*Level)(nil)
+
+func (l *Level) String() string {
+	switch *l {
+	case LevelNone:
+		return levelNoneStr
+	case LevelBasic:
+		return levelBasicStr
+	case LevelNormal:
+		return levelNormalStr
+	case LevelDetailed:
+		return levelDetailedStr
+	}
+	return "unknown"
+}
+
+func (l *Level) Set(s string) error {
+	lvl, err := parseLevel(s)
+	if err != nil {
+		return err
+	}
+	*l = lvl
+	return nil
+}
+
+// GetMetricsLevelFlagValue returns the value of the "--metrics-level" flag.
+// IMPORTANT: This must be used only in the core collector code for the moment.
+func GetMetricsLevelFlagValue() Level {
+	return *metricsLevelPtr
+}
+
+// TelemetrySetting exposes the common Telemetry configuration for one component.
+type TelemetrySetting struct {
+	// MetricsLevelStr is the level of telemetry metrics, the possible values are:
+	//  - "none" indicates that no telemetry data should be collected;
+	//  - "basic" is the recommended and covers the basics of the service telemetry.
+	//  - "normal" adds some other indicators on top of basic.
+	//  - "detailed" adds dimensions and views to the previous levels.
+	MetricsLevelStr string `mapstructure:"metrics_level"`
+}
+
+// DefaultTelemetrySetting returns the default TelemetrySetting.
+// The level is set to the "--metrics-level" flag if set, otherwise the default "basic" level.
+func DefaultTelemetrySetting() TelemetrySetting {
+	return TelemetrySetting{
+		MetricsLevelStr: metricsLevelPtr.String(),
+	}
+}
+
+// GetMetricsLevel returns the parsed level, or error if unknown value.
+// Empty string is consider unknown value.
+func (ts TelemetrySetting) GetMetricsLevel() (Level, error) {
+	return parseLevel(ts.MetricsLevelStr)
+}
+
 // ParseLevel returns the Level represented by the string. The parsing is case-insensitive
 // and it returns error if the string value is unknown.
-func ParseLevel(str string) (Level, error) {
+func parseLevel(str string) (Level, error) {
 	str = strings.ToLower(str)
 
 	switch str {
-	case "none":
+	case levelNoneStr:
 		return LevelNone, nil
-	case "basic":
+	case levelBasicStr:
 		return LevelBasic, nil
-	case "normal":
+	case levelNormalStr:
 		return LevelNormal, nil
-	case "detailed":
+	case levelDetailedStr:
 		return LevelDetailed, nil
 	}
 	return LevelNone, fmt.Errorf("unknown metrics level %q", str)

config/configtelemetry/configtelemetry_test.go

@@ -18,9 +18,10 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func TestParseLevel(t *testing.T) {
+func TestParseFrom(t *testing.T) {
 	tests := []struct {
 		str   string
 		level Level
@@ -37,26 +38,26 @@ func TestParseLevel(t *testing.T) {
 			err:   true,
 		},
 		{
-			str:   "none",
+			str:   levelNoneStr,
 			level: LevelNone,
 		},
 		{
-			str:   "basic",
+			str:   levelBasicStr,
 			level: LevelBasic,
 		},
 		{
-			str:   "normal",
+			str:   levelNormalStr,
 			level: LevelNormal,
 		},
 		{
-			str:   "detailed",
+			str:   levelDetailedStr,
 			level: LevelDetailed,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.str, func(t *testing.T) {
-			lvl, err := ParseLevel(test.str)
+			lvl, err := parseLevel(test.str)
 			if test.err {
 				assert.Error(t, err)
 			} else {
@@ -66,3 +67,101 @@ func TestParseLevel(t *testing.T) {
 		})
 	}
 }
+
+func TestLevelSet(t *testing.T) {
+	tests := []struct {
+		str   string
+		level Level
+		err   bool
+	}{
+		{
+			str:   "",
+			level: LevelNone,
+			err:   true,
+		},
+		{
+			str:   "other_string",
+			level: LevelNone,
+			err:   true,
+		},
+		{
+			str:   levelNoneStr,
+			level: LevelNone,
+		},
+		{
+			str:   levelBasicStr,
+			level: LevelBasic,
+		},
+		{
+			str:   levelNormalStr,
+			level: LevelNormal,
+		},
+		{
+			str:   levelDetailedStr,
+			level: LevelDetailed,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.str, func(t *testing.T) {
+			lvl := new(Level)
+			err := lvl.Set(test.str)
+			if test.err {
+				assert.Error(t, err)
+				assert.Equal(t, LevelBasic, *lvl)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, test.level, *lvl)
+			}
+		})
+	}
+}
+
+func TestLevelString(t *testing.T) {
+	tests := []struct {
+		str   string
+		level Level
+		err   bool
+	}{
+		{
+			str:   "unknown",
+			level: Level(-10),
+		},
+		{
+			str:   levelNoneStr,
+			level: LevelNone,
+		},
+		{
+			str:   levelBasicStr,
+			level: LevelBasic,
+		},
+		{
+			str:   levelNormalStr,
+			level: LevelNormal,
+		},
+		{
+			str:   levelDetailedStr,
+			level: LevelDetailed,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.str, func(t *testing.T) {
+			assert.Equal(t, test.str, test.level.String())
+		})
+	}
+}
+
+func TestTelemetrySettings(t *testing.T) {
+	ts := &TelemetrySetting{
+		MetricsLevelStr: "unknown",
+	}
+	_, err := ts.GetMetricsLevel()
+	assert.Error(t, err)
+}
+
+func TestDefaultTelemetrySettings(t *testing.T) {
+	ts := DefaultTelemetrySetting()
+	assert.Equal(t, levelBasicStr, ts.MetricsLevelStr)
+	lvl, err := ts.GetMetricsLevel()
+	require.NoError(t, err)
+	assert.Equal(t, LevelBasic, lvl)
+}

internal/collector/telemetry/telemetry.go

@@ -18,31 +18,23 @@ package telemetry
 import (
 	"flag"
 
-	"go.opentelemetry.io/collector/config/configtelemetry"
 	"go.opentelemetry.io/collector/internal/version"
 )
 
 const (
 	metricsAddrCfg   = "metrics-addr"
-	metricsLevelCfg  = "metrics-level"
 	metricsPrefixCfg = "metrics-prefix"
 )
 
 var (
 	// Command-line flags that control publication of telemetry data.
-	metricsLevelPtr  *string
 	metricsAddrPtr   *string
 	metricsPrefixPtr *string
 
 	addInstanceIDPtr *bool
 )
 
 func Flags(flags *flag.FlagSet) {
-	metricsLevelPtr = flags.String(
-		metricsLevelCfg,
-		"BASIC",
-		"Output level of telemetry metrics (NONE, BASIC, NORMAL, DETAILED)")
-
 	// At least until we can use a generic, i.e.: OpenCensus, metrics exporter
 	// we default to Prometheus at port 8888, if not otherwise specified.
 	metricsAddrPtr = flags.String(
@@ -75,16 +67,6 @@ func GetAddInstanceID() bool {
 	return *addInstanceIDPtr
 }
 
-// GetLevel returns the Level represented by the string. The parsing is case-insensitive
-// and it returns error if the string value is unknown.
-func GetLevel() (configtelemetry.Level, error) {
-	if metricsLevelPtr != nil && *metricsLevelPtr != "" {
-		return configtelemetry.ParseLevel(*metricsLevelPtr)
-	}
-
-	return configtelemetry.LevelBasic, nil
-}
-
 func GetMetricsAddr() string {
 	return *metricsAddrPtr
 }

processor/batchprocessor/factory.go

@@ -20,8 +20,8 @@ import (
 
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/configmodels"
+	"go.opentelemetry.io/collector/config/configtelemetry"
 	"go.opentelemetry.io/collector/consumer"
-	"go.opentelemetry.io/collector/internal/collector/telemetry"
 	"go.opentelemetry.io/collector/processor/processorhelper"
 )
 
@@ -61,8 +61,7 @@ func createTraceProcessor(
 	nextConsumer consumer.TracesConsumer,
 ) (component.TracesProcessor, error) {
 	oCfg := cfg.(*Config)
-	// error can be ignored, level is parsed at the service startup
-	level, _ := telemetry.GetLevel()
+	level := configtelemetry.GetMetricsLevelFlagValue()
 	return newBatchTracesProcessor(params, nextConsumer, oCfg, level), nil
 }
 
@@ -73,7 +72,7 @@ func createMetricsProcessor(
 	nextConsumer consumer.MetricsConsumer,
 ) (component.MetricsProcessor, error) {
 	oCfg := cfg.(*Config)
-	level, _ := telemetry.GetLevel()
+	level := configtelemetry.GetMetricsLevelFlagValue()
 	return newBatchMetricsProcessor(params, nextConsumer, oCfg, level), nil
 }
 
@@ -84,6 +83,6 @@ func createLogsProcessor(
 	nextConsumer consumer.LogsConsumer,
 ) (component.LogsProcessor, error) {
 	oCfg := cfg.(*Config)
-	level, _ := telemetry.GetLevel()
+	level := configtelemetry.GetMetricsLevelFlagValue()
 	return newBatchLogsProcessor(params, nextConsumer, oCfg, level), nil
 }

processor/metrics.go

@@ -23,7 +23,6 @@ import (
 
 	"go.opentelemetry.io/collector/config/configtelemetry"
 	"go.opentelemetry.io/collector/consumer/pdata"
-	"go.opentelemetry.io/collector/internal/collector/telemetry"
 	"go.opentelemetry.io/collector/obsreport"
 	"go.opentelemetry.io/collector/translator/conventions"
 )
@@ -147,8 +146,8 @@ func RecordsSpanCountMetrics(ctx context.Context, scm *SpanCountStats, measure *
 }
 
 func serviceTagsEnabled() bool {
-	level, err := telemetry.GetLevel()
-	return err == nil && level == configtelemetry.LevelDetailed
+	level := configtelemetry.GetMetricsLevelFlagValue()
+	return level == configtelemetry.LevelDetailed
 }
 
 // spanCountByResourceStringAttribute calculates the number of spans by resource specified by

service/service.go

@@ -38,6 +38,7 @@ import (
 	"go.opentelemetry.io/collector/config"
 	"go.opentelemetry.io/collector/config/configcheck"
 	"go.opentelemetry.io/collector/config/configmodels"
+	"go.opentelemetry.io/collector/config/configtelemetry"
 	"go.opentelemetry.io/collector/internal/collector/telemetry"
 	"go.opentelemetry.io/collector/internal/version"
 	"go.opentelemetry.io/collector/service/builder"
@@ -162,6 +163,7 @@ func New(params Parameters) (*Application, error) {
 	// TODO: coalesce this code and expose this information to other components.
 	flagSet := new(flag.FlagSet)
 	addFlagsFns := []func(*flag.FlagSet){
+		configtelemetry.Flags,
 		telemetry.Flags,
 		builder.Flags,
 		loggerFlags,