Skip to content

Commit

Permalink
[UsageCollection] Remove formatBulkUpload and other unused APIs (#8…
Browse files Browse the repository at this point in the history
…4313)

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
afharo and kibanamachine authored Nov 30, 2020
1 parent d11ca6c commit cb55898
Show file tree
Hide file tree
Showing 24 changed files with 64 additions and 326 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('telemetry_application_usage', () => {

const logger = loggingSystemMock.createLogger();

let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function getCoreUsageCollector(
usageCollection: UsageCollectionSetup,
getCoreUsageDataService: () => CoreUsageDataStart
) {
return usageCollection.makeUsageCollector<CoreUsageData, { core: CoreUsageData }>({
return usageCollection.makeUsageCollector<CoreUsageData>({
type: 'core',
isReady: () => typeof getCoreUsageDataService() !== 'undefined',
schema: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { CoreUsageData } from 'src/core/server/';
const logger = loggingSystemMock.createLogger();

describe('telemetry_core', () => {
let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { registerKibanaUsageCollector } from './';
const logger = loggingSystemMock.createLogger();

describe('telemetry_kibana', () => {
let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
Expand Down Expand Up @@ -66,23 +66,4 @@ describe('telemetry_kibana', () => {
timelion_sheet: { total: 0 },
});
});

test('formatForBulkUpload', async () => {
const resultFromFetch = {
index: '.kibana-tests',
dashboard: { total: 0 },
visualization: { total: 0 },
search: { total: 0 },
index_pattern: { total: 0 },
graph_workspace: { total: 0 },
timelion_sheet: { total: 0 },
};

expect(collector.formatForBulkUpload!(resultFromFetch)).toStrictEqual({
type: 'kibana_stats',
payload: {
usage: resultFromFetch,
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { Observable } from 'rxjs';
import { take } from 'rxjs/operators';
import { SharedGlobalConfig } from 'kibana/server';
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { KIBANA_STATS_TYPE } from '../../../common/constants';
import { getSavedObjectsCounts, KibanaSavedObjectCounts } from './get_saved_object_counts';

interface KibanaUsage extends KibanaSavedObjectCounts {
Expand All @@ -32,7 +31,7 @@ export function getKibanaUsageCollector(
usageCollection: UsageCollectionSetup,
legacyConfig$: Observable<SharedGlobalConfig>
) {
return usageCollection.makeUsageCollector<KibanaUsage, { usage: KibanaUsage }>({
return usageCollection.makeUsageCollector<KibanaUsage>({
type: 'kibana',
isReady: () => true,
schema: {
Expand All @@ -53,20 +52,6 @@ export function getKibanaUsageCollector(
...(await getSavedObjectsCounts(callCluster, index)),
};
},

/*
* Format the response data into a model for internal upload
* 1. Make this data part of the "kibana_stats" type
* 2. Organize the payload in the usage namespace of the data payload (usage.index, etc)
*/
formatForBulkUpload: (result) => {
return {
type: KIBANA_STATS_TYPE,
payload: {
usage: result,
},
};
},
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { registerManagementUsageCollector } from './';
const logger = loggingSystemMock.createLogger();

describe('telemetry_application_usage_collector', () => {
let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { loggingSystemMock } from '../../../../../core/server/mocks';
const logger = loggingSystemMock.createLogger();

describe('telemetry_ops_stats', () => {
let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeStatsCollector.mockImplementation((config) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { registerUiMetricUsageCollector } from './';
const logger = loggingSystemMock.createLogger();

describe('telemetry_ui_metric', () => {
let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
Expand Down
44 changes: 0 additions & 44 deletions src/plugins/usage_collection/server/collector/collector.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import { loggingSystemMock } from '../../../../core/server/mocks';
import { Collector } from './collector';
import { UsageCollector } from './usage_collector';

const logger = loggingSystemMock.createLogger();

Expand Down Expand Up @@ -88,49 +87,6 @@ describe('collector', () => {
});
});

describe('formatForBulkUpload', () => {
it('should use the default formatter', () => {
const fetchOutput = { testPass: 100 };
const collector = new Collector(logger, {
type: 'my_test_collector',
isReady: () => false,
fetch: () => fetchOutput,
});
expect(collector.formatForBulkUpload(fetchOutput)).toStrictEqual({
type: 'my_test_collector',
payload: fetchOutput,
});
});

it('should use a custom formatter', () => {
const fetchOutput = { testPass: 100 };
const collector = new Collector(logger, {
type: 'my_test_collector',
isReady: () => false,
fetch: () => fetchOutput,
formatForBulkUpload: (a) => ({ type: 'other_value', payload: { nested: a } }),
});
expect(collector.formatForBulkUpload(fetchOutput)).toStrictEqual({
type: 'other_value',
payload: { nested: fetchOutput },
});
});

it("should use UsageCollector's default formatter", () => {
const fetchOutput = { testPass: 100 };
const collector = new UsageCollector(logger, {
type: 'my_test_collector',
isReady: () => false,
fetch: () => fetchOutput,
schema: { testPass: { type: 'long' } },
});
expect(collector.formatForBulkUpload(fetchOutput)).toStrictEqual({
type: 'kibana_stats',
payload: { usage: { my_test_collector: fetchOutput } },
});
});
});

describe('schema TS validations', () => {
// These tests below are used to ensure types inference is working as expected.
// We don't intend to test any logic as such, just the relation between the types in `fetch` and `schema`.
Expand Down
59 changes: 10 additions & 49 deletions src/plugins/usage_collection/server/collector/collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ import {
KibanaRequest,
} from 'src/core/server';

export type CollectorFormatForBulkUpload<T, U> = (result: T) => { type: string; payload: U };

export type AllowedSchemaNumberTypes = 'long' | 'integer' | 'short' | 'byte' | 'double' | 'float';

export type AllowedSchemaTypes = AllowedSchemaNumberTypes | 'keyword' | 'text' | 'boolean' | 'date';
Expand Down Expand Up @@ -87,7 +85,7 @@ export type CollectorFetchMethod<
TReturn,
ExtraOptions extends object = {}
> = (
this: Collector<TReturn, unknown> & ExtraOptions, // Specify the context of `this` for this.log and others to become available
this: Collector<TReturn> & ExtraOptions, // Specify the context of `this` for this.log and others to become available
context: CollectorFetchContext<WithKibanaRequest>
) => Promise<TReturn> | TReturn;

Expand All @@ -108,7 +106,6 @@ export type CollectorOptionsFetchExtendedContext<

export type CollectorOptions<
TFetchReturn = unknown,
UFormatBulkUploadPayload = TFetchReturn, // TODO: Once we remove bulk_uploader's dependency on usageCollection, we'll be able to remove this type
WithKibanaRequest extends boolean = boolean,
ExtraOptions extends object = {}
> = {
Expand All @@ -130,13 +127,6 @@ export type CollectorOptions<
* @param collectorFetchContext {@link CollectorFetchContext}
*/
fetch: CollectorFetchMethod<WithKibanaRequest, TFetchReturn, ExtraOptions>;
/**
* A hook for allowing the fetched data payload to be organized into a typed
* data model for internal bulk upload. See defaultFormatterForBulkUpload for
* a generic example.
* @deprecated Used only by the Legacy Monitoring collection (to be removed in 8.0)
*/
formatForBulkUpload?: CollectorFormatForBulkUpload<TFetchReturn, UFormatBulkUploadPayload>;
} & ExtraOptions &
(WithKibanaRequest extends true // If enforced to true via Types, the config must be enforced
? {
Expand All @@ -146,40 +136,27 @@ export type CollectorOptions<
extendFetchContext?: CollectorOptionsFetchExtendedContext<WithKibanaRequest>;
});

export class Collector<
TFetchReturn,
UFormatBulkUploadPayload = TFetchReturn,
ExtraOptions extends object = {}
> {
export class Collector<TFetchReturn, ExtraOptions extends object = {}> {
public readonly extendFetchContext: CollectorOptionsFetchExtendedContext<any>;
public readonly type: CollectorOptions<TFetchReturn, UFormatBulkUploadPayload, any>['type'];
public readonly init?: CollectorOptions<TFetchReturn, UFormatBulkUploadPayload, any>['init'];
public readonly type: CollectorOptions<TFetchReturn, any>['type'];
public readonly init?: CollectorOptions<TFetchReturn, any>['init'];
public readonly fetch: CollectorFetchMethod<any, TFetchReturn, ExtraOptions>;
public readonly isReady: CollectorOptions<TFetchReturn, UFormatBulkUploadPayload, any>['isReady'];
private readonly _formatForBulkUpload?: CollectorFormatForBulkUpload<
TFetchReturn,
UFormatBulkUploadPayload
>;
/*
* @param {Object} logger - logger object
* @param {String} options.type - property name as the key for the data
* @param {Function} options.init (optional) - initialization function
* @param {Function} options.fetch - function to query data
* @param {Function} options.formatForBulkUpload - optional
* @param {Function} options.isReady - method that returns a boolean or Promise of a boolean to indicate the collector is ready to report data
* @param {Function} options.rest - optional other properties
public readonly isReady: CollectorOptions<TFetchReturn, any>['isReady'];
/**
* @private Constructor of a Collector. It should be called via the CollectorSet factory methods: `makeStatsCollector` and `makeUsageCollector`
* @param log {@link Logger}
* @param collectorDefinition {@link CollectorOptions}
*/
constructor(
public readonly log: Logger,
{
type,
init,
fetch,
formatForBulkUpload,
isReady,
extendFetchContext = {},
...options
}: CollectorOptions<TFetchReturn, UFormatBulkUploadPayload, any, ExtraOptions>
}: CollectorOptions<TFetchReturn, any, ExtraOptions>
) {
if (type === undefined) {
throw new Error('Collector must be instantiated with a options.type string property');
Expand All @@ -200,21 +177,5 @@ export class Collector<
this.fetch = fetch;
this.isReady = typeof isReady === 'function' ? isReady : () => true;
this.extendFetchContext = extendFetchContext;
this._formatForBulkUpload = formatForBulkUpload;
}

public formatForBulkUpload(result: TFetchReturn) {
if (this._formatForBulkUpload) {
return this._formatForBulkUpload(result);
} else {
return this.defaultFormatterForBulkUpload(result);
}
}

protected defaultFormatterForBulkUpload(result: TFetchReturn) {
return {
type: this.type,
payload: (result as unknown) as UFormatBulkUploadPayload,
};
}
}
Loading

0 comments on commit cb55898

Please sign in to comment.