Skip to content

Commit

Permalink
[cmd/opampsupervisor] Add config option for setting the timeout for t…
Browse files Browse the repository at this point in the history
…he initial bootstrap information retrieval from the agent (#35000)

**Description:** This PR adds a configuration option for setting the
timeout for the initial bootstrap information retrieval from the agent.

**Link to tracking Issue:** #34996

**Testing:** Added unit tests

**Documentation:** Added description for the new option in the
specification readme

---------

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
  • Loading branch information
bacherfl authored Sep 27, 2024
1 parent 507ec47 commit 4b90b60
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 2 deletions.
27 changes: 27 additions & 0 deletions .chloggen/opampsupervisor_bootstrap_timeout_option.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: opampsupervisor

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add config option for setting the timeout for the initial bootstrap information retrieval from the agent

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [34996]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
3 changes: 3 additions & 0 deletions cmd/opampsupervisor/specification/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ agent:
# The interval on which the Collector checks to see if it's been orphaned.
orphan_detection_interval: 5s

# The maximum wait duration for retrieving bootstrapping information from the agent
bootstrap_timeout: 3s

# Extra command line flags to pass to the Collector executable.
args:

Expand Down
6 changes: 6 additions & 0 deletions cmd/opampsupervisor/supervisor/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ type Agent struct {
Executable string
OrphanDetectionInterval time.Duration `mapstructure:"orphan_detection_interval"`
Description AgentDescription `mapstructure:"description"`
BootstrapTimeout time.Duration `mapstructure:"bootstrap_timeout"`
HealthCheckPort int `mapstructure:"health_check_port"`
}

Expand All @@ -129,6 +130,10 @@ func (a Agent) Validate() error {
return errors.New("agent::orphan_detection_interval must be positive")
}

if a.BootstrapTimeout <= 0 {
return errors.New("agent::bootstrap_timeout must be positive")
}

if a.HealthCheckPort < 0 || a.HealthCheckPort > 65535 {
return errors.New("agent::health_check_port must be a valid port number")
}
Expand Down Expand Up @@ -180,6 +185,7 @@ func DefaultSupervisor() Supervisor {
},
Agent: Agent{
OrphanDetectionInterval: 5 * time.Second,
BootstrapTimeout: 3 * time.Second,
},
}
}
32 changes: 32 additions & 0 deletions cmd/opampsupervisor/supervisor/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func TestValidate(t *testing.T) {
Agent: Agent{
Executable: "${file_path}",
OrphanDetectionInterval: 5 * time.Second,
BootstrapTimeout: 5 * time.Second,
},
Capabilities: Capabilities{
AcceptsRemoteConfig: true,
Expand Down Expand Up @@ -163,6 +164,7 @@ func TestValidate(t *testing.T) {
Agent: Agent{
Executable: "",
OrphanDetectionInterval: 5 * time.Second,
BootstrapTimeout: 5 * time.Second,
},
Capabilities: Capabilities{
AcceptsRemoteConfig: true,
Expand All @@ -188,6 +190,7 @@ func TestValidate(t *testing.T) {
Agent: Agent{
Executable: "./path/does/not/exist",
OrphanDetectionInterval: 5 * time.Second,
BootstrapTimeout: 5 * time.Second,
},
Capabilities: Capabilities{
AcceptsRemoteConfig: true,
Expand Down Expand Up @@ -239,6 +242,7 @@ func TestValidate(t *testing.T) {
Executable: "${file_path}",
OrphanDetectionInterval: 5 * time.Second,
HealthCheckPort: 65536,
BootstrapTimeout: 5 * time.Second,
},
Capabilities: Capabilities{
AcceptsRemoteConfig: true,
Expand All @@ -265,6 +269,7 @@ func TestValidate(t *testing.T) {
Executable: "${file_path}",
OrphanDetectionInterval: 5 * time.Second,
HealthCheckPort: 0,
BootstrapTimeout: 5 * time.Second,
},
Capabilities: Capabilities{
AcceptsRemoteConfig: true,
Expand All @@ -290,6 +295,7 @@ func TestValidate(t *testing.T) {
Executable: "${file_path}",
OrphanDetectionInterval: 5 * time.Second,
HealthCheckPort: 29848,
BootstrapTimeout: 5 * time.Second,
},
Capabilities: Capabilities{
AcceptsRemoteConfig: true,
Expand All @@ -299,6 +305,32 @@ func TestValidate(t *testing.T) {
},
},
},
{
name: "config with invalid agent bootstrap timeout",
config: Supervisor{
Server: OpAMPServer{
Endpoint: "wss://localhost:9090/opamp",
Headers: http.Header{
"Header1": []string{"HeaderValue"},
},
TLSSetting: configtls.ClientConfig{
Insecure: true,
},
},
Agent: Agent{
Executable: "${file_path}",
OrphanDetectionInterval: 5 * time.Second,
BootstrapTimeout: -5 * time.Second,
},
Capabilities: Capabilities{
AcceptsRemoteConfig: true,
},
Storage: Storage{
Directory: "/etc/opamp-supervisor/storage",
},
},
expectedError: "agent::bootstrap_timeout must be positive",
},
}

// create some fake files for validating agent config
Expand Down
3 changes: 1 addition & 2 deletions cmd/opampsupervisor/supervisor/supervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,7 @@ func (s *Supervisor) getBootstrapInfo() (err error) {
}()

select {
// TODO make timeout configurable
case <-time.After(3 * time.Second):
case <-time.After(s.config.Agent.BootstrapTimeout):
if connected.Load() {
return errors.New("collector connected but never responded with an AgentDescription message")
} else {
Expand Down

0 comments on commit 4b90b60

Please sign in to comment.