-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat: api metrics #88
Conversation
4c1b44a
to
2c40586
Compare
Deploying with Cloudflare Pages
|
2c40586
to
d342a55
Compare
7572044
to
af2b015
Compare
af2b015
to
4bc0ac2
Compare
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.
LGTM, added some non-blocking suggestions.
packages/api/src/metrics.js
Outdated
`nftlinkapi_permacache_size_total ${sizeTotal}`, | ||
`# HELP nftlinkapi_permacache_events_total Total perma cache events.`, | ||
`# TYPE nftlinkapi_permacache_events_total counter`, | ||
`nftlinkapi_permacache_events_total ${eventsTotal}`, |
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.
Split by type?
`nftlinkapi_permacache_events_total ${eventsTotal}`, | |
`nftlinkapi_permacache_events_total{type="Put"} ${putEventsTotal}`, | |
`nftlinkapi_permacache_events_total{type="Delete"} ${deleteEventsTotal}`, |
`# HELP nftlinkapi_permacache_events_total Total perma cache events.`, | ||
`# TYPE nftlinkapi_permacache_events_total counter`, | ||
`nftlinkapi_permacache_events_total ${eventsTotal}`, | ||
].join('\n') |
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 used this prom-client
library in checkup https://github.com/nftstorage/checkup/blob/main/prom.js - it might make things a bit less error prone here.
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 will do this as follow up.
packages/cron/src/jobs/metrics.js
Outdated
const COUNT_USERS = ` | ||
SELECT COUNT(*) AS total | ||
FROM nftstorage.user u JOIN nftstorage.user_tag ut | ||
ON u.id = ut.user_id | ||
WHERE ut.tag = 'HasSuperHotAccess'::user_tag_type AND ut.value = 'true' | ||
` |
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.
Isn't this just a count of HasSuperHotAccess
tags?
const COUNT_USERS = ` | |
SELECT COUNT(*) AS total | |
FROM nftstorage.user u JOIN nftstorage.user_tag ut | |
ON u.id = ut.user_id | |
WHERE ut.tag = 'HasSuperHotAccess'::user_tag_type AND ut.value = 'true' | |
` | |
const COUNT_USERS = ` | |
SELECT COUNT(*) AS total | |
FROM nftstorage.user_tag | |
WHERE tag = 'HasSuperHotAccess'::user_tag_type AND value = 'true' | |
` |
7572f10
to
81103d0
Compare
This PR adds metrics endpoint to the API by:
From previous knowledge doing the queries in the API itself would work in the beginning. But let's not wait for fires, and go straight for cron job
Blocked on #86
Closes #56