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 tags as labels to prometheus metrics #898

Conversation

proffalken
Copy link
Contributor

@proffalken proffalken commented Nov 9, 2021

Description

Fixes #680 - Once working, this should add any tags added in the UI as labels for Prometheus

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and test it
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

Please do not use any external image service. Instead, just paste in or drag and drop the image here and it will be uploaded automatically.

@proffalken proffalken changed the title Add tags as labels to prometheus metrics [WIP] Add tags as labels to prometheus metrics Nov 9, 2021
@proffalken
Copy link
Contributor Author

@chakflying / @louislam - I'm not sure how to do this in NodeJS, hopefully this is a reasonable start to show the kind of thing I'm trying to achieve?

Instead of

monitor_status{monitor_name="news.bbc.co.uk",monitor_type="http",monitor_url="https://news.bbc.co.uk",monitor_hostname="null",monitor_port="null"} 1

then I should see

monitor_status{monitor_name="news.bbc.co.uk",monitor_type="http",monitor_url="https://news.bbc.co.uk",monitor_hostname="null",monitor_port="null", my_tag_1="my_value_1", my_tag_2="my_value_2"} 1

Does that make sense?

@chakflying
Copy link
Collaborator

If that's the data structure you want, you would probably do this:

for (const tag of monitor.tags) {
  this.monitorLabelValues[tag.name] = tag.value
}

@proffalken
Copy link
Contributor Author

proffalken commented Nov 10, 2021

Looks like the tags aren't part of the monitor object?

console.log(monitor); returns the following

Monitor {
  beanMeta: BeanMeta {
    noCache: false,
    fetchAs: '',
    alias: '',
    via: '',
    withCondition: '',
    withConditionData: []
  },
  _id: 5,
  _name: 'news.bbc.co.uk',
  _active: 1,
  _userId: 1,
  _interval: 60,
  _url: 'https://news.bbc.co.uk',
  _type: 'http',
  _weight: 2000,
  _hostname: null,
  _port: null,
  _createdDate: '2021-11-09 15:40:17',
  _keyword: null,
  _maxretries: 0,
  _ignoreTls: 0,
  _upsideDown: 0,
  _maxredirects: 10,
  _acceptedStatuscodesJson: '["200-299"]',
  _dnsResolveType: 'A',
  _dnsResolveServer: '1.1.1.1',
  _dnsLastResult: null,
  _retryInterval: 60,
  _pushToken: null,
  _method: 'GET',
  _body: null,
  _headers: null
}

however I'd expect it to include two tags based on the data in the UI:
image

server/prometheus.js Outdated Show resolved Hide resolved
server/prometheus.js Outdated Show resolved Hide resolved
server/prometheus.js Outdated Show resolved Hide resolved
Copy link
Contributor

@Saibamen Saibamen left a comment

Choose a reason for hiding this comment

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

Please use ESLint!

You are using var in your code...

@proffalken
Copy link
Contributor Author

Please use ESLint!

You are using var in your code...

Yup, I learned Javascript nearly 20 years ago and spend most of my life writing other languages. I had no idea that var had been replaced. I've added eslint to my codebase and fixed the syntax issues, however it doesn't fix the actual issue that the tags in monitor.tags don't show up as prometheus labels.

Any ideas how I get this working?

@proffalken proffalken force-pushed the feature/680_add_labels_to_prometheus_metrics branch from 6acc10b to 0e6e42c Compare November 16, 2021 19:34
@proffalken proffalken force-pushed the feature/680_add_labels_to_prometheus_metrics branch from 0e6e42c to 144b8f1 Compare November 16, 2021 19:36
@chakflying
Copy link
Collaborator

Currently we use the ORM in a very raw way, meaning that table joins have to be done manually. You can check monitor.js:52 for how this is done. But it does seem a bit weird that we need to have another DB query everytime we want to use tags. Maybe @louislam will know if the ORM can do this automatically.

@proffalken
Copy link
Contributor Author

@louislam - I'm hoping to find time to come back to this shortly, is the ORM capable of doing the links or should I just go for a raw query?

@proffalken proffalken mentioned this pull request Dec 31, 2021
@proffalken
Copy link
Contributor Author

@chakflying - I've just found time to come back to this.

I've added some code that does a lookup and pulls back the tags, however I can't seem to get them to be passed through to the correct dict.

In the logs, I get the following:

Getting Tags for Prometheus
TAGS: [object Promise]
Found the following tags for 1 :
[
  {
    id: 1,
    monitor_id: 1,
    tag_id: 1,
    value: 'garage',
    name: 'location',
    color: '#4B5563'
  },
  {
    id: 2,
    monitor_id: 1,
    tag_id: 2,
    value: 'se-1',
    name: 'region',
    color: '#DC2626'
  }
]

but I'd expect to see all of the tags listed on the line that starts TAGS, rather than a promise?

I've tried to add an await to the call to get_tags(monitor), but this results in the following error:

/home/mmw/Projects/uptime-kuma/server/prometheus.js:58
        let tags = await this.get_tags(monitor);
                   ^^^^^

SyntaxError: await is only valid in async functions and the top level bodies of modules
    at Object.compileFunction (node:vm:352:18)
    at wrapSafe (node:internal/modules/cjs/loader:1026:15)
    at Module._compile (node:internal/modules/cjs/loader:1061:27)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1151:10)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:999:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/home/mmw/Projects/uptime-kuma/server/model/monitor.js:8:24)
    at Module._compile (node:internal/modules/cjs/loader:1097:14)

Any idea how I can get around this?

@proffalken
Copy link
Contributor Author

OK, the latest code now takes specific tags and applies them as labels to Prometheus metrics.

This isn't ideal, because I'd really like to have the labels added dynamically, but it's a constraint from the upstream Prom-client package, so it will have to do for now.

image

image

@proffalken proffalken changed the title [WIP] Add tags as labels to prometheus metrics Add tags as labels to prometheus metrics Feb 21, 2022
@proffalken
Copy link
Contributor Author

Tags that will automatically be turned into labels are:

    "location",
    "region",
    "datacenter",
    "cloud_provider",
    "az",
    "rack",
    "shelf",
    "room",
    "floor"

and new ones can be added by updating https://github.com/louislam/uptime-kuma/pull/898/files#diff-a2ea08464c146b6af2888bdf744cb4789cac4bcda6731289d02431bcc9451363R10-R18

Once this is merged, I'll update the wiki accordingly.

Tags that do not meet one of the above names are ignored silently

server/prometheus.js Outdated Show resolved Hide resolved
server/prometheus.js Outdated Show resolved Hide resolved
server/prometheus.js Outdated Show resolved Hide resolved
@proffalken
Copy link
Contributor Author

@chakflying / @louislam - any chance you can take a look at this please?

@louislam louislam added this to the 1.15.0 milestone Apr 12, 2022
@louislam
Copy link
Owner

Looking for a tester who are using prometheus to test this pull request.

spali added a commit to spali/uptime-kuma that referenced this pull request Jan 24, 2023
spali added a commit to spali/uptime-kuma that referenced this pull request Jan 24, 2023
spali added a commit to spali/uptime-kuma that referenced this pull request Jan 24, 2023
@spali spali mentioned this pull request Feb 23, 2023
7 tasks
@burakksglu
Copy link

burakksglu commented Mar 16, 2023

Hey everyone! Your suggestions are great. I was testing the Prometheus endpoint with Grafana and found this PR. In the endgame, the Prometheus endpoint should include some of the parameters in api.get_monitors() of API also. Even the Tag parameter alone would add much more flexibility.

For dashboarding and reporting purposes, these would be great additions for users that can't access UI because of the lack of user permissions:

'accepted_statuscodes' 'active' 'expiryNotification' 'id' 'interval' 'keyword' 'maintenance' 'method' 'name' 'port' 'proxyId' 'tags' 'type' 'upsideDown' 'url'

Reference:https://uptime-kuma-api.readthedocs.io/en/latest/api.html#uptime_kuma_api.UptimeKumaApi.get_monitors

@michaelyork
Copy link

Any progress here by chance? Happy to sponsor someone's work to get this shipped!

@spali
Copy link
Contributor

spali commented Apr 14, 2023

First preparation PR was merged.
Waiting for #2830 to be merged. So I can create the last PR which should solve/replace this PR.

Wanted to split it to make the reviews easier.

@mhkarimi1383

This comment was marked as spam.

@mabed-fr
Copy link

mabed-fr commented Jun 25, 2023

Do you want more tests ? it works fine.

@mabed-fr mabed-fr mentioned this pull request Jun 28, 2023
1 task
@mateuszdrab

This comment was marked as spam.

@CommanderStorm

This comment was marked as outdated.

@chakflying chakflying added blocked cannot progress because of external reasons and removed question Further information is requested labels Dec 4, 2023
@CommanderStorm CommanderStorm added the area:metrics related to monitoring metrics label Dec 8, 2023
@mabed-fr
Copy link

Where is the PR, what are we missing?

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Apr 21, 2024

#2830 was merged 1 month ago
=> previous block is now resolved

@mabed-fr
Copy link

#2830 was merged 1 month ago => previous block is now resolved

What is the logical next step?

spali added a commit to spali/uptime-kuma that referenced this pull request Apr 22, 2024
spali added a commit to spali/uptime-kuma that referenced this pull request Apr 22, 2024
spali added a commit to spali/uptime-kuma that referenced this pull request Apr 22, 2024
@spali
Copy link
Contributor

spali commented Apr 22, 2024

See 4704
I didn't had time to retest after fixing the merge conflicts... wrote and tested the code over a year ago ... I would appreciate if someone could test a bit my PR.

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Apr 22, 2024

Thanks for the wor that went into this PR

Closing in favor of #4704 as said PR seems to also handle the editing of tags.

=> lets continue the conversation there.

spali added a commit to spali/uptime-kuma that referenced this pull request Apr 30, 2024
spali added a commit to spali/uptime-kuma that referenced this pull request May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics related to monitoring metrics blocked cannot progress because of external reasons
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend existing Prometheus labels to include tags