Skip to content

Commit 659ba41

Browse files
dpaasman00f7o
authored andcommitted
[opampsupervisor] Add HealthCheckPort configuration parameter (open-telemetry#34704)
**Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Add a new configuration parameter to `agent` called `health_check_port`. If this is set, then the supervisor will configure the agent's healthcheck extension to use the given port. If it is unset, then we will grab a random port same as before. **Link to tracking Issue:** open-telemetry#34643 **Testing:** <Describe what testing was performed and which tests were added.> - Updated config validation tests - Verified that healthcheck extension is configured with the correct port and works as expected
1 parent 7247c42 commit 659ba41

File tree

6 files changed

+138
-7
lines changed

6 files changed

+138
-7
lines changed
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 new config parameter `agent.health_check_port` to allow configuring the port used by the agent healthcheck extension."
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: [34643]
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/e2e_test.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -337,18 +337,19 @@ func TestSupervisorStartsWithNoOpAMPServer(t *testing.T) {
337337

338338
// The supervisor is started without a running OpAMP server.
339339
// The supervisor should start successfully, even if the OpAMP server is stopped.
340-
s := newSupervisor(t, "basic", map[string]string{
341-
"url": server.addr,
340+
s := newSupervisor(t, "healthcheck_port", map[string]string{
341+
"url": server.addr,
342+
"healthcheck_port": "12345",
342343
})
343344

344345
require.Nil(t, s.Start())
345346
defer s.Shutdown()
346347

347348
// Verify the collector is running by checking the metrics endpoint
348349
require.Eventually(t, func() bool {
349-
resp, err := http.DefaultClient.Get("http://localhost:8888/metrics")
350+
resp, err := http.DefaultClient.Get("http://localhost:12345")
350351
if err != nil {
351-
t.Logf("Failed check for prometheus metrics: %s", err)
352+
t.Logf("Failed agent healthcheck request: %s", err)
352353
return false
353354
}
354355
require.NoError(t, resp.Body.Close())

cmd/opampsupervisor/supervisor/config/config.go

+5
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,18 @@ type Agent struct {
121121
Executable string
122122
OrphanDetectionInterval time.Duration `mapstructure:"orphan_detection_interval"`
123123
Description AgentDescription `mapstructure:"description"`
124+
HealthCheckPort int `mapstructure:"health_check_port"`
124125
}
125126

126127
func (a Agent) Validate() error {
127128
if a.OrphanDetectionInterval <= 0 {
128129
return errors.New("agent::orphan_detection_interval must be positive")
129130
}
130131

132+
if a.HealthCheckPort < 0 || a.HealthCheckPort > 65535 {
133+
return errors.New("agent::health_check_port must be a valid port number")
134+
}
135+
131136
if a.Executable == "" {
132137
return errors.New("agent::executable must be specified")
133138
}

cmd/opampsupervisor/supervisor/config/config_test.go

+76
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,82 @@ func TestValidate(t *testing.T) {
223223
},
224224
expectedError: "agent::orphan_detection_interval must be positive",
225225
},
226+
{
227+
name: "Invalid port number",
228+
config: Supervisor{
229+
Server: OpAMPServer{
230+
Endpoint: "wss://localhost:9090/opamp",
231+
Headers: http.Header{
232+
"Header1": []string{"HeaderValue"},
233+
},
234+
TLSSetting: configtls.ClientConfig{
235+
Insecure: true,
236+
},
237+
},
238+
Agent: Agent{
239+
Executable: "${file_path}",
240+
OrphanDetectionInterval: 5 * time.Second,
241+
HealthCheckPort: 65536,
242+
},
243+
Capabilities: Capabilities{
244+
AcceptsRemoteConfig: true,
245+
},
246+
Storage: Storage{
247+
Directory: "/etc/opamp-supervisor/storage",
248+
},
249+
},
250+
expectedError: "agent::health_check_port must be a valid port number",
251+
},
252+
{
253+
name: "Zero value port number",
254+
config: Supervisor{
255+
Server: OpAMPServer{
256+
Endpoint: "wss://localhost:9090/opamp",
257+
Headers: http.Header{
258+
"Header1": []string{"HeaderValue"},
259+
},
260+
TLSSetting: configtls.ClientConfig{
261+
Insecure: true,
262+
},
263+
},
264+
Agent: Agent{
265+
Executable: "${file_path}",
266+
OrphanDetectionInterval: 5 * time.Second,
267+
HealthCheckPort: 0,
268+
},
269+
Capabilities: Capabilities{
270+
AcceptsRemoteConfig: true,
271+
},
272+
Storage: Storage{
273+
Directory: "/etc/opamp-supervisor/storage",
274+
},
275+
},
276+
},
277+
{
278+
name: "Normal port number",
279+
config: Supervisor{
280+
Server: OpAMPServer{
281+
Endpoint: "wss://localhost:9090/opamp",
282+
Headers: http.Header{
283+
"Header1": []string{"HeaderValue"},
284+
},
285+
TLSSetting: configtls.ClientConfig{
286+
Insecure: true,
287+
},
288+
},
289+
Agent: Agent{
290+
Executable: "${file_path}",
291+
OrphanDetectionInterval: 5 * time.Second,
292+
HealthCheckPort: 29848,
293+
},
294+
Capabilities: Capabilities{
295+
AcceptsRemoteConfig: true,
296+
},
297+
Storage: Storage{
298+
Directory: "/etc/opamp-supervisor/storage",
299+
},
300+
},
301+
},
226302
}
227303

228304
// create some fake files for validating agent config

cmd/opampsupervisor/supervisor/supervisor.go

+6-3
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,13 @@ func (s *Supervisor) Start() error {
179179
return fmt.Errorf("could not get bootstrap info from the Collector: %w", err)
180180
}
181181

182-
healthCheckPort, err := s.findRandomPort()
182+
healthCheckPort := s.config.Agent.HealthCheckPort
183+
if healthCheckPort == 0 {
184+
healthCheckPort, err = s.findRandomPort()
183185

184-
if err != nil {
185-
return fmt.Errorf("could not find port for health check: %w", err)
186+
if err != nil {
187+
return fmt.Errorf("could not find port for health check: %w", err)
188+
}
186189
}
187190

188191
s.agentHealthCheckEndpoint = fmt.Sprintf("localhost:%d", healthCheckPort)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
server:
2+
endpoint: ws://{{.url}}/v1/opamp
3+
tls:
4+
insecure: true
5+
6+
capabilities:
7+
reports_effective_config: true
8+
reports_own_metrics: true
9+
reports_health: true
10+
accepts_remote_config: true
11+
reports_remote_config: true
12+
accepts_restart_command: true
13+
14+
storage:
15+
directory: "{{.storage_dir}}"
16+
17+
agent:
18+
executable: ../../bin/otelcontribcol_{{.goos}}_{{.goarch}}{{.extension}}
19+
health_check_port: "{{ .healthcheck_port }}"

0 commit comments

Comments
 (0)