[SLO] Auto generating OpenAPI Spec (OAS) using @kbn/config-schema for a sample SLO API

maryam-saeidi commented 2 months ago

🍒 Summary

As mentioned in this email, platform team worked on introducing a way to automatically generate OpenAPI Spec (OAS) using @kbn/config-schema.

Here is their first iteration on the /api/status API. (PR)

In this ticket, we would like to use @kbn/config-schema for one of the SLO APIs and compare the result with the existing OpenAPI Spec to figure out if something is missing and, if not, how much effort is needed for this migration.

Here is the existing list of SLO OpenAPI Specifications. We need to decide which SLO API is a good candidate based on the complexity, to make sure our estimates are realistic.

✅ Acceptance criteria

Auto-generate Open API Spec for one of the complicated SLO APIs and compare it with existing specifications. (Write down any challenges or limitations in this process)
Provide an estimation of how much work it would be to migrate all SLO API specifications.

elasticmachine commented 2 months ago

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

maryam-saeidi commented 1 month ago

In this comment, I will log the findings that I have related to changing a sample validation to using @kbn/config-schema

I've started with Delete /api/observability/slos/{id} first to see how the flow works and I will move to a more complicated API in the next step.

Config schema documentation

https://github.com/elastic/kibana/blob/main/packages/kbn-config-schema/README.md

Implementation

After changing registering a route with @kbn/config-schema validation, and enabling server.oas.enabled: true in kibana.dev.yaml (as mentioned in this PR), I had 2 options to see related OAS specifications:

Using pluginId: http://localhost:5601/api/oas?pluginId=@kbn/slo-plugin
Using pathStartsWith: http://localhost:5601/api/oas?pathStartsWith=/api/observability/slos

Next, I tried to recreate deleteSloOp as specified here.

Findings

The pluginId seemed to have a bug, and we need to specify the package ID instead. (issue)
The Description field was actually the summary field in OAS; here is the PR for fixing this mismatch.
Example is not supported yet. (There is an item for this in this issue)
Config-schema does not support partial for an object. It provides schema.maybe that can be used with every field separately. We don't need to separate partial vs mandatory fields similar to io-ts when we use config-schema. The only case that might be useful to have partial is when we already have a type but in some scenarios we would like to allow passing partial objects, like:

Config-schema support for intersection is a bit verbose: (I've created an improvement ticket for intersection)

const a = schema.object({ a: schema.string() });
const bKeys = { b: schema.string() };
const b = schema.object(bKeys);

// C = A & B
const c = a.extends(bKeys);

simianhacker commented 1 month ago

I see you're converting a schema that's pretty simple, Delete /api/observability/slos/{id}. I think where this becomes really tricky is that the other schemas re-use io-ts schema for most of the bits and pieces which are also used in our models, like the updateSLOParamsSchema. Does that mean we are going to have to convert everything to config-schema?

The issue I have with this effort is that there isn't feature parity yet and I'd hate have to maintain 2 sets of schemas. I love the promise of mostly getting the OpenAPI docs for free by using the code but I think we might need to push back until the replacement library is up to par with the io-ts features we use. The features that come to mind:

Runtime types that are reused to create Typescript types
Type guards using schemas throughout the code
Custom types like the Duration type
Documentation for io-ts with examples

maryam-saeidi commented 1 month ago

@simianhacker Thanks for sharing the list of features that we need to evaluate. Indeed, the delete API was just a starting point for me to see how OAS works in action, but the main goal is to evaluate the gaps that we might have between config-schema and io-ts. I will use the list that you provided as a guide, thanks.

jasonrhodes commented 1 month ago

What would be the best endpoint to choose as the next one, to highlight the gaps as starkly as possible? As Mary said, the goal here isn't to start a migration, but rather to help expose the gaps so we can provide feedback to the platform team re: config-schema limitations. The more we look at this, the less likely it seems we'd want to spend time on converting/refactoring lots of code just for auto-generated OpenAPI specs.

kdelemme commented 1 month ago

The update or find APIs would be very good candidate:

The update API because the indicator type is complex
The find API because the response is used everywhere in the app, and part of its schema is used for runtime validation, e.g. occurrencesBudgetingMethod.is(slo.budgetingMethod)

maryam-saeidi commented 1 month ago

Based on the above discussions, here is a list of items that need to be verified and related investigation: (Related POC)

1. Runtime types that are reused to create Typescript types

It is possible to return type a schema using TypeOf<typeof rt>, For example:

type CreateSLOInputConfigSchema = TypeOf\<typeof createSLOParamsConfigSchema.body>

The challenge comes from supporting type transformation, which will be discussed in the third section, the Duration type.

2.Type guards using schemas throughout the code

At the moment, in the cases that we use Type guard (like the example that Kevin shared, e.g. occurrencesBudgetingMethodSchema.is(slo.budgetingMethod)), we only use the validation part of the is functionality as far as I see. For those, we could use validate function, but if the type does not match, then it throws an error which is not the expected behaviour and we need to maybe have a wrapper around it if we don't want to throw an error.

🗒️ I've created a ticket for adding support for is type checking.

3.Custom types like the Duration type

For custom types, the validation can be done using config schema custom validation. The challenge is about converting string -> Duration and having proper input and output types as defined here.

🗒️ I've created a ticket to add transform functionality to the config-schema.

4. Documentation with examples

Here is schema-config documentation which has lots of good examples. Maybe the only challenge is how to make it easier for developers to find this documentation and ensure it is up to date.

🗒️ I've created a ticket to cover adding extends to the documentation which was missing.

elastic / kibana