Implement HTTP API versioned types in usage collection plugin #152875

TinaHeiligers · 2023-03-08T00:20:57Z

Similar to 152111 prepares usage collection plugin types for versioning

In this PR we ensure that we are adhering to the goals of versioned HTTP APIs to:

Not send SO attributes directly over the wire (the usage collection plugin doesn't consume the saved objects HTTP APIs)
Have a separate set of interfaces that can be referenced publically and maintained by the server

The types for the following HTTP API routes have been versioned:

stats
ui_counters
selected usage_counters types requiring changes

Stats API

The stats route reports (along with other metrics), a subset of cores' OpsMetrics. Since usage isn't included in the response anymore (ref #151082), we have a few options to allow us to detect changes to these metrics:

Move the entire stats route to core
Keep the types for metrics generic and rely on some sort of validation elsewhere
Directly import the types for metrics from packages/core/metrics/core-metrics-server/index.ts and rely on validation elsewhere
Create duplicates of these types and explicitly declare, with the current types, the full response payload.
something else?

This PR takes the 4th approach: Use duplicates with explicitly declaring the full response body.

How to test this:

functional tests will ensure that the implementations of these APIs haven't changed
Run kibana with es as normal and navigate to the stats route. It should remain working with and without extended and legacy queries:

{basepath}/api/stats?extended&legacy
{basepath}/api/stats?extended
{basepath}/api/stats

src/plugins/usage_collection/common/types/stats/v1.ts

jloleysens

Did a quick review, let me know if that helps with the failed test!

jloleysens · 2023-03-08T15:11:55Z

src/plugins/usage_collection/server/routes/stats/stats.ts

+      const body: Stats.v1.StatsHTTPBodyTyped = {
+        ...kibanaStats,
+        ...extended,
+      };


jloleysens · 2023-03-08T15:13:58Z

src/plugins/usage_collection/server/routes/stats/stats.ts

+        const extendedClusterUuid = isLegacy ? { clusterUuid } : { cluster_uuid: clusterUuid };
+        extended = {
+          usage: {},
+          extendedClusterUuid,


Do we need to spread this?

Suggested change

extendedClusterUuid,

...extendedClusterUuid,

ah, that makes sense! I'll give it a go.

jloleysens · 2023-03-08T15:15:51Z

src/plugins/usage_collection/server/routes/stats/stats.ts


      let extended;
      if (isExtended) {
        const core = await context.core;
        const { asCurrentUser } = core.elasticsearch.client;
+        // as of https://github.com/elastic/kibana/pull/151082, usage will always be an empty object.


Thanks for adding this comment!

TinaHeiligers · 2023-03-08T16:37:58Z

src/plugins/usage_collection/common/types/index.ts

+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+


Exporting the types as aliased to the unversioned type definitions allows us to use:
Stats.v1.StatsHTTPBodyTyped for example

TinaHeiligers

Self review with comments to reviewers

TinaHeiligers · 2023-03-08T16:41:33Z

src/plugins/usage_collection/common/types/stats/core_metrics.ts

+ */
+
+/**
+ * The types are exact duplicates of the ops metrics types declared in core needed for reporting in the stats api.


We're duplicating the types from core so that we'll detect any changes to the original metrics.

TinaHeiligers · 2023-03-08T16:42:36Z

src/plugins/usage_collection/common/types/stats/v1.ts

+  };
+}
+/** Stats response body */
+export type StatsHTTPBodyTyped = LastOpsMetrics | KibanaStats | ExtendedStats;


explicitly declare (with appropriate types) the full response body from the stats api

TinaHeiligers · 2023-03-08T16:45:58Z

src/plugins/usage_collection/public/services/create_reporter.ts

-
-      if (response.status !== 'ok') {
+      const okStatus: UiCounters.v1.UiCountersResponseOk = response.status;
+      if (response.status !== okStatus) {


The responses are hard-coded, depending on whether fetch throws or not, so we declare separate types for each outcome.

TinaHeiligers · 2023-03-08T16:46:13Z

src/plugins/usage_collection/server/routes/stats/stats.ts

 const SNAPSHOT_REGEX = /-snapshot/i;

-interface UsageObject {


moved to v1

TinaHeiligers · 2023-03-08T16:47:23Z

src/plugins/usage_collection/server/routes/ui_counters.ts

@@ -26,16 +27,22 @@ export function registerUiCountersRoute(
      },
    },
    async (context, req, res) => {
-      const { report } = req.body;
+      const requestBody: UiCounters.v1.UiCountersRequestBody = req.body;


The request needs to be fully versioned to accommodate future changes.

TinaHeiligers · 2023-03-08T16:48:01Z

src/plugins/usage_collection/server/routes/ui_counters.ts

+        const bodyOk: UiCounters.v1.UiCountersResponseOk = {
+          status: 'ok',
+        };
+        return res.ok({ body: bodyOk });


Explicitly type the hard-coded response.

TinaHeiligers · 2023-03-08T16:49:02Z

src/plugins/usage_collection/server/usage_counters/usage_counter.test.ts

@@ -5,13 +5,14 @@
 * in compliance with, at your election, the Elastic License 2.0 or the Server
 * Side Public License, v 1.
 */
-import { UsageCounter, CounterMetric } from './usage_counter';
+import { UsageCounter } from './usage_counter';
+import type { UsageCounters } from '../../common/types';


The type and implementation having the same name might not be appropriate. I'm open to renaming the type, if needed.

TinaHeiligers · 2023-03-08T18:18:58Z

@elasticmachine merge upstream

TinaHeiligers · 2023-03-08T18:20:59Z

@elasticmachine merge upstream

kibana-ci · 2023-03-08T19:29:05Z

💚 Build Succeeded

Buildkite Build
Commit: b6a9593

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`usageCollection`	15	16	+1

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`usageCollection`	1	2	+1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`usageCollection`	5.0KB	5.0KB	+8.0B

Unknown metric groups

API count

id	before	after	diff
`usageCollection`	58	55	-3

ESLint disabled line counts

id	before	after	diff
`securitySolution`	431	433	+2

Total ESLint disabled count

id	before	after	diff
`securitySolution`	508	510	+2

History

💔 Build #112807 failed 5daf48b
💔 Build #112587 failed 01e2d9e

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

elasticmachine · 2023-03-08T20:16:23Z

Pinging @elastic/kibana-core (Team:Core)

jloleysens

Great work @TinaHeiligers !

…c#152875) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

TinaHeiligers added 3 commits March 7, 2023 16:58

Implement HTTP API versioned types in stats route

4d8ed70

Version ui_counters http api types

069d8f7

version usage counters route types

01e2d9e

TinaHeiligers added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.8.0 labels Mar 8, 2023

TinaHeiligers mentioned this pull request Mar 8, 2023

Prepares usage-collection plugin for versioned HTTP APIs #152770

Closed

9 tasks

TinaHeiligers commented Mar 8, 2023

View reviewed changes

src/plugins/usage_collection/common/types/stats/v1.ts Show resolved Hide resolved

TinaHeiligers requested a review from jloleysens March 8, 2023 14:37

jloleysens reviewed Mar 8, 2023

View reviewed changes

spread extended additions

5daf48b

TinaHeiligers commented Mar 8, 2023

View reviewed changes

kibanamachine added 2 commits March 8, 2023 13:21

Merge branch 'main' into versioned-apis-usage-collection

b805d64

Merge branch 'main' into versioned-apis-usage-collection

b6a9593

TinaHeiligers marked this pull request as ready for review March 8, 2023 20:16

TinaHeiligers requested a review from a team as a code owner March 8, 2023 20:16

jloleysens self-requested a review March 9, 2023 09:48

jloleysens approved these changes Mar 9, 2023

View reviewed changes

TinaHeiligers merged commit 8a9789b into elastic:main Mar 9, 2023

TinaHeiligers deleted the versioned-apis-usage-collection branch March 9, 2023 16:20

bmorelli25 pushed a commit to bmorelli25/kibana that referenced this pull request Mar 10, 2023

Implement HTTP API versioned types in usage collection plugin (elasti…

1a93930

…c#152875) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement HTTP API versioned types in usage collection plugin #152875

Implement HTTP API versioned types in usage collection plugin #152875

TinaHeiligers commented Mar 8, 2023 •

edited

Loading

jloleysens left a comment

jloleysens Mar 8, 2023

jloleysens Mar 8, 2023

TinaHeiligers Mar 8, 2023

jloleysens Mar 8, 2023

TinaHeiligers Mar 8, 2023

TinaHeiligers left a comment

TinaHeiligers Mar 8, 2023

TinaHeiligers Mar 8, 2023

TinaHeiligers Mar 8, 2023

TinaHeiligers Mar 8, 2023

TinaHeiligers Mar 8, 2023

TinaHeiligers Mar 8, 2023

TinaHeiligers Mar 8, 2023

TinaHeiligers commented Mar 8, 2023

TinaHeiligers commented Mar 8, 2023

kibana-ci commented Mar 8, 2023

API count

ESLint disabled line counts

Total ESLint disabled count

elasticmachine commented Mar 8, 2023

jloleysens left a comment

Implement HTTP API versioned types in usage collection plugin #152875

Implement HTTP API versioned types in usage collection plugin #152875

Conversation

TinaHeiligers commented Mar 8, 2023 • edited Loading

Stats API

How to test this:

jloleysens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TinaHeiligers commented Mar 8, 2023

TinaHeiligers commented Mar 8, 2023

kibana-ci commented Mar 8, 2023

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Public APIs missing exports

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

elasticmachine commented Mar 8, 2023

jloleysens left a comment

Choose a reason for hiding this comment

TinaHeiligers commented Mar 8, 2023 •

edited

Loading