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

Making possible to add rules via docker secret #57

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

albertogviana
Copy link

I am using DFM to monitoring my docker swarm cluster, and I am also using it to monitoring machines that are outside docker swarm. I know it is possible to create scrapes files, although rules files were not possible. I did some changes, and now it is possible to create rules files as docker secrets, the filename needs to have .rules as a suffix.

Below you will find an example:

Creating a new rule file:

echo 'groups:
- name: node.rules
  rules:
  - alert: instance_down
    expr: up == 0
    for: 5m
    labels:
      severity: high
    annotations:
      summary: "Instance {{$labels.instance}} down"
      description: "{{$labels.instance}} of job {{$labels.job}} has been down for more than 5 minutes."
  - alert: disk_will_fill_in_4_hours
    expr: predict_linear(node_filesystem_free{service="exporter_node-exporter",fstype="ext4"}[1h], 4 * 3600) < 0
    for: 5m
    labels:
      severity: high
    annotations:
      summary: "Disk will fill in 4 hours in the {{$labels.node}}"
      description: "The {{$labels.device}} disk in the {{$labels.node}} node will fill in 4 hours." ' | docker secret create node.rules -

Adding it to docker-flow-monitor.yml file:

version: "3"
services:
  monitor:
    image: dockerflow/docker-flow-monitor:${TAG:-latest}
    environment:
      - GLOBAL_SCRAPE_INTERVAL=10s
    secrets:
      - node.rules
    networks:
      - monitor
    ports:
      - 9090:9090
networks:
    monitor:
       external: true
secrets:
  node.rules:
    external: true

I don't know if you want to have this feature, but considering that I did the change, I would like to share it. If this change is ok for you, I can also update the documentation.

@vfarcic
Copy link
Collaborator

vfarcic commented Aug 18, 2018

Sorry for not reviewing this earlier...

The PR looks great. I'd appreciate if you could extend the docs with this new feature. Also, can you write a sentence or two for the release notes?

@albertogviana
Copy link
Author

Yes, I can do that.

Copy link
Collaborator

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Overall, this PR looks good.

What was the motivation of using docker secrets rather than docker config?

if len(alerts) > 0 {
logPrintf("Writing to alert.rules")
afero.WriteFile(FS, alertRulesPath, []byte(GetAlertConfig(alerts)), 0644)
c.RuleFiles = []string{"alert.rules"}
// c.RuleFiles = []string{"alert.rules"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This commented line can be removed.

@@ -33,10 +33,12 @@ func WriteConfig(configPath string, scrapes map[string]Scrape,
c.InsertScrapesFromDir(configsDir)
}

c.RuleFiles = []string{"/run/secrets/*.rules"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

How do you feel about using: https://godoc.org/github.com/spf13/afero#Glob to explicitly scan for all the rules, adding these rules to prometheus.yml and logging the rules here?

Copy link
Author

@albertogviana albertogviana Aug 28, 2018

Choose a reason for hiding this comment

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

Hi @thomasjpfan,

I am using secrets because since the beginning I am injecting the scrapes via secret, so this was my first approach.

I believe I don't need to use https://godoc.org/github.com/spf13/afero#Glob because Prometheus already implemented it.

Yes @vfarcic , I can provide some documentation and release notes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@albertogviana Yes. Please add the docs and release notes.

@thomasjpfan Do you think we can merge this after the docs and RNs are done by @albertogviana ?

@vfarcic
Copy link
Collaborator

vfarcic commented Jul 6, 2019

Sorry for not responding earlier... I was traveling for two weeks and could not find time to go through this PR.

I'll do my best to go through it sometime next week.

As a side note @dorsany, this project is looking for adoption. How about I make you a contributor?

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.

5 participants