Skip to content

Commit

Permalink
Don't catch fetch function exceptions, let usage collector handle it
Browse files Browse the repository at this point in the history
  • Loading branch information
rudolf committed Oct 2, 2020
1 parent a508a0a commit dbee8f9
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 46 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export interface CoreStart
| --- | --- | --- |
| [auditTrail](./kibana-plugin-core-server.corestart.audittrail.md) | <code>AuditTrailStart</code> | [AuditTrailSetup](./kibana-plugin-core-server.audittrailsetup.md) |
| [capabilities](./kibana-plugin-core-server.corestart.capabilities.md) | <code>CapabilitiesStart</code> | [CapabilitiesStart](./kibana-plugin-core-server.capabilitiesstart.md) |
| [coreUsageData](./kibana-plugin-core-server.corestart.coreusagedata.md) | <code>CoreUsageDataStart</code> | |
| [elasticsearch](./kibana-plugin-core-server.corestart.elasticsearch.md) | <code>ElasticsearchServiceStart</code> | [ElasticsearchServiceStart](./kibana-plugin-core-server.elasticsearchservicestart.md) |
| [http](./kibana-plugin-core-server.corestart.http.md) | <code>HttpServiceStart</code> | [HttpServiceStart](./kibana-plugin-core-server.httpservicestart.md) |
| [metrics](./kibana-plugin-core-server.corestart.metrics.md) | <code>MetricsServiceStart</code> | [MetricsServiceStart](./kibana-plugin-core-server.metricsservicestart.md) |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,6 @@ describe('CoreUsageDataService', () => {

describe('start', () => {
describe('getCoreUsageData', () => {
it('returns null if an exception occurs', () => {
const metrics = metricsServiceMock.createInternalSetupContract();
metrics.getOpsMetrics$.mockReturnValueOnce(new BehaviorSubject({} as any));
service.setup({ metrics });
const { getCoreUsageData } = service.start();
expect(getCoreUsageData()).toEqual(null);
});

it('returns core metrics for default config', () => {
const metrics = metricsServiceMock.createInternalSetupContract();
service.setup({ metrics });
Expand Down
11 changes: 2 additions & 9 deletions src/core/server/core_usage_data/core_usage_data_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { CoreService } from 'src/core/types';
import { CoreContext } from '../core_context';
import { ElasticsearchConfigType } from '../elasticsearch/elasticsearch_config';
import { HttpConfigType } from '../http';
import { Logger, LoggingConfigType } from '../logging';
import { LoggingConfigType } from '../logging';
import { SavedObjectsConfigType } from '../saved_objects/saved_objects_config';
import { CoreUsageData, CoreUsageDataStart } from './types';
import { SavedObjectsService } from '../saved_objects';
Expand All @@ -40,7 +40,6 @@ export interface StartDeps {
}

export class CoreUsageDataService implements CoreService<void, CoreUsageDataStart> {
private readonly log: Logger;
private elasticsearchConfig?: ElasticsearchConfigType;
private configService: CoreContext['configService'];
private httpConfig?: HttpConfigType;
Expand All @@ -50,7 +49,6 @@ export class CoreUsageDataService implements CoreService<void, CoreUsageDataStar
private opsMetrics?: OpsMetrics;

constructor(core: CoreContext) {
this.log = core.logger.get('core_usage_data');
this.configService = core.configService;
this.stop$ = new Subject();
}
Expand Down Expand Up @@ -201,12 +199,7 @@ export class CoreUsageDataService implements CoreService<void, CoreUsageDataStar
start() {
return {
getCoreUsageData: () => {
try {
return this.getCoreUsageData();
} catch (e) {
this.log.error('Error collecting core usage data', e);
return null;
}
return this.getCoreUsageData();
},
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/core_usage_data/is_configured.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ describe('isConfigured', () => {
expect(isConfigured.record(undefined)).toEqual(false);
});
it('returns false for null', () => {
expect(isConfigured.record(null)).toEqual(false);
expect(isConfigured.record(null as any)).toEqual(false);
});
});

Expand Down
2 changes: 1 addition & 1 deletion src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ export interface CoreStart {
uiSettings: UiSettingsServiceStart;
/** {@link AuditTrailSetup} */
auditTrail: AuditTrailStart;
/** {@link CoreUsageDataStart} */
/** @internal {@link CoreUsageDataStart} */
coreUsageData: CoreUsageDataStart;
}

Expand Down
4 changes: 1 addition & 3 deletions src/core/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -438,9 +438,7 @@ export interface CoreStart {
auditTrail: AuditTrailStart;
// (undocumented)
capabilities: CapabilitiesStart;
// Warning: (ae-incompatible-release-tags) The symbol "coreUsageData" is marked as @public, but its signature references "CoreUsageDataStart" which is marked as @internal
//
// (undocumented)
// @internal (undocumented)
coreUsageData: CoreUsageDataStart;
// (undocumented)
elasticsearch: ElasticsearchServiceStart;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { KIBANA_STATS_TYPE } from '../../../common/constants';

export function getCoreUsageCollector(
usageCollection: UsageCollectionSetup,
getCoreUsageData: () => CoreUsageDataStart
getCoreUsageDataService: () => CoreUsageDataStart
) {
return usageCollection.makeUsageCollector<CoreUsageData, { core: CoreUsageData }>({
type: 'core',
Expand Down Expand Up @@ -116,14 +116,7 @@ export function getCoreUsageCollector(
},
},
fetch() {
return new Promise((resolve, reject) => {
const result = getCoreUsageData().getCoreUsageData();
if (result == null) {
reject(new Error('Unable to collect Core telemetry'));
} else {
resolve(result);
}
});
return getCoreUsageDataService().getCoreUsageData();
},

/*
Expand All @@ -144,7 +137,9 @@ export function getCoreUsageCollector(

export function registerCoreUsageCollector(
usageCollection: UsageCollectionSetup,
getCoreUsageData: () => CoreUsageDataStart
getCoreUsageDataService: () => CoreUsageDataStart
) {
usageCollection.registerCollector(getCoreUsageCollector(usageCollection, getCoreUsageData));
usageCollection.registerCollector(
getCoreUsageCollector(usageCollection, getCoreUsageDataService)
);
}

0 comments on commit dbee8f9

Please sign in to comment.