From d7b4e6d7396e7683430bdc20ed0a385fb4aa9935 Mon Sep 17 00:00:00 2001 From: Natali Date: Mon, 13 Feb 2023 23:07:44 +0100 Subject: [PATCH 1/6] chore: remove usage for stats endpoint --- .../server/routes/stats/README.md | 2 +- .../server/routes/stats/stats.ts | 58 +++---------------- 2 files changed, 10 insertions(+), 50 deletions(-) diff --git a/src/plugins/usage_collection/server/routes/stats/README.md b/src/plugins/usage_collection/server/routes/stats/README.md index 09dabefbab44a..48e9aeb86c3a6 100644 --- a/src/plugins/usage_collection/server/routes/stats/README.md +++ b/src/plugins/usage_collection/server/routes/stats/README.md @@ -10,7 +10,7 @@ However, the information detailed above can be extended, with the combination of |:----------------|:-------------:|:------------| |`extended`|`false`|When `true`, it adds `clusterUuid` and `usage`. The latter contains the information reported by all the Usage Collectors registered in the Kibana server. It may throw `503 Stats not ready` if any of the collectors is not fully initialized yet.| |`legacy`|`false`|By default, when `extended=true`, the key names of the data in `usage` are transformed into API-friendlier `snake_case` format (i.e.: `clusterUuid` is transformed to `cluster_uuid`). When this parameter is `true`, the data is returned as-is.| -|`exclude_usage`|`false`|When `true`, and `extended=true`, it will report `clusterUuid` but no `usage`.| +|`exclude_usage`|`true`|When `true`, and `extended=true`, it will report `clusterUuid` but no `usage`.| ## Known use cases diff --git a/src/plugins/usage_collection/server/routes/stats/stats.ts b/src/plugins/usage_collection/server/routes/stats/stats.ts index 9db7f5a8638af..a32cb605b6261 100644 --- a/src/plugins/usage_collection/server/routes/stats/stats.ts +++ b/src/plugins/usage_collection/server/routes/stats/stats.ts @@ -8,7 +8,6 @@ import { schema } from '@kbn/config-schema'; import { i18n } from '@kbn/i18n'; -import defaultsDeep from 'lodash/defaultsDeep'; import { firstValueFrom, Observable } from 'rxjs'; import { @@ -77,7 +76,7 @@ export function registerStatsRoute({ extended: schema.oneOf([schema.literal(''), schema.boolean()], { defaultValue: false }), legacy: schema.oneOf([schema.literal(''), schema.boolean()], { defaultValue: false }), exclude_usage: schema.oneOf([schema.literal(''), schema.boolean()], { - defaultValue: false, + defaultValue: true, }), }), }, @@ -85,61 +84,22 @@ export function registerStatsRoute({ async (context, req, res) => { const isExtended = req.query.extended === '' || req.query.extended; const isLegacy = req.query.legacy === '' || req.query.legacy; - const shouldGetUsage = req.query.exclude_usage === false; let extended; if (isExtended) { const core = await context.core; const { asCurrentUser } = core.elasticsearch.client; - const savedObjectsClient = core.savedObjects.client; - const [usage, clusterUuid] = await Promise.all([ - shouldGetUsage - ? getUsage(asCurrentUser, savedObjectsClient) - : Promise.resolve({}), - getClusterUuid(asCurrentUser), - ]); + const clusterUuid = await getClusterUuid(asCurrentUser); - let modifiedUsage = usage; - if (isLegacy) { - // In an effort to make telemetry more easily augmented, we need to ensure - // we can passthrough the data without every part of the process needing - // to know about the change; however, to support legacy use cases where this - // wasn't true, we need to be backwards compatible with how the legacy data - // looked and support those use cases here. - modifiedUsage = Object.keys(usage).reduce((accum, usageKey) => { - if (usageKey === 'kibana') { - accum = { - ...accum, - ...usage[usageKey], - }; - } else if (usageKey === 'reporting') { - accum = { - ...accum, - xpack: { - ...accum.xpack, - reporting: usage[usageKey], - }, - }; - } else { - // I don't think we need to it this for the above conditions, but do it for most as it will - // match the behavior done in monitoring/bulk_uploader - defaultsDeep(accum, { [usageKey]: usage[usageKey] }); - } - - return accum; - }, {} as UsageObject); - - extended = { - usage: modifiedUsage, - clusterUuid, - }; - } else { - extended = collectorSet.toApiFieldNames({ - usage: modifiedUsage, + // In an effort to make telemetry more easily augmented, we need to ensure + // we can passthrough the data without every part of the process needing + // to know about the change; however, to support legacy use cases where this + // wasn't true, we need to be backwards compatible with how the legacy data + // looked and support those use cases here. + extended = isLegacy ? { clusterUuid, } : collectorSet.toApiFieldNames({ clusterUuid, - }); - } + }); } // Guaranteed to resolve immediately due to replay effect on getOpsMetrics$ From fa96fbd7b7c158c85e4b6b97ab0d82ed2497a17b Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 14 Feb 2023 01:50:39 +0000 Subject: [PATCH 2/6] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- src/plugins/usage_collection/server/routes/stats/stats.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/plugins/usage_collection/server/routes/stats/stats.ts b/src/plugins/usage_collection/server/routes/stats/stats.ts index a32cb605b6261..149ba8497fca9 100644 --- a/src/plugins/usage_collection/server/routes/stats/stats.ts +++ b/src/plugins/usage_collection/server/routes/stats/stats.ts @@ -97,9 +97,11 @@ export function registerStatsRoute({ // to know about the change; however, to support legacy use cases where this // wasn't true, we need to be backwards compatible with how the legacy data // looked and support those use cases here. - extended = isLegacy ? { clusterUuid, } : collectorSet.toApiFieldNames({ - clusterUuid, - }); + extended = isLegacy + ? { clusterUuid } + : collectorSet.toApiFieldNames({ + clusterUuid, + }); } // Guaranteed to resolve immediately due to replay effect on getOpsMetrics$ From 6a0f180cb0bf237d3b844735823e2feaa3b36206 Mon Sep 17 00:00:00 2001 From: Natali Date: Fri, 17 Feb 2023 23:15:51 +0100 Subject: [PATCH 3/6] fix: usage is always present but is empty object; update to few tests --- .../usage/integration_tests/usage.test.ts | 22 +------------ .../server/routes/stats/stats.ts | 33 +++++++++---------- test/api_integration/apis/stats/stats.js | 3 ++ 3 files changed, 20 insertions(+), 38 deletions(-) diff --git a/src/plugins/files/server/usage/integration_tests/usage.test.ts b/src/plugins/files/server/usage/integration_tests/usage.test.ts index 0bccb869c822e..199313ca42a4b 100644 --- a/src/plugins/files/server/usage/integration_tests/usage.test.ts +++ b/src/plugins/files/server/usage/integration_tests/usage.test.ts @@ -46,26 +46,6 @@ describe('Files usage telemetry', () => { const { body } = await request.get(root, `/api/stats?extended=true&legacy=true`); - expect(body.usage.files).toMatchInlineSnapshot(` - Object { - "countByExtension": Array [ - Object { - "count": 3, - "extension": "png", - }, - ], - "countByStatus": Object { - "AWAITING_UPLOAD": 2, - "READY": 1, - }, - "storage": Object { - "esFixedSizeIndex": Object { - "available": 53687091187, - "capacity": 53687091200, - "used": 13, - }, - }, - } - `); + expect(body.usage).to.eql({}); }); }); diff --git a/src/plugins/usage_collection/server/routes/stats/stats.ts b/src/plugins/usage_collection/server/routes/stats/stats.ts index a32cb605b6261..e7a04e34fe10b 100644 --- a/src/plugins/usage_collection/server/routes/stats/stats.ts +++ b/src/plugins/usage_collection/server/routes/stats/stats.ts @@ -14,7 +14,6 @@ import { ElasticsearchClient, IRouter, type MetricsServiceSetup, - SavedObjectsClientContract, ServiceStatus, ServiceStatusLevels, } from '@kbn/core/server'; @@ -50,14 +49,6 @@ export function registerStatsRoute({ metrics: MetricsServiceSetup; overallStatus$: Observable; }) { - const getUsage = async ( - esClient: ElasticsearchClient, - savedObjectsClient: SavedObjectsClientContract - ): Promise => { - const usage = await collectorSet.bulkFetchUsage(esClient, savedObjectsClient); - return collectorSet.toObject(usage); - }; - const getClusterUuid = async (asCurrentUser: ElasticsearchClient): Promise => { const body = await asCurrentUser.info({ filter_path: 'cluster_uuid' }); const { cluster_uuid: uuid } = body; @@ -90,16 +81,24 @@ export function registerStatsRoute({ const core = await context.core; const { asCurrentUser } = core.elasticsearch.client; + const usage = {} as UsageObject; const clusterUuid = await getClusterUuid(asCurrentUser); - - // In an effort to make telemetry more easily augmented, we need to ensure - // we can passthrough the data without every part of the process needing - // to know about the change; however, to support legacy use cases where this - // wasn't true, we need to be backwards compatible with how the legacy data - // looked and support those use cases here. - extended = isLegacy ? { clusterUuid, } : collectorSet.toApiFieldNames({ + if (isLegacy) { + // In an effort to make telemetry more easily augmented, we need to ensure + // we can passthrough the data without every part of the process needing + // to know about the change; however, to support legacy use cases where this + // wasn't true, we need to be backwards compatible with how the legacy data + // looked and support those use cases here. + extended = { + usage, + clusterUuid, + }; + } else { + extended = collectorSet.toApiFieldNames({ + usage, clusterUuid, - }); + }); + } } // Guaranteed to resolve immediately due to replay effect on getOpsMetrics$ diff --git a/test/api_integration/apis/stats/stats.js b/test/api_integration/apis/stats/stats.js index a95204b5fff4a..3d69a949a4db3 100644 --- a/test/api_integration/apis/stats/stats.js +++ b/test/api_integration/apis/stats/stats.js @@ -91,6 +91,7 @@ export default function ({ getService }) { .then(({ body }) => { expect(body.cluster_uuid).to.be.a('string'); expect(body.usage).to.be.an('object'); // no usage collectors have been registered so usage is an empty object + expect(body.usage).to.eql({}); assertStatsAndMetrics(body); }); }); @@ -103,6 +104,7 @@ export default function ({ getService }) { .then(({ body }) => { expect(body.cluster_uuid).to.be.a('string'); expect(body.usage).to.be.an('object'); + expect(body.usage).to.eql({}); assertStatsAndMetrics(body); }); }); @@ -116,6 +118,7 @@ export default function ({ getService }) { .then(({ body }) => { expect(body.clusterUuid).to.be.a('string'); expect(body.usage).to.be.an('object'); // no usage collectors have been registered so usage is an empty object + expect(body.usage).to.eql({}); assertStatsAndMetrics(body, true); }); }); From 611b99a3f8a69dff6cef6de6d7411f8ff2d5c819 Mon Sep 17 00:00:00 2001 From: Natali Date: Tue, 21 Feb 2023 22:46:09 +0100 Subject: [PATCH 4/6] chore: update test based on suggestion --- .../usage/integration_tests/usage.test.ts | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/plugins/files/server/usage/integration_tests/usage.test.ts b/src/plugins/files/server/usage/integration_tests/usage.test.ts index 199313ca42a4b..02cc7dfee55fa 100644 --- a/src/plugins/files/server/usage/integration_tests/usage.test.ts +++ b/src/plugins/files/server/usage/integration_tests/usage.test.ts @@ -44,8 +44,30 @@ describe('Files usage telemetry', () => { request.post(root, `/api/files/shares/${fileKind}/${file3.id}`).send({}).expect(200), ]); - const { body } = await request.get(root, `/api/stats?extended=true&legacy=true`); + const { body } = await request + .post(root, '/api/telemetry/v2/clusters/_stats') + .send({ unencrypted: true }); - expect(body.usage).to.eql({}); + expect(body[0].stats.stack_stats.kibana.plugins.files).toMatchInlineSnapshot(` + Object { + "countByExtension": Array [ + Object { + "count": 3, + "extension": "png", + }, + ], + "countByStatus": Object { + "AWAITING_UPLOAD": 2, + "READY": 1, + }, + "storage": Object { + "esFixedSizeIndex": Object { + "available": 53687091187, + "capacity": 53687091200, + "used": 13, + }, + }, + } + `); }); }); From b589ae8f248d24c36c220d96d785616d0b01f9f0 Mon Sep 17 00:00:00 2001 From: Natali Date: Mon, 27 Feb 2023 11:26:23 +0100 Subject: [PATCH 5/6] Update src/plugins/usage_collection/server/routes/stats/README.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alejandro Fernández Haro --- src/plugins/usage_collection/server/routes/stats/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/usage_collection/server/routes/stats/README.md b/src/plugins/usage_collection/server/routes/stats/README.md index 48e9aeb86c3a6..4d96ddad281b3 100644 --- a/src/plugins/usage_collection/server/routes/stats/README.md +++ b/src/plugins/usage_collection/server/routes/stats/README.md @@ -10,7 +10,7 @@ However, the information detailed above can be extended, with the combination of |:----------------|:-------------:|:------------| |`extended`|`false`|When `true`, it adds `clusterUuid` and `usage`. The latter contains the information reported by all the Usage Collectors registered in the Kibana server. It may throw `503 Stats not ready` if any of the collectors is not fully initialized yet.| |`legacy`|`false`|By default, when `extended=true`, the key names of the data in `usage` are transformed into API-friendlier `snake_case` format (i.e.: `clusterUuid` is transformed to `cluster_uuid`). When this parameter is `true`, the data is returned as-is.| -|`exclude_usage`|`true`|When `true`, and `extended=true`, it will report `clusterUuid` but no `usage`.| +|`exclude_usage`|`true`| Deprecated. Only kept for backward-compatibility. Setting this to `false` has no effect. Usage is always excluded. | ## Known use cases From e711325e8d8b93ad5f332d43b6f1ac9d1bc3386a Mon Sep 17 00:00:00 2001 From: Natali Date: Mon, 27 Feb 2023 11:26:51 +0100 Subject: [PATCH 6/6] Update src/plugins/usage_collection/server/routes/stats/README.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alejandro Fernández Haro --- src/plugins/usage_collection/server/routes/stats/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/usage_collection/server/routes/stats/README.md b/src/plugins/usage_collection/server/routes/stats/README.md index 4d96ddad281b3..48ebda9cecb05 100644 --- a/src/plugins/usage_collection/server/routes/stats/README.md +++ b/src/plugins/usage_collection/server/routes/stats/README.md @@ -8,8 +8,8 @@ However, the information detailed above can be extended, with the combination of | Query Parameter | Default value | Description | |:----------------|:-------------:|:------------| -|`extended`|`false`|When `true`, it adds `clusterUuid` and `usage`. The latter contains the information reported by all the Usage Collectors registered in the Kibana server. It may throw `503 Stats not ready` if any of the collectors is not fully initialized yet.| -|`legacy`|`false`|By default, when `extended=true`, the key names of the data in `usage` are transformed into API-friendlier `snake_case` format (i.e.: `clusterUuid` is transformed to `cluster_uuid`). When this parameter is `true`, the data is returned as-is.| +|`extended`|`false`|When `true`, it adds `clusterUuid`.| +|`legacy`|`false`|By default, when `extended=true`, the key names are transformed into API-friendlier `snake_case` format (i.e.: `clusterUuid` is transformed to `cluster_uuid`). When this parameter is `true`, the data is returned as-is.| |`exclude_usage`|`true`| Deprecated. Only kept for backward-compatibility. Setting this to `false` has no effect. Usage is always excluded. | ## Known use cases