feature/psa-metrics #969
feature/psa-metrics #969
Conversation
packages/db/postgres/functions.sql
Outdated
| @@ -325,6 +325,19 @@ BEGIN | |||
| END | |||
| $$; | |||
|
|
|||
| CREATE OR REPLACE FUNCTION uploads_by_type(query_type TEXT) RETURNS TEXT | |||
There was a problem hiding this comment.
I'm wondering why do we need a procedure here? Shouldn't we just use client count API?
I see that's how it is done for other metrics, but I'm not sure if/why it's needed?
I wonder if was done like this because of the Supabase client issue loading the all table even when just "counting"?
But that can be workarounded by adding range(0, 1)
Probably one for @vasco-santos
There was a problem hiding this comment.
For sum we can't do with the client, and that was the main motivation to not use the client. The getPinStatusMetrics using pin_from_status_total I don't recall if there was a reason. Either the cast to pin status, or was just added here with the sums and we didn't consider using the client.
We use the client for other metrics counts, so if we can use it here, I think we should
There was a problem hiding this comment.
@alexandrastoica did you find a reason to not have it using supabase client?
There was a problem hiding this comment.
Will change to client! :)
|
|
||
| '# HELP web3storage_pins_status_queued_total Total number of pins that are queued.', | ||
| '# TYPE web3storage_pins_status_queued_total counter', | ||
| `web3storage_pins_status_queued_total ${pinsQueuedTotal}`, |
There was a problem hiding this comment.
I would like to change this. But we need to take into account older values. Should we rename things here to Prometheus best practises and do a sum in Grafana query with the older name?
packages/db/metrics.js
Outdated
| uploads_car_total: 'Car', | ||
| uploads_blob_total: 'Blob', | ||
| uploads_multipart_total: 'Multipart', | ||
| uploads_upload_total: 'Upload' |
There was a problem hiding this comment.
The DB client shouldn't be concerned with the keys we're using in the metrics. An actual upload "type" should be passed to getUploadTypeMetrics.
There was a problem hiding this comment.
LGTM 👌🏼
@alexandrastoica please make sure that when this PR is merged to leave a comment in the Release PR that when the PR is releases we need to update Grafana Dashboard linking to this PR (a Release PR for the API will be created or updated)
Thanks :)
alexandrastoica
dismissed
alanshaw’s stale review
Addressed the comment and got LGTM from Vasco
Adds metrics:
BEGIN_COMMIT_OVERRIDE
feat: update metrics and add psa pin requests to metrics
END_COMMIT_OVERRIDE