Skip to content
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

Add metrics output #5

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

Add metrics output #5

wants to merge 35 commits into from

Conversation

jspaleta
Copy link

@jspaleta jspaleta commented Mar 4, 2022

  • Make search configuration optional, allows generating metrics for all processes
  • Convert alert output summary text into prom comment strings
  • Add procstat metric family output for Sumo Logic dashboard compatibility
  • Add processes metric family output for Sumo Logic dashboard compatibility

@jspaleta jspaleta marked this pull request as draft March 4, 2022 00:24
@jspaleta jspaleta linked an issue Mar 4, 2022 that may be closed by this pull request
@jspaleta jspaleta marked this pull request as ready for review March 4, 2022 23:29
@calebhailey
Copy link

I think we might need to bump the version in go.mod to 1.17... otherwise I was getting a compile error on UnixMilli

$ go build                                                                                                                      
# github.com/sensu/sensu-processes-check
./metrics.go:24:21: time.Now().UnixMilli undefined (type time.Time has no field or method UnixMilli)
$ go version
go version go1.16.3 linux/amd64

@calebhailey
Copy link

calebhailey commented Mar 10, 2022

I think we should drop the hard-coded host.name tag. I know it's there for dashboard compat, but we should use output_metric_tags for that. We should try to keep any built-in tags as minimal as possible.

Same for the search_string tag. Although you couldn't add this tag via output_metric_tags when searching for multiple processes, it seems likely that it might often be the same as process.executable.name.

@calebhailey
Copy link

calebhailey commented May 12, 2022

Encountered an error when configured with output_metrics_format: prometheus_text:

{
  "check": "process-monitoring",
  "component": "agent",
  "error": "unable to extract metric from check output",
  "level": "error",
  "msg": "text format parsing error in line 3: expected '=' after label name, found '.'",
  "namespace": "default",
  "time": "2022-05-12T01:07:02Z"
}

From the Prometheus docs:

Label names may contain ASCII letters, numbers, as well as underscores. They must match the regex [a-zA-Z_][a-zA-Z0-9_]*. Label names beginning with __ are reserved for internal use.

We'll need to change the following labels:

  • host.name: let's remove this (this or an alternative may be added via output_metric_tags)
  • process.executable.name: rename to process_executable_name or process_name or name?
  • process.executable.pid: rename to process_executable_pid or process_pid or pid?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for generating metrics
2 participants