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 new parameters for collector.service: services-list and custom labels #1185

Conversation

peekjef72
Copy link
Contributor

replace previous PR PR#1180

This PR adds some feature to service collector:

  • add config parameter collector.service.services-list with a comma separated list of service names:
collector:
 service:
   services-list: windows_exporter, winRM, pushprox_client, Dhcp

the param will only be checked if services-where is not set.
the list will be used to build a service-where query based on Name and is equivalent to:

collector:
  service:
    services-where: "Name = 'elmt_1' or Name = "elmt_X' or ..."
  • add a new parameter that can only be set in config file:
    It allows to set any number of custom labels value for each service:
    e.g.:
collector:
  service:
    services:
      windows_exporter:
        application: prometheus
        custom1: val1
      pushprox_client:
        application: prometheus
        custom1: val1
      winRM:
        application: windows
        custom1: val2
      Dhcp:
        application: windows
        custom1: val3

Use case: allow to build a generic Prometheus alert on not running service and to use the specified associated labels to drive a specific behavior. For me they are used to route for a specific documentation based on context.

Label's names must be identical for each service. Not identical labels names are removed !
This parameter as a lower priority than service-where and service-list.
It accepts a dict (see above example) or a list; in this case it behaves like services-list.
e.g.:

collector:
  service:
    services:
      - dhcp
      - pushprox_client
      - windows_exporter
      - winRM

Then generic alert services:

groups:
- name: Window Server Alerts
  rules:

  # Sends an alert when the 'sqlserveragent' service is not in the running state for 3 minutes.
  - alert: WindowServiceNotRunning
    expr: windows_service_state{state="running"} == 0
    for: 3m
    labels:
      severity: high
    annotations:
      summary: "Service {{ $labels.name }} for {{ $labels.application }} down for 3 min."
      description: "Service {{ $labels.name }} for Application {{ $labels.application }} on instance {{ $labels.instance }} has been down for more than 3 minutes."

@peekjef72 peekjef72 requested a review from a team as a code owner April 15, 2023 10:12
@peekjef72 peekjef72 mentioned this pull request Apr 15, 2023
Copy link
Contributor

@breed808 breed808 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been reviewing each commit individually, and it appears they may be out of order?
9d8270f fails to build:

$ git status
HEAD detached at 9d8270f
...
$ go build
# github.com/prometheus-community/windows_exporter/collector
collector/init.go:17:33: undefined: config.ConfigHook
collector/init.go:253:21: undefined: ServiceBuildHook
collector/collector.go:59:51: undefined: config.ConfigHook
collector/collector.go:62:87: undefined: config.ConfigHook
collector/collector.go:85:38: undefined: config.ConfigHook

mtimes[f.Name()] = f.ModTime()
info, err := f.Info()
if err != nil {
error = 1.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's be worth adding a log entry here, to prevent user confusion in the event of a failure.

@peekjef72
Copy link
Contributor Author

I've been reviewing each commit individually, and it appears they may be out of order? 9d8270f fails to build:

$ git status
HEAD detached at 9d8270f
...
$ go build
# github.com/prometheus-community/windows_exporter/collector
collector/init.go:17:33: undefined: config.ConfigHook
collector/init.go:253:21: undefined: ServiceBuildHook
collector/collector.go:59:51: undefined: config.ConfigHook
collector/collector.go:62:87: undefined: config.ConfigHook
collector/collector.go:85:38: undefined: config.ConfigHook

I'm a little bit confused with what you have tried to do.
I have committed files by group of changes ( what they do), not by independant function.
The program globally compiles. That is all I can say.
Do you prefer I post a single global commit ?

@breed808
Copy link
Contributor

You're on the right track; each commit should be a self-contained change.
However 9d8270f wasn't self-contained as it referenced objects that didn't exist (config.ConfigHook, ServiceBuildHook, config.ConfigHook).

These were likely introduced in a later commit. Interactively rebasing the feature branch such that 9d8270f occurs after the commit introducing these objects should have worked.

A single, global commit does work but can become difficult to review once it reaches a certain size.

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

Successfully merging this pull request may close these issues.

None yet

2 participants