Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@rafaelroquetto
rafaelroquetto committed Jul 23, 2024
1 parent 7a4bb45 commit 69dc012
Show file tree
Hide file tree
Showing 6 changed files with 343 additions and 283 deletions.
12 changes: 5 additions & 7 deletions pkg/beyla/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ var DefaultConfig = Config{
SpanMetricsServiceCacheSize: 10000,
},
Printer: false, // Deprecated: use TracePrinter instead
TracePrinter: debug.TracePrinterNone,
Noop: false,
TracePrinter: debug.TracePrinterDisabled,
InternalMetrics: imetrics.Config{
Prometheus: imetrics.PrometheusConfig{
Port: 0, // disabled by default
Expand Down Expand Up @@ -164,10 +163,9 @@ type Config struct {
// From this comment, the properties below will remain undocumented, as they
// are useful for development purposes. They might be helpful for customer support.

ChannelBufferLen int `yaml:"channel_buffer_len" env:"BEYLA_CHANNEL_BUFFER_LEN"`
Noop debug.NoopEnabled `yaml:"noop" env:"BEYLA_NOOP_TRACES"`
ProfilePort int `yaml:"profile_port" env:"BEYLA_PROFILE_PORT"`
InternalMetrics imetrics.Config `yaml:"internal_metrics"`
ChannelBufferLen int `yaml:"channel_buffer_len" env:"BEYLA_CHANNEL_BUFFER_LEN"`
ProfilePort int `yaml:"profile_port" env:"BEYLA_PROFILE_PORT"`
InternalMetrics imetrics.Config `yaml:"internal_metrics"`

// Processes metrics for application. They will be only enabled if there is a metrics exporter enabled,
// and both the "application" and "application_process" features are enabled
Expand Down Expand Up @@ -241,7 +239,7 @@ func (c *Config) Validate() error {
c.TracePrinter = debug.TracePrinterText
}

if c.Enabled(FeatureAppO11y) && !c.Noop.Enabled() && !c.Printer.Enabled() &&
if c.Enabled(FeatureAppO11y) && !c.Printer.Enabled() &&
!c.Grafana.OTLP.MetricsEnabled() && !c.Grafana.OTLP.TracesEnabled() &&
!c.Metrics.Enabled() && !c.Traces.Enabled() &&
!c.Prometheus.Enabled() && !c.TracePrinter.Enabled() {
Expand Down
71 changes: 64 additions & 7 deletions pkg/beyla/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/grafana/beyla/pkg/export/attributes"
"github.com/grafana/beyla/pkg/export/debug"
"github.com/grafana/beyla/pkg/export/instrumentations"
"github.com/grafana/beyla/pkg/export/otel"
"github.com/grafana/beyla/pkg/export/prom"
Expand All @@ -24,8 +25,11 @@ import (
"github.com/grafana/beyla/pkg/transform"
)

type envMap map[string]string

func TestConfig_Overrides(t *testing.T) {
userConfig := bytes.NewBufferString(`
trace_printer: json
channel_buffer_len: 33
ebpf:
functions:
Expand Down Expand Up @@ -66,7 +70,7 @@ network:
require.NoError(t, os.Setenv("GRAFANA_CLOUD_SUBMIT", "metrics,traces"))
require.NoError(t, os.Setenv("KUBECONFIG", "/foo/bar"))
require.NoError(t, os.Setenv("BEYLA_NAME_RESOLVER_SOURCES", "k8s,dns"))
defer unsetEnv(t, map[string]string{
defer unsetEnv(t, envMap{
"KUBECONFIG": "",
"BEYLA_OPEN_PORT": "", "BEYLA_EXECUTABLE_NAME": "", "OTEL_SERVICE_NAME": "", "BEYLA_NOOP_TRACES": "",
"OTEL_EXPORTER_OTLP_ENDPOINT": "", "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "", "GRAFANA_CLOUD_SUBMIT": "",
Expand Down Expand Up @@ -97,7 +101,7 @@ network:
ChannelBufferLen: 33,
LogLevel: "INFO",
Printer: false,
Noop: true,
TracePrinter: "json",
EBPF: ebpfcommon.TracerConfig{
BatchLength: 100,
BatchTimeout: time.Second,
Expand Down Expand Up @@ -198,12 +202,18 @@ func TestConfig_ServiceName(t *testing.T) {
}

func TestConfigValidate(t *testing.T) {
testCases := []map[string]string{
testCases := []envMap{
{"OTEL_EXPORTER_OTLP_ENDPOINT": "localhost:1234", "BEYLA_EXECUTABLE_NAME": "foo", "INSTRUMENT_FUNC_NAME": "bar"},
{"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "localhost:1234", "BEYLA_EXECUTABLE_NAME": "foo", "INSTRUMENT_FUNC_NAME": "bar"},
{"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "localhost:1234", "BEYLA_EXECUTABLE_NAME": "foo", "INSTRUMENT_FUNC_NAME": "bar"},
{"BEYLA_PRINT_TRACES": "true", "BEYLA_EXECUTABLE_NAME": "foo", "INSTRUMENT_FUNC_NAME": "bar"},
{"BEYLA_TRACE_PRINTER": "text", "BEYLA_EXECUTABLE_NAME": "foo", "INSTRUMENT_FUNC_NAME": "bar"},
{"BEYLA_PRINT_TRACES": "true", "BEYLA_TRACE_PRINTER": "disabled", "BEYLA_EXECUTABLE_NAME": "foo"},
{"BEYLA_PRINT_TRACES": "true", "BEYLA_TRACE_PRINTER": "", "BEYLA_EXECUTABLE_NAME": "foo"},
{"BEYLA_PRINT_TRACES": "false", "BEYLA_TRACE_PRINTER": "text", "BEYLA_EXECUTABLE_NAME": "foo"},
{"BEYLA_TRACE_PRINTER": "text", "BEYLA_EXECUTABLE_NAME": "foo"},
{"BEYLA_TRACE_PRINTER": "json", "BEYLA_EXECUTABLE_NAME": "foo"},
{"BEYLA_TRACE_PRINTER": "json_indent", "BEYLA_EXECUTABLE_NAME": "foo"},
{"BEYLA_TRACE_PRINTER": "counter", "BEYLA_EXECUTABLE_NAME": "foo"},
{"BEYLA_PROMETHEUS_PORT": "8080", "BEYLA_EXECUTABLE_NAME": "foo", "INSTRUMENT_FUNC_NAME": "bar"},
}
for n, tc := range testCases {
Expand All @@ -215,10 +225,13 @@ func TestConfigValidate(t *testing.T) {
}

func TestConfigValidate_error(t *testing.T) {
testCases := []map[string]string{
testCases := []envMap{
{"OTEL_EXPORTER_OTLP_ENDPOINT": "localhost:1234", "INSTRUMENT_FUNC_NAME": "bar"},
{"BEYLA_EXECUTABLE_NAME": "foo", "INSTRUMENT_FUNC_NAME": "bar", "BEYLA_PRINT_TRACES": "false"},
{"BEYLA_EXECUTABLE_NAME": "foo", "BEYLA_PRINT_TRACES": "true", "BEYLA_TRACE_PRINTER": "text"},
{"BEYLA_EXECUTABLE_NAME": "foo", "BEYLA_PRINT_TRACES": "true", "BEYLA_TRACE_PRINTER": "json"},
{"BEYLA_EXECUTABLE_NAME": "foo", "BEYLA_PRINT_TRACES": "true", "BEYLA_TRACE_PRINTER": "json_indent"},
{"BEYLA_EXECUTABLE_NAME": "foo", "BEYLA_PRINT_TRACES": "true", "BEYLA_TRACE_PRINTER": "counter"},
}
for n, tc := range testCases {
t.Run(fmt.Sprint("case", n), func(t *testing.T) {
Expand Down Expand Up @@ -283,6 +296,50 @@ network:
require.NoError(t, cfg.Validate())
}

func TestConfigValidate_TracePrinter(t *testing.T) {
type test struct {
env envMap
errorMsg string
}

testCases := []test{
{
env: envMap{"BEYLA_EXECUTABLE_NAME": "foo", "BEYLA_TRACE_PRINTER": "invalid_printer"},
errorMsg: "invalid value for trace_printer: 'invalid_printer'",
},
{
env: envMap{"BEYLA_EXECUTABLE_NAME": "foo", "BEYLA_TRACE_PRINTER": "json", "BEYLA_PRINT_TRACES": "true"},
errorMsg: "print_traces and trace_printer are mutually exclusive, use trace_printer instead",
},
{
env: envMap{"BEYLA_EXECUTABLE_NAME": "foo"},
errorMsg: "you need to define at least one exporter: trace_printer, grafana, otel_metrics_export, otel_traces_export or prometheus_export",
},
}

for i := range testCases {
cfg := loadConfig(t, testCases[i].env)
unsetEnv(t, testCases[i].env)

err := cfg.Validate()
require.Error(t, err)
assert.Equal(t, err.Error(), testCases[i].errorMsg)
}
}

func TestConfigValidate_TracePrinterFallback(t *testing.T) {
env := envMap{"BEYLA_EXECUTABLE_NAME": "foo", "BEYLA_PRINT_TRACES": "true"}

cfg := loadConfig(t, env)

unsetEnv(t, env)

err := cfg.Validate()
require.NoError(t, err)
assert.True(t, cfg.Printer.Enabled())
assert.Equal(t, cfg.TracePrinter, debug.TracePrinterText)
}

func TestConfig_OtelGoAutoEnv(t *testing.T) {
// OTEL_GO_AUTO_TARGET_EXE is an alias to BEYLA_EXECUTABLE_NAME
// (Compatibility with OpenTelemetry)
Expand Down Expand Up @@ -312,7 +369,7 @@ func TestConfig_NetworkImplicitProm(t *testing.T) {
assert.True(t, cfg.Enabled(FeatureNetO11y)) // Net o11y should be on
}

func loadConfig(t *testing.T, env map[string]string) *Config {
func loadConfig(t *testing.T, env envMap) *Config {
for k, v := range env {
require.NoError(t, os.Setenv(k, v))
}
Expand All @@ -321,7 +378,7 @@ func loadConfig(t *testing.T, env map[string]string) *Config {
return cfg
}

func unsetEnv(t *testing.T, env map[string]string) {
func unsetEnv(t *testing.T, env envMap) {
for k := range env {
require.NoError(t, os.Unsetenv(k))
}
Expand Down
Loading

0 comments on commit 69dc012

Please sign in to comment.