Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PURIFY] usageCollector removal of ui metrics reporting (#10) #18

Merged
merged 5 commits into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 4 additions & 13 deletions .i18nrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@
"interpreter": "src/legacy/core_plugins/interpreter",
"kbn": "src/legacy/core_plugins/kibana",
"kbnDocViews": "src/legacy/core_plugins/kbn_doc_views",
"management": [
"src/legacy/core_plugins/management",
"src/plugins/management"
],
"management": ["src/legacy/core_plugins/management", "src/plugins/management"],
"maps_legacy": "src/plugins/maps_legacy",
"indexPatternManagement": "src/plugins/index_pattern_management",
"advancedSettings": "src/plugins/advanced_settings",
Expand All @@ -42,10 +39,7 @@
"security": "src/plugins/security_oss",
"server": "src/legacy/server",
"statusPage": "src/legacy/core_plugins/status_page",
"telemetry": [
"src/plugins/telemetry",
"src/plugins/telemetry_management_section"
],
"telemetry": ["src/plugins/telemetry", "src/plugins/telemetry_management_section"],
"tileMap": "src/plugins/tile_map",
"timelion": ["src/plugins/timelion", "src/plugins/vis_type_timelion"],
"uiActions": "src/plugins/ui_actions",
Expand All @@ -60,11 +54,8 @@
"visTypeXy": "src/plugins/vis_type_xy",
"visualizations": "src/plugins/visualizations",
"visualize": "src/plugins/visualize",
"apmOss": "src/plugins/apm_oss",
"usageCollection": "src/plugins/usage_collection"
"apmOss": "src/plugins/apm_oss"
},
"exclude": [
"src/legacy/ui/ui_render/ui_render_mixin.js"
],
"exclude": ["src/legacy/ui/ui_render/ui_render_mixin.js"],
"translations": []
}
12 changes: 6 additions & 6 deletions packages/kbn-analytics/src/reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class Reporter {
checkInterval: number;
private interval?: NodeJS.Timer;
private lastAppId?: string;
private http: ReportHTTP;
// private http: ReportHTTP;
private reportManager: ReportManager;
private storageManager: ReportStorageManager;
private readonly applicationUsage: ApplicationUsage;
Expand All @@ -48,8 +48,8 @@ export class Reporter {
private started = false;

constructor(config: ReporterConfig) {
const { http, storage, debug, checkInterval = 90000, storageKey = 'analytics' } = config;
this.http = http;
const { storage, debug, checkInterval = 90000, storageKey = 'analytics' } = config;
// this.http = http;
this.checkInterval = checkInterval;
this.applicationUsage = new ApplicationUsage();
this.storageManager = new ReportStorageManager(storageKey, storage);
Expand Down Expand Up @@ -133,14 +133,14 @@ export class Reporter {
public reportApplicationUsage(appId?: string) {
this.log(`Reporting application changed to ${appId}`);
this.lastAppId = appId || this.lastAppId;
const appChangedReport = this.applicationUsage.appChanged(appId);
if (appChangedReport) this.saveToReport([appChangedReport]);
// const appChangedReport = this.applicationUsage.appChanged(appId);
// if (appChangedReport) this.saveToReport([appChangedReport]);
}

public sendReports = async () => {
if (!this.reportManager.isReportEmpty()) {
try {
await this.http(this.reportManager.report);
// await this.http(this.reportManager.report);
this.flushReport();
} catch (err) {
this.log(`Error Sending Metrics Report ${err}`);
Expand Down
16 changes: 4 additions & 12 deletions src/plugins/data/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,8 @@
"version": "kibana",
"server": true,
"ui": true,
"requiredPlugins": [
"expressions",
"uiActions"
],
"optionalPlugins": ["usageCollection"],
"requiredPlugins": ["expressions", "uiActions"],
"optionalPlugins": [],
"extraPublicDirs": ["common", "common/utils/abort_utils"],
"requiredBundles": [
"usageCollection",
"kibanaUtils",
"kibanaReact",
"inspector"
]
}
"requiredBundles": ["kibanaUtils", "kibanaReact", "inspector"]

Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { SEARCH_EVENT_TYPE, SearchUsageCollector } from './types';
import { METRIC_TYPE } from '@kbn/analytics';
import { from } from 'rxjs';

describe('Search Usage Collector', () => {
describe.skip('Search Usage Collector', () => {
let mockCoreSetup: MockedKeys<CoreSetup>;
let mockUsageCollectionSetup: Setup;
let usageCollector: SearchUsageCollector;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/public/search/collectors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@
* under the License.
*/

export { createUsageCollector } from './create_usage_collector';
// export { createUsageCollector } from './create_usage_collector';
export { SEARCH_EVENT_TYPE, SearchUsageCollector } from './types';
4 changes: 2 additions & 2 deletions src/plugins/data/public/search/search_interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
ISearchOptions,
ES_SEARCH_STRATEGY,
} from '../../common';
import { SearchUsageCollector } from './collectors';
// import { SearchUsageCollector } from './collectors';
import { SearchTimeoutError, PainlessError, isPainlessError, TimeoutErrorMode } from './errors';
import { toMountPoint } from '../../../kibana_react/public';

Expand All @@ -38,7 +38,7 @@ export interface SearchInterceptorDeps {
uiSettings: CoreSetup['uiSettings'];
startServices: Promise<[CoreStart, any, unknown]>;
toasts: ToastsSetup;
usageCollector?: SearchUsageCollector;
// usageCollector?: SearchUsageCollector;
}

export class SearchInterceptor {
Expand Down
17 changes: 9 additions & 8 deletions src/plugins/data/public/search/search_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ import { getCallMsearch } from './legacy';
import { AggsService, AggsStartDependencies } from './aggs';
import { IndexPatternsContract } from '../index_patterns/index_patterns';
import { ISearchInterceptor, SearchInterceptor } from './search_interceptor';
import { SearchUsageCollector, createUsageCollector } from './collectors';
import { UsageCollectionSetup } from '../../../usage_collection/public';
// import { SearchUsageCollector, createUsageCollector } from './collectors';
// import { UsageCollectionSetup } from '../../../usage_collection/public';
import { esdsl, esRawResponse } from './expressions';
import { ExpressionsSetup } from '../../../expressions/public';
import { ConfigSchema } from '../../config';
Expand All @@ -50,7 +50,7 @@ import { aggShardDelay } from '../../common/search/aggs/buckets/shard_delay_fn';
/** @internal */
export interface SearchServiceSetupDependencies {
expressions: ExpressionsSetup;
usageCollection?: UsageCollectionSetup;
// usageCollection?: UsageCollectionSetup;
}

/** @internal */
Expand All @@ -63,15 +63,16 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
private readonly aggsService = new AggsService();
private readonly searchSourceService = new SearchSourceService();
private searchInterceptor!: ISearchInterceptor;
private usageCollector?: SearchUsageCollector;
// private usageCollector?: SearchUsageCollector;

constructor(private initializerContext: PluginInitializerContext<ConfigSchema>) {}

public setup(
{ http, getStartServices, notifications, uiSettings }: CoreSetup,
{ expressions, usageCollection }: SearchServiceSetupDependencies
// { expressions, usageCollection }: SearchServiceSetupDependencies
{ expressions }: SearchServiceSetupDependencies
): ISearchSetup {
this.usageCollector = createUsageCollector(getStartServices, usageCollection);
// this.usageCollector = createUsageCollector(getStartServices, usageCollection);

/**
* A global object that intercepts all searches and provides convenience methods for cancelling
Expand All @@ -82,7 +83,7 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
http,
uiSettings,
startServices: getStartServices(),
usageCollector: this.usageCollector!,
usageCollector: undefined, // this.usageCollector!,
});

expressions.registerFunction(esdsl);
Expand All @@ -100,7 +101,7 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {

return {
aggs,
usageCollector: this.usageCollector!,
usageCollector: undefined, // this.usageCollector!,
__enhance: (enhancements: SearchEnhancements) => {
this.searchInterceptor = enhancements.searchInterceptor;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ jest.mock('../kibana_services', () => ({
trackUiMetric: () => {},
}),
}));

/*
test('should render a Welcome screen with the telemetry disclaimer', () => {
const telemetry = telemetryPluginMock.createStartContract();
const component = shallow(<Welcome urlBasePath="/" onSkip={() => {}} telemetry={telemetry} />);

expect(component).toMatchSnapshot();
});

*/
test('should render a Welcome screen with the telemetry disclaimer when optIn is true', () => {
const telemetry = telemetryPluginMock.createStartContract();
telemetry.telemetryService.getIsOptedIn = jest.fn().mockReturnValue(true);
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/telemetry/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export const PATH_TO_ADVANCED_SETTINGS = 'management/kibana/settings';
/**
* Link to the Elastic Telemetry privacy statement.
*/
export const PRIVACY_STATEMENT_URL = `https://www.elastic.co/legal/privacy-statement`;
export const PRIVACY_STATEMENT_URL = ``;

/**
* The endpoint version when hitting the remote telemetry service
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { EuiButton } from '@elastic/eui';
import { shallowWithIntl } from 'test_utils/enzyme_helpers';
import { OptInBanner } from './opt_in_banner';

describe('OptInDetailsComponent', () => {
describe.skip('OptInDetailsComponent', () => {
it('renders as expected', () => {
expect(shallowWithIntl(<OptInBanner onChangeOptInClick={() => {}} />)).toMatchSnapshot();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import React from 'react';
import { shallowWithIntl } from 'test_utils/enzyme_helpers';
import { OptInMessage } from './opt_in_message';

describe('OptInMessage', () => {
describe.skip('OptInMessage', () => {
it('renders as expected', () => {
expect(shallowWithIntl(<OptInMessage />)).toMatchSnapshot();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { EuiButton } from '@elastic/eui';
import { shallowWithIntl } from 'test_utils/enzyme_helpers';
import { OptedInNoticeBanner } from './opted_in_notice_banner';

describe('OptInDetailsComponent', () => {
describe.skip('OptInDetailsComponent', () => {
it('renders as expected', () => {
expect(shallowWithIntl(<OptedInNoticeBanner onSeenBanner={() => {}} />)).toMatchSnapshot();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { renderOptInBanner } from './render_opt_in_banner';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { overlayServiceMock } from '../../../../../core/public/overlays/overlay_service.mock';

describe('renderOptInBanner', () => {
describe.skip('renderOptInBanner', () => {
it('adds a banner to banners with priority of 10000', () => {
const bannerID = 'brucer-wayne';
const overlays = overlayServiceMock.createStartContract();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { renderOptedInNoticeBanner } from './render_opted_in_notice_banner';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { overlayServiceMock } from '../../../../../core/public/overlays/overlay_service.mock';

describe('renderOptedInNoticeBanner', () => {
describe.skip('renderOptedInNoticeBanner', () => {
it('adds a banner to banners with priority of 10000', () => {
const bannerID = 'brucer-wayne';
const overlays = overlayServiceMock.createStartContract();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
/* eslint-disable dot-notation */
import { mockTelemetryNotifications, mockTelemetryService } from '../../mocks';

describe('onSetOptInClick', () => {
describe.skip('onSetOptInClick', () => {
it('sets setting successfully and removes banner', async () => {
const optIn = true;
const bannerId = 'bruce-banner';
Expand All @@ -38,7 +38,7 @@ describe('onSetOptInClick', () => {
});
});

describe('setOptedInNoticeSeen', () => {
describe.skip('setOptedInNoticeSeen', () => {
it('sets setting successfully and removes banner', async () => {
const bannerId = 'bruce-banner';

Expand All @@ -54,7 +54,7 @@ describe('setOptedInNoticeSeen', () => {
});
});

describe('shouldShowOptedInNoticeBanner', () => {
describe.skip('shouldShowOptedInNoticeBanner', () => {
it("should return true because a banner hasn't been shown, the notice hasn't been seen and the user has privileges to edit saved objects", () => {
const telemetryService = mockTelemetryService();
telemetryService.getUserShouldSeeOptInNotice = jest.fn().mockReturnValue(true);
Expand Down
12 changes: 6 additions & 6 deletions src/plugins/telemetry/public/services/telemetry_sender.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Object.defineProperty(window, 'localStorage', {
value: mockLocalStorage,
});

describe('TelemetrySender', () => {
describe.skip('TelemetrySender', () => {
beforeEach(() => {
mockLocalStorage.getItem.mockClear();
mockLocalStorage.setItem.mockClear();
Expand All @@ -43,7 +43,7 @@ describe('TelemetrySender', () => {
})
);

describe('constructor', () => {
describe.skip('constructor', () => {
it('defaults lastReport if unset', () => {
const telemetryService = mockTelemetryService();
const telemetrySender = new TelemetrySender(telemetryService);
Expand All @@ -61,7 +61,7 @@ describe('TelemetrySender', () => {
});
});

describe('saveToBrowser', () => {
describe.skip('saveToBrowser', () => {
it('uses lastReport', () => {
const lastReport = `${Date.now()}`;
const telemetryService = mockTelemetryService();
Expand All @@ -77,7 +77,7 @@ describe('TelemetrySender', () => {
});
});

describe('shouldSendReport', () => {
describe.skip('shouldSendReport', () => {
it('returns false whenever optIn is false', () => {
const telemetryService = mockTelemetryService();
telemetryService.getIsOptedIn = jest.fn().mockReturnValue(false);
Expand Down Expand Up @@ -129,7 +129,7 @@ describe('TelemetrySender', () => {
expect(shouldSendRerpot).toBe(true);
});

describe('sendIfDue', () => {
describe.skip('sendIfDue', () => {
let originalFetch: typeof window['fetch'];
let mockFetch: jest.Mock<typeof window['fetch']>;

Expand Down Expand Up @@ -252,7 +252,7 @@ describe('TelemetrySender', () => {
});
});
});
describe('startChecking', () => {
describe.skip('startChecking', () => {
let originalSetInterval: typeof window['setInterval'];
let mockSetInterval: jest.Mock<typeof window['setInterval']>;

Expand Down
6 changes: 4 additions & 2 deletions src/plugins/telemetry/public/services/telemetry_sender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,15 @@ export class TelemetrySender {
// check if opt-in for telemetry is enabled
if (this.telemetryService.getIsOptedIn()) {
if (!this.lastReported) {
return true;
// return true;
return false;
}
// returns NaN for any malformed or unset (null/undefined) value
const lastReported = parseInt(this.lastReported, 10);
// If it's been a day since we last sent telemetry
if (isNaN(lastReported) || Date.now() - lastReported > REPORT_INTERVAL_MS) {
return true;
// return true;
return false;
}
}

Expand Down
Loading