-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[cmd/opampsupervisor]: Allow configuring agent description #32819
Merged
andrzej-stencel
merged 9 commits into
open-telemetry:main
from
observIQ:feat/supervisor-agent-description
May 15, 2024
Merged
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
d4b2472
add agent description to supervisor config
BinaryFissionGames 02a5c4a
add time.sleep (race condition prevents shutdown)
BinaryFissionGames 10cf481
add chlog entry
BinaryFissionGames 49b1fe1
add agent description config to spec
BinaryFissionGames a65064c
remove zaptest logger
BinaryFissionGames 4131185
add license header
BinaryFissionGames 67aa2f1
update issue number
BinaryFissionGames 8e83150
Allow overriding instance id
BinaryFissionGames cf57dfa
Update cmd/opampsupervisor/specification/README.md
BinaryFissionGames 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# 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: Adds the ability to configure the agent description | ||
|
||
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. | ||
issues: [32644] |
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
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,87 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package config | ||
|
||
import ( | ||
"net/http" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
"go.opentelemetry.io/collector/config/configtls" | ||
semconv "go.opentelemetry.io/collector/semconv/v1.21.0" | ||
) | ||
|
||
func TestValidate(t *testing.T) { | ||
testCases := []struct { | ||
name string | ||
config Supervisor | ||
expectedError string | ||
}{ | ||
{ | ||
name: "Empty Config is valid", | ||
config: Supervisor{}, | ||
}, | ||
{ | ||
name: "Valid filled out config", | ||
config: Supervisor{ | ||
Server: &OpAMPServer{ | ||
Endpoint: "wss://localhost:9090/opamp", | ||
Headers: http.Header{ | ||
"Header1": []string{"HeaderValue"}, | ||
}, | ||
TLSSetting: configtls.ClientConfig{ | ||
Insecure: true, | ||
}, | ||
}, | ||
Agent: &Agent{ | ||
Executable: "../../otelcol", | ||
Description: AgentDescription{ | ||
IdentifyingAttributes: map[string]string{ | ||
"client.id": "some-client-id", | ||
}, | ||
NonIdentifyingAttributes: map[string]string{ | ||
"env": "dev", | ||
}, | ||
}, | ||
}, | ||
Capabilities: &Capabilities{ | ||
AcceptsRemoteConfig: asPtr(true), | ||
}, | ||
Storage: &Storage{ | ||
Directory: "/etc/opamp-supervisor/storage", | ||
}, | ||
}, | ||
}, | ||
{ | ||
name: "Cannot override instance ID", | ||
config: Supervisor{ | ||
Agent: &Agent{ | ||
Executable: "../../otelcol", | ||
Description: AgentDescription{ | ||
IdentifyingAttributes: map[string]string{ | ||
semconv.AttributeServiceInstanceID: "instance-id", | ||
}, | ||
}, | ||
}, | ||
}, | ||
expectedError: `cannot override identifying attribute "service.instance.id"`, | ||
}, | ||
} | ||
|
||
for _, tc := range testCases { | ||
t.Run(tc.name, func(t *testing.T) { | ||
err := tc.config.Validate() | ||
|
||
if tc.expectedError == "" { | ||
require.NoError(t, err) | ||
} else { | ||
require.ErrorContains(t, err, tc.expectedError) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func asPtr[T any](t T) *T { | ||
return &t | ||
} |
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
20 changes: 20 additions & 0 deletions
20
cmd/opampsupervisor/testdata/supervisor/supervisor_agent_description.yaml
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,20 @@ | ||
server: | ||
endpoint: ws://{{.url}}/v1/opamp | ||
tls: | ||
insecure: true | ||
|
||
capabilities: | ||
reports_effective_config: true | ||
reports_own_metrics: true | ||
reports_health: true | ||
accepts_remote_config: true | ||
reports_remote_config: true | ||
accepts_restart_command: true | ||
|
||
agent: | ||
executable: ../../bin/otelcontribcol_{{.goos}}_{{.goarch}}{{.extension}} | ||
description: | ||
identifying_attributes: | ||
client.id: "my-client-id" | ||
non_identifying_attributes: | ||
env: "prod" |
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.
Without this sleep, the supervisor gets stuck shutting down.
This is the same race I had to address in #32618 in order to get tests to run properly for that PR. This can be removed once the fix from that PR is merged.