Skip to content

Commit fb15de8

Browse files
bacherfljriguera
authored andcommitted
[cmd/opampsupervisor] Add config option for setting the timeout for the initial bootstrap information retrieval from the agent (open-telemetry#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:** open-telemetry#34996 **Testing:** Added unit tests **Documentation:** Added description for the new option in the specification readme --------- Signed-off-by: Florian Bacher <[email protected]>
1 parent 2a149b7 commit fb15de8

File tree

5 files changed

+69
-2
lines changed

5 files changed

+69
-2
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: opampsupervisor
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Add config option for setting the timeout for the initial bootstrap information retrieval from the agent
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [34996]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

cmd/opampsupervisor/specification/README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ agent:
126126
# The interval on which the Collector checks to see if it's been orphaned.
127127
orphan_detection_interval: 5s
128128

129+
# The maximum wait duration for retrieving bootstrapping information from the agent
130+
bootstrap_timeout: 3s
131+
129132
# Extra command line flags to pass to the Collector executable.
130133
args:
131134

cmd/opampsupervisor/supervisor/config/config.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ type Agent struct {
121121
Executable string
122122
OrphanDetectionInterval time.Duration `mapstructure:"orphan_detection_interval"`
123123
Description AgentDescription `mapstructure:"description"`
124+
BootstrapTimeout time.Duration `mapstructure:"bootstrap_timeout"`
124125
HealthCheckPort int `mapstructure:"health_check_port"`
125126
}
126127

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

133+
if a.BootstrapTimeout <= 0 {
134+
return errors.New("agent::bootstrap_timeout must be positive")
135+
}
136+
132137
if a.HealthCheckPort < 0 || a.HealthCheckPort > 65535 {
133138
return errors.New("agent::health_check_port must be a valid port number")
134139
}
@@ -180,6 +185,7 @@ func DefaultSupervisor() Supervisor {
180185
},
181186
Agent: Agent{
182187
OrphanDetectionInterval: 5 * time.Second,
188+
BootstrapTimeout: 3 * time.Second,
183189
},
184190
}
185191
}

cmd/opampsupervisor/supervisor/config/config_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ func TestValidate(t *testing.T) {
3636
Agent: Agent{
3737
Executable: "${file_path}",
3838
OrphanDetectionInterval: 5 * time.Second,
39+
BootstrapTimeout: 5 * time.Second,
3940
},
4041
Capabilities: Capabilities{
4142
AcceptsRemoteConfig: true,
@@ -163,6 +164,7 @@ func TestValidate(t *testing.T) {
163164
Agent: Agent{
164165
Executable: "",
165166
OrphanDetectionInterval: 5 * time.Second,
167+
BootstrapTimeout: 5 * time.Second,
166168
},
167169
Capabilities: Capabilities{
168170
AcceptsRemoteConfig: true,
@@ -188,6 +190,7 @@ func TestValidate(t *testing.T) {
188190
Agent: Agent{
189191
Executable: "./path/does/not/exist",
190192
OrphanDetectionInterval: 5 * time.Second,
193+
BootstrapTimeout: 5 * time.Second,
191194
},
192195
Capabilities: Capabilities{
193196
AcceptsRemoteConfig: true,
@@ -239,6 +242,7 @@ func TestValidate(t *testing.T) {
239242
Executable: "${file_path}",
240243
OrphanDetectionInterval: 5 * time.Second,
241244
HealthCheckPort: 65536,
245+
BootstrapTimeout: 5 * time.Second,
242246
},
243247
Capabilities: Capabilities{
244248
AcceptsRemoteConfig: true,
@@ -265,6 +269,7 @@ func TestValidate(t *testing.T) {
265269
Executable: "${file_path}",
266270
OrphanDetectionInterval: 5 * time.Second,
267271
HealthCheckPort: 0,
272+
BootstrapTimeout: 5 * time.Second,
268273
},
269274
Capabilities: Capabilities{
270275
AcceptsRemoteConfig: true,
@@ -290,6 +295,7 @@ func TestValidate(t *testing.T) {
290295
Executable: "${file_path}",
291296
OrphanDetectionInterval: 5 * time.Second,
292297
HealthCheckPort: 29848,
298+
BootstrapTimeout: 5 * time.Second,
293299
},
294300
Capabilities: Capabilities{
295301
AcceptsRemoteConfig: true,
@@ -299,6 +305,32 @@ func TestValidate(t *testing.T) {
299305
},
300306
},
301307
},
308+
{
309+
name: "config with invalid agent bootstrap timeout",
310+
config: Supervisor{
311+
Server: OpAMPServer{
312+
Endpoint: "wss://localhost:9090/opamp",
313+
Headers: http.Header{
314+
"Header1": []string{"HeaderValue"},
315+
},
316+
TLSSetting: configtls.ClientConfig{
317+
Insecure: true,
318+
},
319+
},
320+
Agent: Agent{
321+
Executable: "${file_path}",
322+
OrphanDetectionInterval: 5 * time.Second,
323+
BootstrapTimeout: -5 * time.Second,
324+
},
325+
Capabilities: Capabilities{
326+
AcceptsRemoteConfig: true,
327+
},
328+
Storage: Storage{
329+
Directory: "/etc/opamp-supervisor/storage",
330+
},
331+
},
332+
expectedError: "agent::bootstrap_timeout must be positive",
333+
},
302334
}
303335

304336
// create some fake files for validating agent config

cmd/opampsupervisor/supervisor/supervisor.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,7 @@ func (s *Supervisor) getBootstrapInfo() (err error) {
365365
}()
366366

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

0 commit comments

Comments
 (0)