Skip to content

feat: rename config flag to observe-config, use default otel command for our start command #146

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 8, 2025
Merged

feat: rename config flag to observe-config, use default otel command for our start command #146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions internal/commands/diagnose/otelconfigcheck.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/observeinc/observe-agent/internal/commands/start"
logger "github.com/observeinc/observe-agent/internal/commands/util"
"github.com/observeinc/observe-agent/observecol"
"github.com/spf13/viper"
"go.opentelemetry.io/collector/otelcol"
)
Expand All @@ -16,13 +17,14 @@ type OtelConfigTestResult struct {
}

func checkOtelConfig(_ *viper.Viper) (bool, any, error) {
colSettings, cleanup, err := start.SetupAndGenerateCollectorSettings(logger.WithCtx(context.Background(), logger.GetNop()))
if err != nil {
return false, nil, err
}
configFilePaths, cleanup, err := start.SetupAndGetConfigFiles(logger.WithCtx(context.Background(), logger.GetNop()))
if cleanup != nil {
defer cleanup()
}
if err != nil {
return false, nil, err
}
colSettings := observecol.GenerateCollectorSettingsWithConfigFiles(configFilePaths)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnose won't run against otel config overrides, but we never supported this:

$ observe-agent diagnose --otel-config ./tmp.config
Error: unknown flag: --otel-config
Usage:
  observe-agent diagnose [flags]

Flags:
  -h, --help   help for diagnose

Global Flags:
      --config string   config file path

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make a ticket to support the new --observe-config flag in diagnose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created https://observe.atlassian.net/browse/OB-40550

// These are the same checks as the `otelcol validate` command:
// https://github.com/open-telemetry/opentelemetry-collector/blob/main/otelcol/command_validate.go
col, err := otelcol.NewCollector(*colSettings)
Expand Down
60 changes: 25 additions & 35 deletions internal/commands/start/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import (
"github.com/observeinc/observe-agent/internal/root"
"github.com/observeinc/observe-agent/observecol"
"github.com/spf13/cobra"
"github.com/spf13/viper"
collector "go.opentelemetry.io/collector/otelcol"
)

func SetupAndGetConfigFiles(ctx context.Context) ([]string, func(), error) {
Expand Down Expand Up @@ -42,46 +40,38 @@ func DefaultLoggerCtx() context.Context {
return logger.WithCtx(context.Background(), logger.Get())
}

func SetupAndGenerateCollectorSettings(ctx context.Context) (*collector.CollectorSettings, func(), error) {
configFilePaths, cleanup, err := SetupAndGetConfigFiles(ctx)
if err != nil {
return nil, cleanup, err
}
// Generate collector settings with all config files
colSettings := observecol.GenerateCollectorSettings(configFilePaths)
return colSettings, cleanup, nil
}
func MakeStartCommand() *cobra.Command {
// Create the start command from the otel collector command
settings := observecol.GenerateCollectorSettings()
otleCmd := observecol.GetOtelCollectorCommand(settings)
otleCmd.Use = "start"
otleCmd.Short = "Start the Observe agent process."
otleCmd.Long = `The Observe agent is based on the OpenTelemetry Collector.
This command reads in the local config and env vars and starts the
collector on the current host.`
// Drop the sub commands
otleCmd.ResetCommands()

var startCmd = &cobra.Command{
Use: "start",
Short: "Start the Observe agent process.",
Long: `The Observe agent is based on the OpenTelemetry Collector.
This command reads in the local config and env vars and starts the
collector on the current host.`,
RunE: func(cmd *cobra.Command, args []string) error {
colSettings, cleanup, err := SetupAndGenerateCollectorSettings(DefaultLoggerCtx())
// Modify the run function so we can pass in our packaged config files.
originalRunE := otleCmd.RunE
otleCmd.RunE = func(cmd *cobra.Command, args []string) error {
configFilePaths, cleanup, err := SetupAndGetConfigFiles(DefaultLoggerCtx())
if cleanup != nil {
defer cleanup()
}
if err != nil {
return err
}
if cleanup != nil {
defer cleanup()
configFlag := otleCmd.Flags().Lookup("config")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flags for the otel collector are hidden behind private implementations of interfaces, args passed by value, and function scope locals. It's really not possible to access and set them directly if you aren't passing args to the cmd flags. Because of this, I think using the otelcol command itself for our start subcommand is the best way to go.

for _, path := range configFilePaths {
configFlag.Value.Set(path)
}
otelCmd := observecol.GetOtelCollectorCommand(colSettings)
return otelCmd.RunE(cmd, args)
},
return originalRunE(cmd, args)
}
return otleCmd
}

func init() {
startCmd.PersistentFlags().String("otel-config", "", "Path to additional otel configuration file")
viper.BindPFlag("otelConfigFile", startCmd.PersistentFlags().Lookup("otel-config"))
startCmd := MakeStartCommand()
root.RootCmd.AddCommand(startCmd)

// Here you will define your flags and configuration settings.

// Cobra supports Persistent Flags which will work for this command
// and all subcommands, e.g.:

// Cobra supports local flags which will only run when this command
// is called directly, e.g.:
// startCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle")
}
4 changes: 0 additions & 4 deletions internal/connections/confighandler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ func GetAllOtelConfigFilePaths(ctx context.Context, tmpDir string) ([]string, er
configFilePaths = append(configFilePaths, connectionPaths...)
}
}
// Read in otel-config flag and add to paths if set
if viper.IsSet("otelConfigFile") {
configFilePaths = append(configFilePaths, viper.GetString("otelConfigFile"))
}
// Generate override file and include path if overrides provided
if viper.IsSet(OTEL_OVERRIDE_YAML_KEY) {
// GetStringMap is more lenient with respect to conversions than Sub, which only handles maps.
Expand Down
2 changes: 1 addition & 1 deletion internal/root/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func Execute() {
func init() {
cobra.OnInitialize(InitConfig)

RootCmd.PersistentFlags().StringVar(&CfgFile, "config", "", "config file path")
RootCmd.PersistentFlags().StringVar(&CfgFile, "observe-config", "", "observe-agent config file path")
}

// InitConfig reads in config file and ENV variables if set.
Expand Down
53 changes: 28 additions & 25 deletions main_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,41 +10,44 @@ import (

"github.com/observeinc/observe-agent/internal/commands/start"
"github.com/observeinc/observe-agent/internal/root"
"github.com/observeinc/observe-agent/observecol"
"go.opentelemetry.io/collector/otelcol"
"golang.org/x/sys/windows"
"golang.org/x/sys/windows/svc"
)

func run() error {
inService, err := svc.IsWindowsService()
if err != nil {
// If we're not running as a service, we run normally.
if inService, err := svc.IsWindowsService(); err != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reorganized this method so it now looks more like the default OCB method. The diff for this condition is just code motion.

log.Fatalf("failed to determine if we are running in service: %v", err)
} else if !inService {
return runInteractive()
}

if inService {
if len(os.Args) != 2 {
log.Fatal("Expected to run svc as: observe-agent.exe <path to observe-agent.yaml>")
}
root.CfgFile = os.Args[1]
root.InitConfig()
colSettings, cleanup, err := start.SetupAndGenerateCollectorSettings(start.DefaultLoggerCtx())
if err != nil {
return err
}
if cleanup != nil {
defer cleanup()
}
if err := svc.Run("", otelcol.NewSvcHandler(*colSettings)); err != nil {
if errors.Is(err, windows.ERROR_FAILED_SERVICE_CONTROLLER_CONNECT) {
// Per https://learn.microsoft.com/en-us/windows/win32/api/winsvc/nf-winsvc-startservicectrldispatchera#return-value
// this means that the process is not running as a service, so run interactively.
return runInteractive()
}

return fmt.Errorf("failed to start collector server: %w", err)
if len(os.Args) != 2 {
log.Fatal("Expected to run svc as: observe-agent.exe <path to observe-agent.yaml>")
}
root.CfgFile = os.Args[1]
root.InitConfig()

// Get the collector settings along with our bundled config files.
configFilePaths, cleanup, err := start.SetupAndGetConfigFiles(start.DefaultLoggerCtx())
if cleanup != nil {
defer cleanup()
}
if err != nil {
return err
}
colSettings := observecol.GenerateCollectorSettingsWithConfigFiles(configFilePaths)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: Generating the settings this way instead of using the otel command means it won't work with --config. That's fine though, since we have a hard assert that the windows service is only ever run with two args and we have never supported --config this way.


if err := svc.Run("", otelcol.NewSvcHandler(*colSettings)); err != nil {
if errors.Is(err, windows.ERROR_FAILED_SERVICE_CONTROLLER_CONNECT) {
// Per https://learn.microsoft.com/en-us/windows/win32/api/winsvc/nf-winsvc-startservicectrldispatchera#return-value
// this means that the process is not running as a service, so run interactively.
return runInteractive()
}
} else {
return runInteractive()

return fmt.Errorf("failed to start collector server: %w", err)
}

return nil
Expand Down
17 changes: 7 additions & 10 deletions observecol/otelcollector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,7 @@ import (
"go.opentelemetry.io/collector/otelcol"
)

func makeMapProvidersMap(providers ...confmap.Provider) map[string]confmap.Provider {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was unused.

ret := make(map[string]confmap.Provider, len(providers))
for _, provider := range providers {
ret[provider.Scheme()] = provider
}
return ret
}

func GenerateCollectorSettings(URIs []string) *otelcol.CollectorSettings {
func GenerateCollectorSettings() *otelcol.CollectorSettings {
buildInfo := component.BuildInfo{
Command: "observe-agent",
Description: "Observe Distribution of Opentelemetry Collector",
Expand All @@ -33,7 +25,6 @@ func GenerateCollectorSettings(URIs []string) *otelcol.CollectorSettings {
Factories: components,
ConfigProviderSettings: otelcol.ConfigProviderSettings{
ResolverSettings: confmap.ResolverSettings{
URIs: URIs,
ProviderFactories: []confmap.ProviderFactory{
fileprovider.NewFactory(),
envprovider.NewFactory(),
Expand All @@ -47,6 +38,12 @@ func GenerateCollectorSettings(URIs []string) *otelcol.CollectorSettings {
return set
}

func GenerateCollectorSettingsWithConfigFiles(configFiles []string) *otelcol.CollectorSettings {
set := GenerateCollectorSettings()
set.ConfigProviderSettings.ResolverSettings.URIs = configFiles
return set
}

func GetOtelCollectorCommand(otelconfig *otelcol.CollectorSettings) *cobra.Command {
cmd := otelcol.NewCommand(*otelconfig)
return cmd
Expand Down
2 changes: 1 addition & 1 deletion packaging/linux/config/observe-agent.service
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Description=Observe Agent
After=network.target

[Service]
ExecStart=/usr/bin/observe-agent start --config /etc/observe-agent/observe-agent.yaml
ExecStart=/usr/bin/observe-agent start --observe-config /etc/observe-agent/observe-agent.yaml
KillMode=mixed
Restart=on-failure
Type=simple
Expand Down
Loading