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

Add support for tag groupings #2310

Closed
lcawl opened this issue Oct 10, 2023 · 11 comments · Fixed by #2961
Closed

Add support for tag groupings #2310

lcawl opened this issue Oct 10, 2023 · 11 comments · Fixed by #2961

Comments

@lcawl
Copy link
Contributor

lcawl commented Oct 10, 2023

🚀 Feature Proposal

Add support for "tags" on each operation in the specification.

Motivation

Per https://spec.openapis.org/oas/latest#operationObject, "Tags can be used for logical grouping of operations by resources or any other qualifier".

Since we have such a long list of Elasticsearch APIs, it would be helpful to group them. For example, we could use the groupings that already exist in the documentation (https://www.elastic.co/guide/en/elasticsearch/reference/master/rest-apis.html) such as machine learning APIs, enrich APIs, Fleet APIs, etc.

At the moment, the tags that are generated are too low-level for our doc purposes (e.g. "async_search.get").

NOTE: OpenAPI specifications also optionally have a global list of the tags with descriptions for each of them per https://spec.openapis.org/oas/v3.0.3#tag-object (and this list can be used to organize the API documentation in Bump.sh). We'll ultimately want that but it could feasibly be added as a overlay via post-processing. IMO the more important action item is to curate the operation-level tags.

Example

It would be nice to have something like @tag that we could add as a modifier, like what exists in typespec.io per #2310 (comment)

@lcawl
Copy link
Contributor Author

lcawl commented Jan 8, 2024

There are now tags in the specifications (e.g. "async_search.get" in https://github.com/elastic/elasticsearch-specification/blob/7e140693f2bbd392b95f54f2cd2b0c0895db2e5d/output/openapi/elasticsearch-serverless-openapi.json#L15), however I think from a docs point of view those values are too low-level. I suspect we'd want "asynch_search", "ml", "cat" tags instead.

@lcawl
Copy link
Contributor Author

lcawl commented Apr 25, 2024

Per https://typespec.io/docs/getting-started/typespec-for-openapi-dev#tags in TypeSpec "Tags can be specified using the @tag decorator on an operation".

@flobernd
Copy link
Member

flobernd commented Aug 1, 2024

As discussed today, it would probably be sufficient to use the existing namespace (e.g. async_search) as the tag. This could be done in the OAPI generator and would not require spec changes.

If required, the @tag functionality could be introduced to the spec at a later point to allow overriding the tag/group without changing the namespace.

@lcawl
Copy link
Contributor Author

lcawl commented Aug 2, 2024

As discussed today, it would probably be sufficient to use the existing namespace (e.g. async_search) as the tag. This could be done in the OAPI generator and would not require spec changes.

That would be great!

@shainaraskas @leemthompo @szabosteve I was playing with tags today and noticed that in our existing docs (https://www.elastic.co/guide/en/elasticsearch/reference/current/search.html) we have a "Search" group of APIs whereas in the specification we have tags like "search_mvt...", "asynch_search", "search", "search_application...", etc. Do you have a preference for how those are grouped in the docs?

In ReDocly they have an extension for grouping tags https://redocly.com/docs/api-reference-docs/specification-extensions/x-tag-groups, so if that's something we think we'll want, I can put it on our ask list for Bump.sh

@lcawl
Copy link
Contributor Author

lcawl commented Aug 2, 2024

Just so I don't forget this after the long week-end, I played with the new support that was added to Bump.sh for tag display names. It works fine in my tests and means that we can optionally add an overlay like this to improve the tag names and descriptions that appear in the published docs:

  - target: '$'
    description: Add displayNames and descriptions to document-level tags
    update:
      tags:
        - name: search
          descsription: >
            Search APIs are used to search and aggregate data stored in Elasticsearch indices and data streams.
            For an overview and related tutorials, check out [The search API](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-your-data.html).
          x-displayName: "Search"

"Search" is probably a poor example, but makes sense for tags like "ml" that we might want to spell out as "Machine learning", etc

@shainaraskas
Copy link
Contributor

shainaraskas commented Aug 6, 2024

based on some of the discussions that stemmed from that meeting, I think we might still have a problem with root - these endpoints don't belong to a namespace, so many operations that target an index or alias (nominally at the root) won't be able to sit in the relevant section in our current plan.

#2758 (comment)

image image

@shainaraskas
Copy link
Contributor

shainaraskas commented Aug 6, 2024

Thinking more about this:

in the case of the root namespace, could we allow for providing alternative tags that align with existing namespaces? e.g. for /{index}/_create/{id} which only gets a create tag in the spec, we could add an override to provide an index tag as well so the grouping is more sensible. technically this is a "document" api, but nesting it with the index apis would maybe be an ok compromise?

while this opens us to the possibility of non-ideal links, this would ensure that we get both accurate categories in the API docs, and links that can always resolve in both client docs and API docs.

let me know if anyone wants to chat synchronously about this ^

edit: term vectors and mvt might be exceptions to always having links that resolve perfectly because they only exist in the root

edit edit: we could also allow for alternative tags that don't align with existing namespaces, but not allow internal linking to it within the spec (i.e. have it fail linting)? as a workaround, people could link to the first endpoint within the group.

@leemthompo
Copy link
Contributor

leemthompo commented Aug 6, 2024

Do you have a preference for how those are grouped in the docs?

The Search API(s) is(are) such a monstrosity and I'm really not up to speed on Bump so I don't know if I have anything intelligent to say about groupings.

From cheap seats, having Search APIs grouped together like we do in the nav on the existing stack page probably makes sense.

@lcawl
Copy link
Contributor Author

lcawl commented Sep 16, 2024

Per #2310 (comment)

... it would probably be sufficient to use the existing namespace (e.g. async_search) as the tag. This could be done in the OAPI generator and would not require spec changes.

I created #2889 which seems to accomplish that goal. If a tag contains a period, it drops everything after the first period. If a tag doesn't contain a period, it just uses it as-is.

If required, the @tag functionality could be introduced to the spec at a later point to allow overriding the tag/group without changing the namespace.

I think this would still be necessary longer term, since things like the document APIs (which are just "create", "delete", "bulk", etc tags/namespaces) could then be overridden to just be "document" for example. It would also be useful for cases like the /_validate/query endpoint, which is currently in the "indices" namespaces but in the docs we group it with the "search" APIs.

@lcawl
Copy link
Contributor Author

lcawl commented Sep 16, 2024

As noted in the original description, once these operation-level tags are cleaned up, the second necessary task is to get all the valid tags into a global (sorted) list. For now I've created #2891 to show what that would look like applied as an overlay but long-term it ought to be automatically generated to avoid getting out of sync. I think it would depend on the existence of @tag functionality mentioned above, so that we could augment the tags with the optional display names, descriptions, and sort the list based on the display names. I've also requested that Bump.sh support x-tag- but it's TBD when/if that would be provided so for now I've prefixed all the "Document - ...." and "Script - ...." and "Search - ...." tags and put them together in the list.

@lcawl
Copy link
Contributor Author

lcawl commented Sep 27, 2024

i've taken a stab at adding a @doc_tag in #2961

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants