Config validate #859
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,114 @@ | ||||||
| // Copyright The OpenTelemetry Authors | ||||||
| // SPDX-License-Identifier: Apache-2.0 | ||||||
|
|
||||||
| package config // import "go.opentelemetry.io/ebpf-profiler/collector/config" | ||||||
|
|
||||||
| import ( | ||||||
| "errors" | ||||||
| "fmt" | ||||||
| "runtime" | ||||||
| "time" | ||||||
|
|
||||||
| "go.opentelemetry.io/collector/confmap/xconfmap" | ||||||
| "go.opentelemetry.io/ebpf-profiler/tracer" | ||||||
| ) | ||||||
|
|
||||||
| const ( | ||||||
| // 1TB of executable address space | ||||||
| MaxArgMapScaleFactor = 8 | ||||||
| ) | ||||||
|
|
||||||
| // Config is the configuration for the collector. | ||||||
| type Config struct { | ||||||
| ReporterInterval time.Duration `mapstructure:"reporter_interval"` | ||||||
| MonitorInterval time.Duration `mapstructure:"monitor_interval"` | ||||||
| SamplesPerSecond int `mapstructure:"samples_per_second"` | ||||||
| ProbabilisticInterval time.Duration `mapstructure:"probabilistic_interval"` | ||||||
| ProbabilisticThreshold uint `mapstructure:"probabilistic_threshold"` | ||||||
| Tracers string `mapstructure:"tracers"` | ||||||
| ClockSyncInterval time.Duration `mapstructure:"clock_sync_interval"` | ||||||
| SendErrorFrames bool `mapstructure:"send_error_frames"` | ||||||
| VerboseMode bool `mapstructure:"verbose_mode"` | ||||||
| OffCPUThreshold float64 `mapstructure:"off_cpu_threshold"` | ||||||
| IncludeEnvVars string `mapstructure:"include_env_vars"` | ||||||
| UProbeLinks []string `mapstructure:"u_probe_links"` | ||||||
| LoadProbe bool `mapstructure:"load_probe"` | ||||||
| MapScaleFactor uint `mapstructure:"map_scale_factor"` | ||||||
| BPFVerifierLogLevel uint `mapstructure:"bpf_verifier_log_level"` | ||||||
| NoKernelVersionCheck bool `mapstructure:"no_kernel_version_check"` | ||||||
| MaxGRPCRetries uint32 `mapstructure:"max_grpc_retries"` | ||||||
| MaxRPCMsgSize int `mapstructure:"max_rpc_msg_size"` | ||||||
| } | ||||||
|
|
||||||
| var _ xconfmap.Validator = (*Config)(nil) | ||||||
|
|
||||||
| // Validate validates the config. | ||||||
| // This is automatically called by the config parser as it implements the xconfmap.Validator interface. | ||||||
| func (cfg *Config) Validate() error { | ||||||
| if cfg.SamplesPerSecond < 1 { | ||||||
| return fmt.Errorf("invalid sampling frequency: %d", cfg.SamplesPerSecond) | ||||||
| } | ||||||
|
|
||||||
| if cfg.MapScaleFactor > 8 { | ||||||
|
Member
There was a problem hiding this comment.
Suggested change
|
||||||
| return fmt.Errorf( | ||||||
| "eBPF map scaling factor %d exceeds limit (max: %d)", | ||||||
| cfg.MapScaleFactor, MaxArgMapScaleFactor, | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| if cfg.BPFVerifierLogLevel > 2 { | ||||||
| return fmt.Errorf("invalid eBPF verifier log level: %d", cfg.BPFVerifierLogLevel) | ||||||
| } | ||||||
|
|
||||||
| if cfg.ProbabilisticInterval < 1*time.Minute || cfg.ProbabilisticInterval > 5*time.Minute { | ||||||
| return errors.New( | ||||||
| "invalid argument for probabilistic-interval: use " + | ||||||
| "a duration between 1 and 5 minutes", | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| if cfg.ProbabilisticThreshold < 1 || | ||||||
| cfg.ProbabilisticThreshold > tracer.ProbabilisticThresholdMax { | ||||||
| return fmt.Errorf( | ||||||
| "invalid argument for probabilistic-threshold. Value "+ | ||||||
| "should be between 1 and %d", | ||||||
| tracer.ProbabilisticThresholdMax, | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| if cfg.OffCPUThreshold < 0.0 || cfg.OffCPUThreshold > 1.0 { | ||||||
| return errors.New( | ||||||
| "invalid argument for off-cpu-threshold. The value " + | ||||||
| "should be in the range [0..1]. 0 disables off-cpu profiling") | ||||||
| } | ||||||
|
|
||||||
| if !cfg.NoKernelVersionCheck { | ||||||
|
Member
There was a problem hiding this comment. The following part is mixing configuration validation with system validation. This should be avoided.
Contributor
Author
There was a problem hiding this comment. I have just moved the existing code.
Member
There was a problem hiding this comment. maybe it's time to separate it.
Member
There was a problem hiding this comment. System checks should maybe stay in |
||||||
| major, minor, patch, err := tracer.GetCurrentKernelVersion() | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to get kernel version: %v", err) | ||||||
| } | ||||||
|
|
||||||
| var minMajor, minMinor uint32 | ||||||
| switch runtime.GOARCH { | ||||||
| case "amd64": | ||||||
| if cfg.VerboseMode { | ||||||
| minMajor, minMinor = 5, 2 | ||||||
| } else { | ||||||
| minMajor, minMinor = 4, 19 | ||||||
|
Comment on lines
+94
to
+97
Member
There was a problem hiding this comment. This check is incorrect. 4.19 is no longer supported, as the project moved on and the instruction limit (4096) is no longer enforced on eBPF programs. As an example - unwind_native currently uses 7014 instructions on amd64.
Member
There was a problem hiding this comment. The minimal supported kernel version (for x86) should align with https://github.com/open-telemetry/opentelemetry-ebpf-profiler?tab=readme-ov-file#supported-linux-kernel-version and not make a difference based on |
||||||
| } | ||||||
| case "arm64": | ||||||
| // Older ARM64 kernel versions have broken bpf_probe_read. | ||||||
| // https://github.com/torvalds/linux/commit/6ae08ae3dea2cfa03dd3665a3c8475c2d429ef47 | ||||||
| minMajor, minMinor = 5, 5 | ||||||
| default: | ||||||
| return fmt.Errorf("unsupported architecture: %s", runtime.GOARCH) | ||||||
| } | ||||||
|
|
||||||
| if major < minMajor || (major == minMajor && minor < minMinor) { | ||||||
| return fmt.Errorf("host Agent requires kernel version "+ | ||||||
| "%d.%d or newer but got %d.%d.%d", minMajor, minMinor, major, minor, patch) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| return nil | ||||||
| } | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package config // import "go.opentelemetry.io/ebpf-profiler/collector/config" | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| "go.opentelemetry.io/collector/confmap/xconfmap" | ||
| ) | ||
|
|
||
| func TestValidate(t *testing.T) { | ||
| cfg := &Config{ | ||
| SamplesPerSecond: 0, | ||
| } | ||
| err := xconfmap.Validate(cfg) | ||
| require.Error(t, err) | ||
| require.Equal(t, "invalid sampling frequency: 0", err.Error()) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a check on MaxRPCMsgSize? As it is of type
int, it could also be negative.