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

[Dashboards][Discuss] Public API URL path #193758

Open
nickpeihl opened this issue Sep 23, 2024 · 7 comments
Open

[Dashboards][Discuss] Public API URL path #193758

nickpeihl opened this issue Sep 23, 2024 · 7 comments
Assignees
Labels
discuss Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@nickpeihl
Copy link
Member

Prior to making the Dashboards Public API available, we should decide on how best to organize the base URL path for the CRUD routes.

Two suggested base paths are

  1. /api/dashboards
  2. /api/dashboards/dashboard

The expected CRUD routes for Dashboards use the following HTTP verbs and URL templates.

Create
POST <BASEPATH>/{id?} - creates a new Dashboard using the attributes in the request body. In the URL id is the optionally specified saved object id. If id is not provided, then a random uuid is used.

Read
GET <BASEPATH>/{id} - retrieves a Dashboard with the matching saved object id.

Update
PUT <BASEPATH>/{id} - updates an existing Dashboard using the attributes in the request body. The id in the URL is required and must match an existing saved object.

Delete
DELETE <BASEPATH>/{id} - deletes an existing Dashboard that matches the provided saved object id.

List
GET <BASEPATH> - retrieves a paginated list of Dashboards. The URL may contain a query string that can be used to filter dashboards.

Kibana has a mixed usage when it comes to organizing API paths for CRUD:

  • Data views uses /api/data_views/data_view for CRUD routes. Based on a conversation with @mattkime the CRUD routes were likely organized under the data_views domain to allow for future paths like /api/data_vews/default for setting the default data view.
  • Alerting rules uses /api/alerting/rule for CRUD routes. But there are additional non-CRUD routes under the alerting domain such as rule_types and _health. This path also has the deprecated /api/alerting/alert under the alerting domain.
  • Elastic agents does not appear to register a Create route. But it uses /api/agents as the basepath for the Read, Update, and Delete routes. Additionally some other operations such as bulk reassign and initiate agent setup fall under the agents domain.
  • SLOs registers CRUD routes under /api/observability/slos, presumably this allows for other operations or CRUD routes to be created under the observability domain. However, the Synthetics APIs notably do not use the observability domain and instead use the top-level synthetics domain for monitors and params.
@nickpeihl nickpeihl added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Sep 23, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@timductive
Copy link
Member

My preference would be to start standardizing on:
/api/{pluralnoun}

So in this case:
/api/dashboards

GET /api/dashboards should be the route to return the list of dashboards.

This doesn't prevent a future use of:
POST/PUT /api/dashboards/config or
POST/PUT /api/dashboards/default

This is the common usage pattern I've seen in the industry and avoids the redundancy of identifying a singularnoun dashboard in every api request.

@nickpeihl
Copy link
Member Author

nickpeihl commented Oct 1, 2024

This doesn't prevent a future use of:
POST/PUT /api/dashboards/config or
POST/PUT /api/dashboards/default

True. My primary concern with this is a very strict edge case. Consider that we allow users to optionally specify the saved object ID of their dashboard at POST /api/dashboards/<id>. At some point maybe a user creates their dashboard with a saved object ID of default, e.g. curl -XPOST "/api/dashboards/default" ... -d { "attributes": { ... } }.

Later we decide to add a default subpath that allows users to set default dashboards in the dashboards app. The request and response schemas at the /api/dashboards/default endpoint have suddenly changed. The user who naively used default as a saved object ID can no longer perform CRUD operations on their dashboard. They can still use the UI to perform CRUD operations since the UI uses the internal RPC endpoints.

That said, I do like the shorter and REST-like path you propose. But I wonder if that should restrict us from creating any other paths (e.g. config, default) under the dashboards domain? In that case, maybe we would introduce a different endpoint for configuring the Dashboards app (e.g. /api/dashboards-management/[config, default, etc]).

I guess it really comes down to the question, does dashboards in /api/dashboards refer to the Dashboards app or a collection of dashboards?

@Heenawter
Copy link
Contributor

Heenawter commented Oct 1, 2024

Could we not have /api/dashboards as the route to list all dashboards, while keeping /api/dashboards/dashboard/<id> as the route for creating/updating dashboards? So, anything under api/dashboards refers to a collection of dashboards - and then once you are under api/dashboards/dashboard, you are referring to a single dashboard.

To me, this pattern still makes sense and removes the edge case you mentioned @nickpeihl 🤔 But not sure if we want to be that explicit or if it's better to keep everything under the same level.

@nickpeihl
Copy link
Member Author

Could we not have /api/dashboards as the route to list all dashboards, while keeping /api/dashboards/dashboard/<id> as the route for creating/updating dashboards? So, anything under api/dashboards refers to a collection of dashboards - and then once you are under api/dashboards/dashboard, you are referring to a single dashboard.

IMO, this further muddles the question of what is dashboards exactly? If we consider dashboards to be the Dashboards App, then listing the Collection of dashboard items at /api/dashboards conflates the App with the Collection. The CRUD operations at /api/dashboards/dashboard/<id?> implicitly suggests "dashboards" is the App and "dashboard" is the Collection.

@lukeelmers
Copy link
Member

I guess it really comes down to the question, does dashboards in /api/dashboards refer to the Dashboards app or a collection of dashboards?

I would suggest keeping our API path names focused on resources rather than individual applications. Applications are somewhat malleable, but resources generally aren't. This is especially true for platform-owned components. (e.g. what if we start including dashboards inside other applications in the future? would APIs need to change to /api/some-other-app/dashboard?)

A focus on applications also risks us designing our APIs around the org structure, when for users our entire platform API surface area should ideally feel like one product built by one team.

My general sense here is that we probably shouldn't overthink this... simpler is usually better, and changing path names in the future is a low maintenance burden (i.e. we can easily register the same route handler for multiple paths if needed)

cc @kobelb for his input

@kobelb
Copy link
Contributor

kobelb commented Oct 3, 2024

I agree with Luke. We should keep this simple for the time being, and not try to future proof ourselves.

My preference is that we use /api/dashboard (singular) as the path. This breaks from industry-wide convention, but matches the Elasticsearch convention:

Model names within the path should be in singular form, not pluralized, even when the endpoint is capable of returning more than one of the model.

If in the future, we want a HTTP API for dashboard config, this leaves us with two options.

  1. Follow the ES conventions again, and introduce a "dashboard" namespace, which would be prefixed with a _, leading to a /api/_dashboard/config path. We could either continue to use /api/dashboard for the dashboard resource, or also include it in the _dashboard namespace and use /api/_dashboard/dashboard (a bit more awkward IMO).
  2. No longer follow the ES conventions, and introduce a "dashboards" namespace, not prefixed by a _, leading to /api/dashboards/config. We could then continue to use /api/dashboard for the dashboard resource, or use the "dashboards" namespace and use /api/dashboards/dashboard.

A similar approach could be followed if we want to set a default dashboard.

@thomasneirynck thomasneirynck changed the title [Dashboards] Public API URL path [Dashboards][Discuss] Public API URL path Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

No branches or pull requests

7 participants