Few extra improvements in ExportModes (#403)

* Few extra improvements in ExportMetrics

* add test case for undefined vs nil value

Author: mariomac

pkg/components/discover/typer.go

@@ -97,7 +97,7 @@ func samplerFromConfig(s *services.SamplerConfig) trace.Sampler {
 func makeServiceAttrs(processMatch *ProcessMatch) svc.Attrs {
 	var name string
 	var namespace string
-	var exportModes *services.ExportModes
+	exportModes := services.ExportModeUnset
 	var samplerConfig *services.SamplerConfig
 
 	for _, s := range processMatch.Criteria {
@@ -109,7 +109,7 @@ func makeServiceAttrs(processMatch *ProcessMatch) svc.Attrs {
 			namespace = n
 		}
 
-		if m := s.GetExportModes(); m != nil {
+		if m := s.GetExportModes(); m != services.ExportModeUnset {
 			exportModes = m
 		}
 

pkg/components/svc/svc.go

@@ -100,7 +100,7 @@ type Attrs struct {
 
 	flags idFlags
 
-	ExportModes *services.ExportModes
+	ExportModes services.ExportModes
 
 	Sampler trace.Sampler
 }

pkg/services/attr_glob.go

@@ -77,7 +77,7 @@ type GlobAttributes struct {
 	// Configures what to export. Allowed values are 'metrics', 'traces',
 	// or an empty array (disabled). An unspecified value (nil) will use the
 	// default configuration value
-	ExportModes *ExportModes `yaml:"exports"`
+	ExportModes ExportModes `yaml:"exports"`
 
 	SamplerConfig *SamplerConfig `yaml:"sampler"`
 }
@@ -177,7 +177,7 @@ func (ga *GlobAttributes) RangePodAnnotations() iter.Seq2[string, StringMatcher]
 	}
 }
 
-func (ga *GlobAttributes) GetExportModes() *ExportModes { return ga.ExportModes }
+func (ga *GlobAttributes) GetExportModes() ExportModes { return ga.ExportModes }
 
 func (ga *GlobAttributes) GetSamplerConfig() *SamplerConfig { return ga.SamplerConfig }
 

pkg/services/attr_regex.go

@@ -83,7 +83,7 @@ type RegexSelector struct {
 	// Configures what to export. Allowed values are 'metrics', 'traces',
 	// or an empty array (disabled). An unspecified value (nil) will use the
 	// default configuration value
-	ExportModes *ExportModes `yaml:"exports"`
+	ExportModes ExportModes `yaml:"exports"`
 
 	SamplerConfig *SamplerConfig `yaml:"sampler"`
 }
@@ -181,6 +181,6 @@ func (a *RegexSelector) RangePodAnnotations() iter.Seq2[string, StringMatcher] {
 	}
 }
 
-func (a *RegexSelector) GetExportModes() *ExportModes { return a.ExportModes }
+func (a *RegexSelector) GetExportModes() ExportModes { return a.ExportModes }
 
 func (a *RegexSelector) GetSamplerConfig() *SamplerConfig { return a.SamplerConfig }

pkg/services/criteria.go

@@ -131,7 +131,7 @@ type Selector interface {
 	RangeMetadata() iter.Seq2[string, StringMatcher]
 	RangePodLabels() iter.Seq2[string, StringMatcher]
 	RangePodAnnotations() iter.Seq2[string, StringMatcher]
-	GetExportModes() *ExportModes
+	GetExportModes() ExportModes
 	GetSamplerConfig() *SamplerConfig
 }
 

pkg/services/export.go

@@ -12,39 +12,61 @@ import (
 )
 
 const (
-	exportMetrics = maps.Bits(1 << iota)
-	exportTraces
+	// if the corresponding bit is set to 1, this means that this signal
+	// is not going to be emitted
+	blockMetrics = maps.Bits(1 << iota)
+	blockTraces
 )
 
 var modeForText = map[string]maps.Bits{
-	"metrics": exportMetrics,
-	"traces":  exportTraces,
+	"metrics": blockMetrics,
+	"traces":  blockTraces,
 }
 
-type ExportModes struct {
-	items maps.Bits
-}
+const (
+	// the zero-value of ExportModes (blockSignal == 0) means that the value is unset.
+	// This is, all the signals are allowed.
+	// In the corresponding YAML is a null or undefined value
+	unset = maps.Bits(0)
+	// the -1 (all bits to one) value of ExportModes means that the value is set to
+	// an empty set: all the signals are blocked.
+	// In the corresponding YAML it's an empty sequence []
+	blockAll = maps.Bits(^uint(0))
+)
 
-func (modes *ExportModes) canExport(mode maps.Bits) bool {
-	if modes == nil {
-		return true
-	}
-	return modes.items.Has(mode)
+// ExportModeUnset corresponds to an undefined export mode in the configuration YAML
+// (null or undefined value). This means that all the signals (traces, metrics) are
+// going to be exported
+var ExportModeUnset = ExportModes{blockSignal: unset}
+
+// ExportModes specifies which signals are going to be exported for a given service,
+// via the public methods CanExportTraces and CanExportMetrics.
+// Internally, it has three modes of operation depending on how it is defined in the YAML:
+//   - When it is undefined or null in the YAML, it will allow exporting all the signals
+//     (as no blocking signals are defined)
+//   - When it is defined as an empty list in the YAML, it will block all the signals. No
+//     metrics nor traces are exported.
+//   - When it is defined as a non-empty list, it will only allow the explicitly specified signals.
+type ExportModes struct {
+	blockSignal maps.Bits
 }
 
 // CanExportTraces reports whether traces can be exported.
 // It's provided as a convenience function.
-func (modes *ExportModes) CanExportTraces() bool {
-	return modes.canExport(exportTraces)
+func (modes ExportModes) CanExportTraces() bool {
+	return !modes.blockSignal.Has(blockTraces)
 }
 
 // CanExportMetrics reports whether metrics can be exported.
 // It's provided as a convenience function.
-func (modes *ExportModes) CanExportMetrics() bool {
-	return modes.canExport(exportMetrics)
+func (modes ExportModes) CanExportMetrics() bool {
+	return !modes.blockSignal.Has(blockMetrics)
 }
 
 func (modes *ExportModes) UnmarshalYAML(value *yaml.Node) error {
+	// by default, everything is blocked, and we will unblock each signal
+	// as long as we parse them in the YAML
+	modes.blockSignal = blockAll
 	if value.Kind != yaml.SequenceNode {
 		return fmt.Errorf("ExportModes: unexpected YAML node kind %d", value.Kind)
 	}
@@ -55,33 +77,27 @@ func (modes *ExportModes) UnmarshalYAML(value *yaml.Node) error {
 		if mode, ok := modeForText[inner.Value]; !ok {
 			return fmt.Errorf("ExportModes[%d]: unknown export mode %q", i, inner.Value)
 		} else {
-			modes.items |= mode
+			// a given signal is defined. Remove it from the blocking list
+			modes.blockSignal ^= mode
 		}
 	}
-	return modes.UnmarshalText([]byte(value.Value))
-}
-
-func (modes *ExportModes) UnmarshalText(text []byte) error {
-	var options []string
-	if err := yaml.Unmarshal(text, &options); err != nil {
-		return fmt.Errorf("invalid export_modes: %w", err)
-	}
-	if options == nil {
-		return nil
-	}
-	modes.items = maps.MappedBits(options, modeForText)
 	return nil
 }
 
-func (modes *ExportModes) MarshalYAML() (any, error) {
-	if modes == nil {
+func (modes ExportModes) MarshalYAML() (any, error) {
+	if modes.blockSignal == unset {
 		return nil, nil
 	}
 	node := yaml.Node{
 		Kind: yaml.SequenceNode,
 	}
+	// return an empty sequence, in opposition of "null" in the unset case
+	if modes.blockSignal == blockAll {
+		return node, nil
+	}
 	for text, mode := range modeForText {
-		if modes.items.Has(mode) {
+		// the given signal is not explicitly blocked, so we can list it as allowed
+		if !modes.blockSignal.Has(mode) {
 			node.Content = append(node.Content, &yaml.Node{
 				Kind:  yaml.ScalarNode,
 				Value: text,
@@ -90,16 +106,3 @@ func (modes *ExportModes) MarshalYAML() (any, error) {
 	}
 	return node, nil
 }
-
-func (modes *ExportModes) MarshalText() ([]byte, error) {
-	var options []string
-	if modes != nil {
-		options = make([]string, 0, len(modeForText))
-		for text, mode := range modeForText {
-			if modes.items.Has(mode) {
-				options = append(options, text)
-			}
-		}
-	}
-	return yaml.Marshal(options)
-}

pkg/services/export_test.go

@@ -13,32 +13,32 @@ import (
 
 func TestYAMLMarshal_Exports(t *testing.T) {
 	type tc struct {
-		Exports *ExportModes
+		Exports ExportModes
 	}
 	t.Run("nil value", func(t *testing.T) {
 		yamlOut, err := yaml.Marshal(&tc{
-			Exports: nil,
+			Exports: ExportModes{},
 		})
 		require.NoError(t, err)
 		assert.YAMLEq(t, `exports: null`, string(yamlOut))
 	})
 	t.Run("empty value", func(t *testing.T) {
 		yamlOut, err := yaml.Marshal(&tc{
-			Exports: &ExportModes{},
+			Exports: ExportModes{blockSignal: blockAll},
 		})
 		require.NoError(t, err)
 		assert.YAMLEq(t, `exports: []`, string(yamlOut))
 	})
 	t.Run("some value", func(t *testing.T) {
 		yamlOut, err := yaml.Marshal(&tc{
-			Exports: &ExportModes{items: exportMetrics},
+			Exports: ExportModes{blockSignal: ^blockMetrics},
 		})
 		require.NoError(t, err)
 		assert.YAMLEq(t, `exports: ["metrics"]`, string(yamlOut))
 	})
 	t.Run("all values", func(t *testing.T) {
 		yamlOut, err := yaml.Marshal(&tc{
-			Exports: &ExportModes{items: exportMetrics | exportTraces},
+			Exports: ExportModes{blockSignal: ^(blockMetrics | blockTraces)},
 		})
 		require.NoError(t, err)
 		assert.YAMLEq(t, `exports: ["metrics", "traces"]`, string(yamlOut))
@@ -47,13 +47,21 @@ func TestYAMLMarshal_Exports(t *testing.T) {
 
 func TestYAMLUnmarshal_Exports(t *testing.T) {
 	type tc struct {
-		Exports *ExportModes
+		Exports ExportModes
 	}
+	t.Run("undefined value", func(t *testing.T) {
+		var tc tc
+		err := yaml.Unmarshal([]byte(``), &tc)
+		require.NoError(t, err)
+		assert.True(t, tc.Exports.CanExportMetrics())
+		assert.True(t, tc.Exports.CanExportTraces())
+	})
 	t.Run("nil value", func(t *testing.T) {
 		var tc tc
 		err := yaml.Unmarshal([]byte(`exports: null`), &tc)
 		require.NoError(t, err)
-		assert.Nil(t, tc.Exports)
+		assert.True(t, tc.Exports.CanExportMetrics())
+		assert.True(t, tc.Exports.CanExportTraces())
 	})
 	t.Run("empty value", func(t *testing.T) {
 		var tc tc