From 4ead66f17dcc2cd91294e2dc4e070badcfc19105 Mon Sep 17 00:00:00 2001 From: Tao liu Date: Tue, 5 Apr 2022 18:02:09 +0000 Subject: [PATCH 1/5] fix search usage telemetry Signed-off-by: Tao liu --- config/opensearch_dashboards.yml | 6 +- src/plugins/data/config.ts | 3 + .../data/server/search/collectors/usage.ts | 73 +++++++++++-------- .../data/server/search/search_service.ts | 2 +- 4 files changed, 51 insertions(+), 33 deletions(-) diff --git a/config/opensearch_dashboards.yml b/config/opensearch_dashboards.yml index 26c11904fa9a..a087cf445130 100644 --- a/config/opensearch_dashboards.yml +++ b/config/opensearch_dashboards.yml @@ -164,4 +164,8 @@ # Set the value of this setting to true to capture region blocked warnings and errors # for your map rendering services. -# map.showRegionBlockedWarning: false \ No newline at end of file +# map.showRegionBlockedWarning: false + +# Set the value of this setting to false to suppress search usage telemetry +# for reducing the load of OpenSearch cluster. +data.searchUsageTelemetry.enabled: false \ No newline at end of file diff --git a/src/plugins/data/config.ts b/src/plugins/data/config.ts index a45be57c60b1..1f5bdda9e5c4 100644 --- a/src/plugins/data/config.ts +++ b/src/plugins/data/config.ts @@ -49,6 +49,9 @@ export const configSchema = schema.object({ }), }), }), + searchUsageTelemetry: schema.object({ + enabled: schema.boolean({ defaultValue: false }), + }), }); export type ConfigSchema = TypeOf; diff --git a/src/plugins/data/server/search/collectors/usage.ts b/src/plugins/data/server/search/collectors/usage.ts index 03008250a0bf..ee3b67b926bb 100644 --- a/src/plugins/data/server/search/collectors/usage.ts +++ b/src/plugins/data/server/search/collectors/usage.ts @@ -28,8 +28,10 @@ * under the License. */ -import { CoreSetup } from 'opensearch-dashboards/server'; +import { CoreSetup, PluginInitializerContext } from 'opensearch-dashboards/server'; +import { first } from 'rxjs/operators'; import { Usage } from './register'; +import { ConfigSchema } from '../../../config'; const SAVED_OBJECT_ID = 'search-telemetry'; @@ -38,44 +40,53 @@ export interface SearchUsage { trackSuccess(duration: number): Promise; } -export function usageProvider(core: CoreSetup): SearchUsage { +export function usageProvider( + core: CoreSetup, + initializerContext: PluginInitializerContext +): SearchUsage { const getTracker = (eventType: keyof Usage) => { return async (duration?: number) => { - const repository = await core - .getStartServices() - .then(([coreStart]) => coreStart.savedObjects.createInternalRepository()); + const config = await initializerContext.config + .create() + .pipe(first()) + .toPromise(); + if (config.searchUsageTelemetry.enabled) { + const repository = await core + .getStartServices() + .then(([coreStart]) => coreStart.savedObjects.createInternalRepository()); - let attributes: Usage; - let doesSavedObjectExist: boolean = true; + let attributes: Usage; + let doesSavedObjectExist: boolean = true; - try { - const response = await repository.get(SAVED_OBJECT_ID, SAVED_OBJECT_ID); - attributes = response.attributes; - } catch (e) { - doesSavedObjectExist = false; - attributes = { - successCount: 0, - errorCount: 0, - averageDuration: 0, - }; - } + try { + const response = await repository.get(SAVED_OBJECT_ID, SAVED_OBJECT_ID); + attributes = response.attributes; + } catch (e) { + doesSavedObjectExist = false; + attributes = { + successCount: 0, + errorCount: 0, + averageDuration: 0, + }; + } - attributes[eventType]++; + attributes[eventType]++; - // Only track the average duration for successful requests - if (eventType === 'successCount') { - attributes.averageDuration = - ((duration ?? 0) + (attributes.averageDuration ?? 0)) / (attributes.successCount ?? 1); - } + // Only track the average duration for successful requests + if (eventType === 'successCount') { + attributes.averageDuration = + ((duration ?? 0) + (attributes.averageDuration ?? 0)) / (attributes.successCount ?? 1); + } - try { - if (doesSavedObjectExist) { - await repository.update(SAVED_OBJECT_ID, SAVED_OBJECT_ID, attributes); - } else { - await repository.create(SAVED_OBJECT_ID, attributes, { id: SAVED_OBJECT_ID }); + try { + if (doesSavedObjectExist) { + await repository.update(SAVED_OBJECT_ID, SAVED_OBJECT_ID, attributes); + } else { + await repository.create(SAVED_OBJECT_ID, attributes, { id: SAVED_OBJECT_ID }); + } + } catch (e) { + // Version conflict error, swallow } - } catch (e) { - // Version conflict error, swallow } }; }; diff --git a/src/plugins/data/server/search/search_service.ts b/src/plugins/data/server/search/search_service.ts index 8ae31b10b48f..42de7fd023ed 100644 --- a/src/plugins/data/server/search/search_service.ts +++ b/src/plugins/data/server/search/search_service.ts @@ -107,7 +107,7 @@ export class SearchService implements Plugin { core: CoreSetup<{}, DataPluginStart>, { registerFunction, usageCollection }: SearchServiceSetupDependencies ): ISearchSetup { - const usage = usageCollection ? usageProvider(core) : undefined; + const usage = usageCollection ? usageProvider(core, this.initializerContext) : undefined; const router = core.http.createRouter(); const routeDependencies = { From 66873a70b540b136a496bd493b1cc55b80f649d9 Mon Sep 17 00:00:00 2001 From: Tao liu Date: Wed, 6 Apr 2022 16:57:51 +0000 Subject: [PATCH 2/5] add unit tests and fixes comments Signed-off-by: Tao liu --- config/opensearch_dashboards.yml | 2 +- src/plugins/data/config.ts | 6 +- .../server/search/collectors/usage.test.ts | 59 +++++++++++++++++++ .../data/server/search/collectors/usage.ts | 2 +- 4 files changed, 64 insertions(+), 5 deletions(-) create mode 100644 src/plugins/data/server/search/collectors/usage.test.ts diff --git a/config/opensearch_dashboards.yml b/config/opensearch_dashboards.yml index a087cf445130..242032edd025 100644 --- a/config/opensearch_dashboards.yml +++ b/config/opensearch_dashboards.yml @@ -168,4 +168,4 @@ # Set the value of this setting to false to suppress search usage telemetry # for reducing the load of OpenSearch cluster. -data.searchUsageTelemetry.enabled: false \ No newline at end of file +# data.search.usageTelemetry.enabled: false \ No newline at end of file diff --git a/src/plugins/data/config.ts b/src/plugins/data/config.ts index 1f5bdda9e5c4..f8b553f3da1b 100644 --- a/src/plugins/data/config.ts +++ b/src/plugins/data/config.ts @@ -48,9 +48,9 @@ export const configSchema = schema.object({ enabled: schema.boolean({ defaultValue: false }), }), }), - }), - searchUsageTelemetry: schema.object({ - enabled: schema.boolean({ defaultValue: false }), + usageTelemetry: schema.object({ + enabled: schema.boolean({ defaultValue: false }), + }), }), }); diff --git a/src/plugins/data/server/search/collectors/usage.test.ts b/src/plugins/data/server/search/collectors/usage.test.ts new file mode 100644 index 000000000000..2e895382278b --- /dev/null +++ b/src/plugins/data/server/search/collectors/usage.test.ts @@ -0,0 +1,59 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Any modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { coreMock } from '../../../../../core/server/mocks'; +import { CoreSetup } from '../../../../../core/server'; +import { first } from 'rxjs/operators'; +import { usageProvider } from '..'; +import { ConfigSchema } from '../../../config'; + +describe('Search usage telemetry', () => { + let mockCoreSetup: MockedKeys; + const initializerContext = coreMock.createPluginInitializerContext({ + search: { usageTelemetry: { enabled: false } }, + }); + + it('trackSuccess should not throw with disabled usageTelemetry', async () => { + const configObject = await initializerContext.config + .create() + .pipe(first()) + .toPromise(); + expect(configObject.search.usageTelemetry.enabled).toBe(false); + + const searchUsage = usageProvider(mockCoreSetup, initializerContext); + expect(searchUsage.trackSuccess(1)).resolves.not.toThrow(); + }); + + it('trackError should not throw with disabled usageTelemetry', () => { + const searchUsage = usageProvider(mockCoreSetup, initializerContext); + expect(searchUsage.trackError.name).toBe('trackError'); + expect(searchUsage.trackError()).resolves.not.toThrow(); + }); +}); \ No newline at end of file diff --git a/src/plugins/data/server/search/collectors/usage.ts b/src/plugins/data/server/search/collectors/usage.ts index ee3b67b926bb..55449c95019b 100644 --- a/src/plugins/data/server/search/collectors/usage.ts +++ b/src/plugins/data/server/search/collectors/usage.ts @@ -50,7 +50,7 @@ export function usageProvider( .create() .pipe(first()) .toPromise(); - if (config.searchUsageTelemetry.enabled) { + if (config.search.usageTelemetry.enabled) { const repository = await core .getStartServices() .then(([coreStart]) => coreStart.savedObjects.createInternalRepository()); From d22f4836a25a9bce11d39200140476ce9454940c Mon Sep 17 00:00:00 2001 From: Tao liu Date: Wed, 6 Apr 2022 17:20:00 +0000 Subject: [PATCH 3/5] add newline at the end of usage.test.ts Signed-off-by: Tao liu --- src/plugins/data/server/search/collectors/usage.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data/server/search/collectors/usage.test.ts b/src/plugins/data/server/search/collectors/usage.test.ts index 2e895382278b..84e8dd89d3dd 100644 --- a/src/plugins/data/server/search/collectors/usage.test.ts +++ b/src/plugins/data/server/search/collectors/usage.test.ts @@ -56,4 +56,4 @@ describe('Search usage telemetry', () => { expect(searchUsage.trackError.name).toBe('trackError'); expect(searchUsage.trackError()).resolves.not.toThrow(); }); -}); \ No newline at end of file +}); From 86196c86985934f49851f7d94c36b9f1de7ec580 Mon Sep 17 00:00:00 2001 From: Tao liu Date: Wed, 6 Apr 2022 21:43:58 +0000 Subject: [PATCH 4/5] add docker var change Signed-off-by: Tao liu --- .../bin/opensearch-dashboards-docker | 1 + .../server/search/collectors/usage.test.ts | 19 ------------------- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/src/dev/build/tasks/os_packages/docker_generator/resources/bin/opensearch-dashboards-docker b/src/dev/build/tasks/os_packages/docker_generator/resources/bin/opensearch-dashboards-docker index 91a3de5014af..a9261f4227f1 100755 --- a/src/dev/build/tasks/os_packages/docker_generator/resources/bin/opensearch-dashboards-docker +++ b/src/dev/build/tasks/os_packages/docker_generator/resources/bin/opensearch-dashboards-docker @@ -25,6 +25,7 @@ opensearch_dashboards_vars=( csp.rules csp.strict csp.warnLegacyBrowsers + data.search.usageTelemetry.enabled opensearch.customHeaders opensearch.hosts opensearch.logQueries diff --git a/src/plugins/data/server/search/collectors/usage.test.ts b/src/plugins/data/server/search/collectors/usage.test.ts index 84e8dd89d3dd..d3b2990a641b 100644 --- a/src/plugins/data/server/search/collectors/usage.test.ts +++ b/src/plugins/data/server/search/collectors/usage.test.ts @@ -9,25 +9,6 @@ * GitHub history for details. */ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - import { coreMock } from '../../../../../core/server/mocks'; import { CoreSetup } from '../../../../../core/server'; import { first } from 'rxjs/operators'; From 96b3fd486ed010751ab4db757bb12b16ef40081e Mon Sep 17 00:00:00 2001 From: Tao liu Date: Thu, 7 Apr 2022 15:56:57 +0000 Subject: [PATCH 5/5] update check configuration Signed-off-by: Tao liu --- src/plugins/data/server/search/collectors/usage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data/server/search/collectors/usage.ts b/src/plugins/data/server/search/collectors/usage.ts index 55449c95019b..d227dea8057c 100644 --- a/src/plugins/data/server/search/collectors/usage.ts +++ b/src/plugins/data/server/search/collectors/usage.ts @@ -50,7 +50,7 @@ export function usageProvider( .create() .pipe(first()) .toPromise(); - if (config.search.usageTelemetry.enabled) { + if (config?.search?.usageTelemetry?.enabled) { const repository = await core .getStartServices() .then(([coreStart]) => coreStart.savedObjects.createInternalRepository());