-
Notifications
You must be signed in to change notification settings - Fork 262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move main.go config into controller package #198
Merged
Merged
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
1f03aaa
move main.go config into controller package
dmathieu a67b6c0
remove parseError method
dmathieu 51713d3
bring back parseError as an helper to ErrorWithExitCode
dmathieu a7e747a
pass string/args to parseError
dmathieu e81fc37
remove specific error code
dmathieu ea11937
remove now unused error
dmathieu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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 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,123 @@ | ||
package controller // import "go.opentelemetry.io/ebpf-profiler/internal/controller" | ||
|
||
import ( | ||
"errors" | ||
"flag" | ||
"fmt" | ||
"runtime" | ||
"time" | ||
|
||
log "github.com/sirupsen/logrus" | ||
"go.opentelemetry.io/ebpf-profiler/tracer" | ||
) | ||
|
||
type Config struct { | ||
BpfVerifierLogLevel uint | ||
CollAgentAddr string | ||
Copyright bool | ||
DisableTLS bool | ||
MapScaleFactor uint | ||
MonitorInterval time.Duration | ||
ClockSyncInterval time.Duration | ||
NoKernelVersionCheck bool | ||
PprofAddr string | ||
ProbabilisticInterval time.Duration | ||
ProbabilisticThreshold uint | ||
ReporterInterval time.Duration | ||
SamplesPerSecond int | ||
SendErrorFrames bool | ||
Tracers string | ||
VerboseMode bool | ||
Version bool | ||
|
||
Fs *flag.FlagSet | ||
} | ||
|
||
const ( | ||
exitParseError int = 2 | ||
|
||
// 1TB of executable address space | ||
MaxArgMapScaleFactor = 8 | ||
) | ||
|
||
// Dump visits all flag sets, and dumps them all to debug | ||
// Used for verbose mode logging. | ||
func (cfg *Config) Dump() { | ||
log.Debug("Config:") | ||
cfg.Fs.VisitAll(func(f *flag.Flag) { | ||
log.Debug(fmt.Sprintf("%s: %v", f.Name, f.Value)) | ||
}) | ||
} | ||
|
||
// Validate runs validations on the provided configuration, and returns errors | ||
// if invalid values were provided. | ||
func (cfg *Config) Validate() error { | ||
if cfg.SamplesPerSecond < 1 { | ||
return ErrorWithExitCode{ | ||
error: fmt.Errorf("invalid sampling frequency: %d", cfg.SamplesPerSecond), | ||
code: exitParseError, | ||
} | ||
dmathieu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
if cfg.MapScaleFactor > 8 { | ||
return ErrorWithExitCode{ | ||
error: fmt.Errorf("eBPF map scaling factor %d exceeds limit (max: %d)", | ||
cfg.MapScaleFactor, MaxArgMapScaleFactor), | ||
code: exitParseError, | ||
} | ||
} | ||
|
||
if cfg.BpfVerifierLogLevel > 2 { | ||
return ErrorWithExitCode{ | ||
error: fmt.Errorf("invalid eBPF verifier log level: %d", cfg.BpfVerifierLogLevel), | ||
code: exitParseError, | ||
} | ||
} | ||
|
||
if cfg.ProbabilisticInterval < 1*time.Minute || cfg.ProbabilisticInterval > 5*time.Minute { | ||
return ErrorWithExitCode{ | ||
error: errors.New("invalid argument for probabilistic-interval: use " + | ||
"a duration between 1 and 5 minutes"), | ||
code: exitParseError, | ||
} | ||
} | ||
|
||
if cfg.ProbabilisticThreshold < 1 || | ||
cfg.ProbabilisticThreshold > tracer.ProbabilisticThresholdMax { | ||
return ErrorWithExitCode{ | ||
error: fmt.Errorf("invalid argument for probabilistic-threshold. Value "+ | ||
"should be between 1 and %d", tracer.ProbabilisticThresholdMax), | ||
code: exitParseError, | ||
} | ||
} | ||
|
||
if !cfg.NoKernelVersionCheck { | ||
major, minor, patch, err := tracer.GetCurrentKernelVersion() | ||
if err != nil { | ||
return fmt.Errorf("failed to get kernel version: %v", err) | ||
dmathieu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
var minMajor, minMinor uint32 | ||
switch runtime.GOARCH { | ||
case "amd64": | ||
if cfg.VerboseMode { | ||
minMajor, minMinor = 5, 2 | ||
} else { | ||
minMajor, minMinor = 4, 19 | ||
} | ||
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) | ||
dmathieu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
if major < minMajor || (major == minMajor && minor < minMinor) { | ||
return fmt.Errorf("host Agent requires kernel version "+ | ||
dmathieu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"%d.%d or newer but got %d.%d.%d", minMajor, minMinor, major, minor, patch) | ||
} | ||
} | ||
|
||
return nil | ||
} |
This file contains 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,13 @@ | ||
package controller // import "go.opentelemetry.io/ebpf-profiler/internal/controller" | ||
dmathieu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// ErrorWithExitCode provides an error with an exit code | ||
// Used to be able to return errors with the exit code the CLI is expected to | ||
// return when exiting. | ||
type ErrorWithExitCode struct { | ||
error | ||
code int | ||
} | ||
|
||
func (e ErrorWithExitCode) Code() int { | ||
return e.code | ||
} |
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.
What about naming this internal package
args
instead ofcontroller
?From a controller I expect something different, than providing a general configuration and validate it.
As this package is likely reused in the OTel profiling controller part, it should not get mixed and suggest something it doesn't do.
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.
This is meant to be the first of a few PRs, with the intent to extract the setup of
main.go
into a shared package.I named this package
controller
, because what it will do in the end is run the agent, either standalone, or as a collector receiver.I'm open to a name different than
controller
, butargs
would be specific to this config, which doesn't do enough to be in its own package IMHO.