Skip to content

Commit

Permalink
[search telemetry] Fixes search telemetry's observable object that wo…
Browse files Browse the repository at this point in the history
…n't be GC-ed (#3390) (#3422)

The search telemetry was disabled by default, there is a issue when search telemetry read configuration and creates an Observable object that won't be GC-ed.

Signed-off-by: Tao Liu <liutaoaz@amazon.com>
(cherry picked from commit 7794b62)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent f9ae7d8 commit e025cc1
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 16 deletions.
4 changes: 2 additions & 2 deletions src/plugins/data/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export class DataServerPlugin
this.autocompleteService = new AutocompleteService(initializerContext);
}

public setup(
public async setup(
core: CoreSetup<DataPluginStartDependencies, DataPluginStart>,
{ expressions, usageCollection, dataSource }: DataPluginSetupDependencies
) {
Expand All @@ -108,7 +108,7 @@ export class DataServerPlugin

core.uiSettings.register(getUiSettings());

const searchSetup = this.searchService.setup(core, {
const searchSetup = await this.searchService.setup(core, {
registerFunction: expressions.registerFunction,
usageCollection,
dataSource,
Expand Down
12 changes: 2 additions & 10 deletions src/plugins/data/server/search/collectors/usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@
* under the License.
*/

import { CoreSetup, PluginInitializerContext } from 'opensearch-dashboards/server';
import { first } from 'rxjs/operators';
import { CoreSetup } from 'opensearch-dashboards/server';
import { Usage } from './register';
import { ConfigSchema } from '../../../config';

Expand All @@ -40,16 +39,9 @@ export interface SearchUsage {
trackSuccess(duration: number): Promise<void>;
}

export function usageProvider(
core: CoreSetup,
initializerContext: PluginInitializerContext<ConfigSchema>
): SearchUsage {
export function usageProvider(core: CoreSetup, config: ConfigSchema): SearchUsage {
const getTracker = (eventType: keyof Usage) => {
return async (duration?: number) => {
const config = await initializerContext.config
.create<ConfigSchema>()
.pipe(first())
.toPromise();
if (config?.search?.usageTelemetry?.enabled) {
const repository = await core
.getStartServices()
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/server/search/search_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('Search service', () => {

describe('setup()', () => {
it('exposes proper contract', async () => {
const setup = plugin.setup(mockCoreSetup, ({
const setup = await plugin.setup(mockCoreSetup, ({
packageInfo: { version: '8' },
registerFunction: jest.fn(),
} as unknown) as SearchServiceSetupDependencies);
Expand Down
10 changes: 7 additions & 3 deletions src/plugins/data/server/search/search_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,15 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
private readonly logger: Logger
) {}

public setup(
public async setup(
core: CoreSetup<{}, DataPluginStart>,
{ registerFunction, usageCollection, dataSource }: SearchServiceSetupDependencies
): ISearchSetup {
const usage = usageCollection ? usageProvider(core, this.initializerContext) : undefined;
): Promise<ISearchSetup> {
const config = await this.initializerContext.config
.create<ConfigSchema>()
.pipe(first())
.toPromise();
const usage = usageCollection ? usageProvider(core, config) : undefined;

const router = core.http.createRouter();
const routeDependencies = {
Expand Down

0 comments on commit e025cc1

Please sign in to comment.