-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add agent config options for internal telemetry and health check, enable use of status cmd in kubernetes #182
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
Conversation
internal/commands/status/flags.go
Outdated
) | ||
|
||
const ( | ||
TelemetryEndpointFlag = "telemetry-endpoint" |
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.
I think actually the more useful use case here is that we could then provide overrides in the config.yaml
we generate in the helm chart. so we could add a telemetry_endpoint: "{{ template "config.local_host"}}:8888"
and that would work correctly. Since that's the case, I'd prefer if we stick to the snake_case style of naming instead of kebabcase and maybe we can nest these under something like endpoints::telemetry_endpoint
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.
Good point, I agree! I will update this PR after my config refactor to simplify the rebasing.
…able use in kubernetes
e4ce7df
to
6bd451c
Compare
}, | ||
} | ||
|
||
func printAllConfigsIndividually(configFilePaths []string) error { |
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.
This is all code motion; I put these methods in util
since the config package depends on the start package, and having these methods available in start
for debugging is very useful.
func GetAgentStatusFromHealthcheck(baseURL string) (AgentStatus, error) { | ||
URL := fmt.Sprintf("%s/status", baseURL) | ||
func GetAgentStatusFromHealthcheck(baseURL string, path string) (AgentStatus, error) { | ||
baseURL = util.ReplaceEnvString(baseURL) |
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.
This is to handle our default k8s use of ${env:MY_POD_IP}
@@ -119,53 +147,53 @@ func GetAgentMetricsFromEndpoint(baseURL string) (*AgentMetrics, error) { | |||
if v.Type.String() == io_prometheus_client.MetricType_HISTOGRAM.String() { | |||
met := v.Metric[0] | |||
switch name := *v.Name; name { | |||
case "otelcol_http_client_duration": | |||
case "http_client_duration_milliseconds": |
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.
These metric names changed; possibly to be inline with prometheus conventions after the config upgrade. I checked the output from the prometheus endpoint and verified the metric names as well as watching the numbers update when calling the status command.
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.
I verified that the names after this update match what's collected now from our prometheus exporter
"gopkg.in/yaml.v3" | ||
) | ||
|
||
func PrintAllConfigsIndividually(configFilePaths []string) error { |
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.
Here's where the config methods moved to. Again, no changes just code motion.
6bd451c
to
bdc1c4f
Compare
Description
Add agent config options for internal telemetry and health check. Enable the use of the agent
status
command in kubernetes. Also upgrade the internal telemetry config as our config for metrics is no longer supported: https://github.com/open-telemetry/opentelemetry-collector/releases/tag/v0.123.0Ex: