[Discuss] API naming convention

[mshustov]

The current Kibana style guide enforces API format compatible with elasticsearch API:
https://github.com/elastic/kibana/blob/master/STYLEGUIDE.md#snake_case

Kibana uses snake_case for the entire API, just like Elasticsearch. All urls, paths, query string parameters, values, and bodies should be snake_case formatted.

The reality that:

• it's not always possible to follow this convention. For example, when passing around user input:

GET  /api/kibana/settings  
settings: {
  mySetting: { <—— registered by a plugin, Kibana shouldn't change the value
    userValue: ....
  },
}

• it's hard to enforce consistency across the Kibana. For example, a plugin provides data to other plugins via contract and API. As a result, the same data set has a different shape, depending on the way it's consumed. We have this problem with saved objects API [Core][WIP] SavedObjects v2 #40380

// pluginB
const data = pluginA.get();
data.someProperty.name

// or plugin B
const data = await fetch('/api/pluginA');
data.some_property.name

• It enforces data transformation between the client and server sides. As a result, client and server sides have to adopt the shape of data for both environments. It's not that bad, but all consumers have to adapt their code not to handle elasticsearch API as a source of truth anymore.

licensing.features.my_feature // received from elasticsearch
licensing.features.myFeature // across the whole Kibana
licensing.features.my_feature // when plugin interacts with elasticsearch directly

Open questions

• Do we still want to enforce elasticsearch API format?
• How to define when a plugin has to follow the convention?
• Should we follow the convention only in Kibana public API? How to define what set of API is public?

[mshustov]

@elastic/kibana-platform @epixa

[pgayvallet]

Being consistent in public API is the whole stack makes sense imho. However,

URLs

I don't see any difficulties in keeping our urls snake case, as this is something that does not depends on any data.
An exception would be an url like /endpoint/{id}. even if id is, say, an uuid, we are already breaking the convention I guess?

Query params and body payload

This is where the difficulty is imho, as our API conventions kinda conflict with our JS naming conventions. I'm not even speaking about user input here, but just plain, static data structure.

Let say we have the following request that respect the API naming conventions:

POST  /api/some_endpoint
{
  some_property: 'someValue',
}

which would results in the following type:

type myDataStructure = {
     some_property: string;
}

is this was not constrained by the API convention, the type would probably have been

type myDataStructure = {
     someProperty: string;
}

However we are in JS. There is not such distinction as serializable vs not-serializable types. We don't have the tooling proper typed languages can offer such as field anotation, type converters and more. By default, our JS data structure is what got serialized.

So what are our options ? From what I see:

• Enforce our js naming convention to be snake case instead of camel case, on the whole platform.

• dissociate transfert data structure (DTO) from actual model, and convert everything to DTO that respect the naming convention when sending/receiving from/to the server/client (heavy boilerplate as every endpoint needs to creates and use DTOs)

• Add some of core data adapter that does the same thing than last point, but auto-magically. However there is no typescript utility to do something like MyDtoType = ToSnakeCase<MyType>, so the TS types will not be properly linked.

• Actually decide that this API convention is more a best effort

• Other ?

[joshdover]

A couple of thoughts:

• Moving to a snake_case convention in JS code seems like a non-starter to me.
  • Virtually the entire ecosystem uses camelCase. If we used snake_case in our 1st-party code, anywhere we import and use library code, we'd have a jumbled unreadable mess.
• Writing DTOs by hand just to get the naming convention consistent seems like a terrible ROI to me.

There are other API niceties we'd like to add features for, at least for public APIs. For instance: OpenAPI spec, breaking change management / detection, etc. We could add an auto-magic renaming layer for public API routes that leverage these features. This automatic renaming could include a feature that allows you define which keys of the schema should and should not be renamed.

If we wanted automatic TypeScript support, we could leverage a TypeScript compiler plugin / transformer that would be able to generate the snake_case and camelCase versions of the interface.

All that said, building the tooling to make all this work, just to avoid the boilerplate of writing DTOs, is significant.

---

The complexity here stems from the mismatch between Elasticsearch's conventions and the JavaScript ecosystem's conventions. I'm like to push back on the assumption that our APIs need to be consistent with Elasticsearch. How valuable is this in the first place?

The primary instance where I would understand Kibana wanting to match Elasticsearch is APIs where part of the request body is actually an Elasticsearch query or filter. Mixing camelCase and snake_case in these requests would be awkward. However, taking a cursory glance at our currently available public REST APIs, none of them appear to expose raw Elasticsearch API interfaces in this way.

This begs the question: do we even plan to have public APIs that give raw access to Elasticsearch features? If not, it seems removing this snake_case API convention from Kibana would be more valuable than keeping it and building tooling to support it.

[pgayvallet]

Moving to a snake_case convention in JS code seems like a non-starter to me

+1

Writing DTOs by hand just to get the naming convention consistent seems like a terrible ROI to me.

+1

This begs the question: do we even plan to have public APIs that give raw access to Elasticsearch features? If not, it seems removing this snake_case API convention from Kibana would be more valuable than keeping it and building tooling to support it.

+100.

[rudolf]

Assuming we keep the API naming convention, I think the best solution is to honour the API convention above the JS naming convention. Although we can introduce renaming layers and typescript support (Saved Objects uses a type-safe renaming function) it still introduces complexity by introducing subtle differences in property names, network requests and the data that's persisted in ES. I'd rather have inconsistent property names than have to wonder whether something is snake case or camel case in each of the different contexts.

I also don't see the value in using the same API conventions as ES other than consistency for consistency's sake. Although consistency is more beautiful, I don't think it would have any impact on API consumers.

[mshustov]

However, taking a cursory glance at our currently available public REST APIs, none of them appear to expose raw Elasticsearch API interfaces in this way.

Is it a full list of Kibana REST API?

Btw there can see inconsistency even within a single endpoint:

  "updated_at": "2019-07-23T00:11:07.059Z",
  "version": "WzQ0LDFd",
  "attributes": {
    "title": "[Flights] Global Flight Dashboard",
    "hits": 0,
    "description": "Analyze mock flight data for ES-Air, Logstash Airways, Kibana Airlines and JetBeats",
    "panelsJSON": ....
    "optionsJSON": "{\"hidePanelTitles\":false,\"useMargins\":true}",
    "version": 1,
    "timeRestore": true,
    "timeTo": "now",
    "timeFrom": "now-24h",
    "refreshInterval": {
      "display": "15 minutes",
      "pause": false,
      "section": 2,
      "value": 900000
    },
    "kibanaSavedObjectMeta": {...
  }

[epixa]

The role APIs accept data for both Kibana and Elasticsearch roles, so it essentially just proxies some portion of the request body to Elasticsearch: https://www.elastic.co/guide/en/kibana/current/role-management-api-put.html

[joshdover]

Saved Objects uses a type-safe renaming function

@rudolf can you point me to this?

[rudolf]

kibana/src/core/public/saved_objects/saved_objects_client.ts

Lines 476 to 492 in bd63596

	/**
	* Returns a new object with the own properties of `obj`, but the
	* keys renamed according to the `keysMap`.
	*
	* @param keysMap - a map of the form `{oldKey: newKey}`
	* @param obj - the object whose own properties will be renamed
	*/
	const renameKeys = <T extends Record<string, any>, U extends Record<string, any>>(
	keysMap: Record<keyof T, keyof U>,
	obj: Record<string, any>
	) =>
	Object.keys(obj).reduce((acc, key) => {
	return {
	...acc,
	...{ [keysMap[key] || key]: obj[key] },
	};
	}, {});

An example of where we use it:

kibana/src/core/public/saved_objects/saved_objects_client.ts

Lines 307 to 320 in bd63596

	const renameMap = {
	defaultSearchOperator: 'default_search_operator',
	fields: 'fields',
	hasReference: 'has_reference',
	page: 'page',
	perPage: 'per_page',
	search: 'search',
	searchFields: 'search_fields',
	sortField: 'sort_field',
	type: 'type',
	filter: 'filter',
	};
	const renamedQuery = renameKeys<SavedObjectsFindOptions, any>(renameMap, options);

[joshdover]

Current plan:

• Keep public APIs as-is for now
• New public APIs should respect snake_case conventions on a best effort basis
• JavaScript code consuming this data should not do any conversion to camelCase
• When we introduce OpenAPI and work on stabilizing Kibana's public APIs, we will converge on a convention and have tooling to generate types and do data conversion.