[nexus] add alert_list and alert_view external API endpoints - #11072
[nexus] add alert_list and alert_view external API endpoints#11072hawkw wants to merge 17 commits into
alert_list and alert_view external API endpoints#11072Conversation
this makes openapi-lint panic...
This reverts commit b5ea2c1.
Co-authored-by: Sean Klein <seanmarionklein@gmail.com>
ahl
left a comment
There was a problem hiding this comment.
I know we already discussed, but now that this is done I still think it looks good.
| ] | ||
| }, | ||
| "Alert": { | ||
| "description": "An alert.\n\nAlerts represent edge-triggered notifications of an event that occurred in the system at a point in time. See the guide-level documentation on alerts for details.", |
There was a problem hiding this comment.
What does “edge-triggered” mean here?
There was a problem hiding this comment.
Ok, I looked it up. I like the precision but it’s a little obscure maybe. Since the sentence already says alerts are of an event at a point in time, I think you could just drop it.
| "format": "date-time" | ||
| }, | ||
| "version": { | ||
| "description": "The schema version of this alert's data payload.\n\nAlert schemas are versioned on a per-alert-class basis. The schema version for a particular alert class does not correspond to an Oxide API version. Clients should expect to encounter earlier schema versions for a given alert class, if the alert was recorded on an earlier Oxide system software version.", |
There was a problem hiding this comment.
This is good but the last sentence could be clearer. How about:
Clients should expect to encounter earlier schema versions when retrieving alerts recorded by an earlier version of the system software.
I think the “for a given alert class” is covered by the previous sentence.
david-crespo
left a comment
There was a problem hiding this comment.
API looks good. Couple of small things, take it or leave it.
As discussed in #11005, the only way to view alerts in the public API is to list the alerts that were dispatched to an alert receiver. This API contains details about the delivery status of that dispatched alert, but does not contain the actual data payload of the alert. Furthermore, it means that there is no way to consume alerts without first creating a receiver, and alerts that a receiver is not subscribed to cannot be accessed. As we use alerts for more things, it will become desirable to be able to list alerts without going through alert receivers. For example, we probably want to have a page in the web console that shows a timeline of all alerts (regardless of whether a receiver is subscribed to them) which the API does not currently let us do.
This branch adds
alert_listandalert_viewAPI endpoints which list alerts, and fetch an alert by UUID, respectively. The alert list can be filtered based on a time range and/or an alert class or alert class glob. I allowed @ahl to convince me that we probably don't want the alert payload schema for every alert class/version pair to be included in the OpenAPI document for the external API, and we should instead communicate about alert payload schemas separately. Therefore, the alert payload is an untyped JSON blob, but we include the class and schema version so the schema for that payload can (eventually; see #8065) be determined.I also did a bit of refactoring to the
alert_class_listendpoint to share internals with some of the code I added in this branch.