[GH-ISSUE #370] Should Checks List return UUID? #283

Closed
opened 2026-02-25 23:41:53 +03:00 by kerem · 9 comments
Owner

Originally created by @jameskirsop on GitHub (May 27, 2020).
Original GitHub issue: https://github.com/healthchecks/healthchecks/issues/370

I'm wondering if it makes sense that for both write and read-only API calls, that the /api/v1/checks endpoint returns the UUID for each check.

For my Grafana integration, I'd like to be able to query for all checks (using a read only account), and then allow the user to select a particular check. From that point forward it makes sense only query for the one check (to be more efficient), but currently the checks endpoint doesn't return the UUID so I can't store it for the purposes of querying specifically for it.

If we think this makes sense, I'll put together a PR.

Originally created by @jameskirsop on GitHub (May 27, 2020). Original GitHub issue: https://github.com/healthchecks/healthchecks/issues/370 I'm wondering if it makes sense that for both write and read-only API calls, that the `/api/v1/checks` endpoint returns the UUID for each check. For my Grafana integration, I'd like to be able to query for all checks (using a read only account), and then allow the user to select a particular check. From that point forward it makes sense only query for the one check (to be more efficient), but currently the checks endpoint doesn't return the UUID so I can't store it for the purposes of querying specifically for it. If we think this makes sense, I'll put together a PR.
kerem closed this issue 2026-02-25 23:41:53 +03:00
Author
Owner

@cuu508 commented on GitHub (May 27, 2020):

The UUID is left out on purpose: if you know check's UUID you can easily construct its ping URL. We want to be able to use the readonly API in public dashboards, where exposing ping URLs could cause problems.

To uniquely identify a check without exposing its ping URL there is the unique_key field. We could look at adding a unique_key query parameter. There's already support for filtering by tag:

https://healthchecks.io/api/v1/checks/?tag=foo&tag=bar

Similarly, we could add:

https://healthchecks.io/api/v1/checks/?unique_key=...
<!-- gh-comment-id:634471987 --> @cuu508 commented on GitHub (May 27, 2020): The UUID is left out on purpose: if you know check's UUID you can easily construct its ping URL. We want to be able to use the readonly API in public dashboards, where exposing ping URLs could cause problems. To uniquely identify a check without exposing its ping URL there is the `unique_key` field. We could look at adding a `unique_key` query parameter. There's already support for filtering by tag: https://healthchecks.io/api/v1/checks/?tag=foo&tag=bar Similarly, we could add: https://healthchecks.io/api/v1/checks/?unique_key=...
Author
Owner

@jameskirsop commented on GitHub (May 27, 2020):

That seems to make sense, though passing the unique key without having to use a query string would be cleaner, if it's possible without reinventing the wheel.

I'll have a think about some options and post back with some ideas unless someone comes up with one first.

<!-- gh-comment-id:634511422 --> @jameskirsop commented on GitHub (May 27, 2020): That seems to make sense, though passing the unique key without having to use a query string would be cleaner, if it's possible without reinventing the wheel. I'll have a think about some options and post back with some ideas unless someone comes up with one first.
Author
Owner

@jameskirsop commented on GitHub (May 28, 2020):

The issue I've discovered with using unique_key, is that we can't filter on it using a QuerySet.

This makes it inefficient to use, because to filter on it we'd need to load all the Checks and do a list comprehension on the QuerySet to filter for the one with the matching key.

Given the unique_key doesn't change, I suggest it becomes a property that's saved in the database, so that we can filter for it.

<!-- gh-comment-id:635052768 --> @jameskirsop commented on GitHub (May 28, 2020): The issue I've discovered with using `unique_key`, is that we can't filter on it using a QuerySet. This makes it inefficient to use, because to filter on it we'd need to load all the Checks and do a list comprehension on the QuerySet to filter for the one with the matching key. Given the `unique_key` doesn't change, I suggest it becomes a property that's saved in the database, so that we can filter for it.
Author
Owner

@cuu508 commented on GitHub (May 29, 2020):

@jameskirsop kudos on the completeness of the PR. You didn't cut any corners, which is great to see!

I have some performance concerns though:

The unique_key column in the PR doesn't have an index. GET SITE_ROOT/api/v1/checks/<unique_key> will require a sequential scan in the database. This would be a problem on bigger installations.

Adding an index on the unique_key column could be problematic too: the api_check table gets updated on every ping. When a row is updated, the index pointing to it gets updated too. So adding one extra index has a performance cost for every single ping.

On comparison,

https://healthchecks.io/api/v1/checks/?unique_key=...

selects checks by project_id which is a foreign key, and so has an index already. Fetching, say, 1000 checks and filtering on the client side appears inelegant for sure. But it may in fact be the more performant solution:

  • everyone uses ping API, not everyone will use the "fetch check by unique_key" API
  • even if you have to do 1000 SHA1 operations per API call in Python, I think that's a negligible performance cost on today's CPUs
  • if you wanted to optimize traffic from database, you could fetch just UUIDS initially (but would have to do measurements to see if it's worth the extra complexity and the extra SQL query)
<!-- gh-comment-id:635802819 --> @cuu508 commented on GitHub (May 29, 2020): @jameskirsop kudos on the completeness of the PR. You didn't cut any corners, which is great to see! I have some performance concerns though: The `unique_key` column in the PR doesn't have an index. `GET SITE_ROOT/api/v1/checks/<unique_key>` will require a sequential scan in the database. This would be a problem on bigger installations. Adding an index on the `unique_key` column could be problematic too: the `api_check` table gets updated on every ping. When a row is updated, the index pointing to it gets updated too. So adding one extra index has a performance cost for every single ping. On comparison, https://healthchecks.io/api/v1/checks/?unique_key=... selects checks by `project_id` which is a foreign key, and so has an index already. Fetching, say, 1000 checks and filtering on the client side appears inelegant for sure. But it may in fact be the more performant solution: * everyone uses ping API, not everyone will use the "fetch check by unique_key" API * even if you have to do 1000 SHA1 operations per API call in Python, I think that's a negligible performance cost on today's CPUs * if you wanted to optimize traffic from database, you could fetch just UUIDS initially (but would have to do measurements to see if it's worth the extra complexity and the extra SQL query)
Author
Owner

@cuu508 commented on GitHub (May 29, 2020):

Looking at it some more, in api.views.get_check, we could change

check = get_object_or_404(Check, unique_key=code)

with

check = get_object_or_404(Check, unique_key=code, project=request.project)    

which would then avoid a sequential scan.

<!-- gh-comment-id:635806593 --> @cuu508 commented on GitHub (May 29, 2020): Looking at it some more, in `api.views.get_check`, we could change check = get_object_or_404(Check, unique_key=code) with check = get_object_or_404(Check, unique_key=code, project=request.project) which would then avoid a sequential scan.
Author
Owner

@cuu508 commented on GitHub (May 29, 2020):

I like the /api/v1/checks/<unique_key> route, compared to the extra query parameter which I had suggested (/v1/checks/?unique_key=...).

I'm not convinced about adding an extra column as it requires a schema and data migration, and an overriden Check.save() method.

I think I would prefer if there would be two separate get_check views – one for fetching by UUID (it would work same as currently) and a separate one for fetching by unique_key. This one would fetch checks by project_id and then filter them by unique_key in Python code.

<!-- gh-comment-id:635810729 --> @cuu508 commented on GitHub (May 29, 2020): I like the `/api/v1/checks/<unique_key>` route, compared to the extra query parameter which I had suggested (`/v1/checks/?unique_key=...`). I'm not convinced about adding an extra column as it requires a schema and data migration, and an overriden `Check.save()` method. I think I would prefer if there would be two separate `get_check` views – one for fetching by UUID (it would work same as currently) and a separate one for fetching by `unique_key`. This one would fetch checks by `project_id` and then filter them by `unique_key` in Python code.
Author
Owner

@jameskirsop commented on GitHub (Jun 11, 2020):

Looking at it some more, in api.views.get_check, we could change

check = get_object_or_404(Check, unique_key=code)

with

check = get_object_or_404(Check, unique_key=code, project=request.project)    

which would then avoid a sequential scan.

For this to work, we'd have to have unique_key as a database column.

I don't mind the suggestion of having two different functions for the different views. My main issue is not being able to do a get_object_or_404 at the start of the function and having the code do more work than is otherwise/typically required to be able to check if the object exists (and even then, you wouldn't be able to use the get_object_or_404 shortcut).

Nonetheless, a new PR has been raised with changes that are in line with the desires of this ticket in the hope this can be merged to master soon.

<!-- gh-comment-id:642454542 --> @jameskirsop commented on GitHub (Jun 11, 2020): > Looking at it some more, in `api.views.get_check`, we could change > > ``` > check = get_object_or_404(Check, unique_key=code) > ``` > > with > > ``` > check = get_object_or_404(Check, unique_key=code, project=request.project) > ``` > > which would then avoid a sequential scan. For this to work, we'd have to have `unique_key` as a database column. I don't mind the suggestion of having two different functions for the different views. My main issue is not being able to do a `get_object_or_404` at the start of the function and having the code do more work than is otherwise/typically required to be able to check if the object exists (and even then, you wouldn't be able to use the `get_object_or_404` shortcut). Nonetheless, a new PR has been raised with changes that are in line with the desires of this ticket in the hope this can be merged to master soon.
Author
Owner

@cuu508 commented on GitHub (Jun 11, 2020):

Great, thanks for the updated PR, merged!

I made one noteworthy change:

In urls.py, instead of routing both UUIDs and unique_keys to the same view, we can route unique_keys directly to the get_check_unique (renamed to get_check_by_unique_key) view. This way:

  • don't need the if type(code) = ... part
  • don't need to worry about what would happen if somebody does POST /api/v1/checks/<unique_key> or DELETE /api/v1/checks/<unique_key>.
<!-- gh-comment-id:642613782 --> @cuu508 commented on GitHub (Jun 11, 2020): Great, thanks for the updated PR, merged! I made one noteworthy change: In urls.py, instead of routing both UUIDs and unique_keys to the same view, we can route unique_keys directly to the `get_check_unique` (renamed to `get_check_by_unique_key`) view. This way: * don't need the `if type(code) = ...` part * don't need to worry about what would happen if somebody does `POST /api/v1/checks/<unique_key>` or `DELETE /api/v1/checks/<unique_key>`.
Author
Owner

@cuu508 commented on GitHub (Jun 11, 2020):

My main issue is not being able to do a get_object_or_404 at the start of the function

Yeah, on the python side it doesn't look as elegant as get_object_or_404 for sure. But the alternative has drawbacks too:

  • needs a database migration to add a column
  • needs a data migration to populate the new column for existing checks
  • needs code to populate it for all future checks
  • the checks table becomes 40 bytes wider. Database now has to do a bit more work every time it is updating a row in the checks table
  • if the unique_key column is not indexed, the database still has to iterate over multiple rows (though it is would now be done inside database, not in python code)

I'd like to start with the naive approach, and revisit if performance starts to become a problem.

<!-- gh-comment-id:642632946 --> @cuu508 commented on GitHub (Jun 11, 2020): >My main issue is not being able to do a get_object_or_404 at the start of the function Yeah, on the python side it doesn't look as elegant as get_object_or_404 for sure. But the alternative has drawbacks too: * needs a database migration to add a column * needs a data migration to populate the new column for existing checks * needs code to populate it for all future checks * the checks table becomes 40 bytes wider. Database now has to do a bit more work every time it is updating a row in the checks table * if the unique_key column is not indexed, the database still has to iterate over multiple rows (though it is would now be done inside database, not in python code) I'd like to start with the naive approach, and revisit if performance starts to become a problem.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/healthchecks#283
No description provided.