Skip to content

Commit

Permalink
[UsageCollection] Expose KibanaRequest to explicitly opted-in colle…
Browse files Browse the repository at this point in the history
…ctors (#83413)
  • Loading branch information
afharo authored Nov 18, 2020
1 parent 6ff61c0 commit 484437f
Show file tree
Hide file tree
Showing 39 changed files with 667 additions and 180 deletions.
2 changes: 1 addition & 1 deletion src/fixtures/telemetry_collectors/nested_collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ interface Usage {
}

export class NestedInside {
collector?: UsageCollector<Usage, Usage>;
collector?: UsageCollector<Usage>;
createMyCollector() {
this.collector = collectorSet.makeUsageCollector<Usage>({
type: 'my_nested_collector',
Expand Down
8 changes: 4 additions & 4 deletions src/plugins/data/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ import { ExpressionAstFunction } from 'src/plugins/expressions/common';
import { ExpressionFunctionDefinition } from 'src/plugins/expressions/common';
import { ExpressionsServerSetup } from 'src/plugins/expressions/server';
import { ExpressionValueBoxed } from 'src/plugins/expressions/common';
import { ISavedObjectsRepository } from 'kibana/server';
import { ISavedObjectsRepository } from 'src/core/server';
import { IScopedClusterClient } from 'src/core/server';
import { ISearchOptions as ISearchOptions_2 } from 'src/plugins/data/public';
import { ISearchSource } from 'src/plugins/data/public';
import { IUiSettingsClient } from 'src/core/server';
import { KibanaRequest } from 'src/core/server';
import { LegacyAPICaller } from 'kibana/server';
import { Logger } from 'kibana/server';
import { Logger as Logger_2 } from 'src/core/server';
import { LegacyAPICaller } from 'src/core/server';
import { Logger } from 'src/core/server';
import { Logger as Logger_2 } from 'kibana/server';
import { LoggerFactory } from '@kbn/logging';
import { Moment } from 'moment';
import moment from 'moment';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import { savedObjectsRepositoryMock, loggingSystemMock } from '../../../../../core/server/mocks';
import {
CollectorOptions,
Collector,
createUsageCollectionSetupMock,
} from '../../../../usage_collection/server/usage_collection.mock';

Expand All @@ -40,11 +40,11 @@ describe('telemetry_application_usage', () => {

const logger = loggingSystemMock.createLogger();

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

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
collector = config;
collector = new Collector(logger, config);
return createUsageCollectionSetupMock().makeUsageCollector(config);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,22 @@
*/

import {
CollectorOptions,
Collector,
createUsageCollectionSetupMock,
} from '../../../../usage_collection/server/usage_collection.mock';
import { createCollectorFetchContextMock } from 'src/plugins/usage_collection/server/mocks';
import { registerCoreUsageCollector } from '.';
import { coreUsageDataServiceMock } from '../../../../../core/server/mocks';
import { coreUsageDataServiceMock, loggingSystemMock } from '../../../../../core/server/mocks';
import { CoreUsageData } from 'src/core/server/';

const logger = loggingSystemMock.createLogger();

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

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
collector = config;
collector = new Collector(logger, config);
return createUsageCollectionSetupMock().makeUsageCollector(config);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,13 @@

import { CspConfig, ICspConfig } from '../../../../../core/server';
import { createCspCollector } from './csp_collector';
import { httpServiceMock } from '../../../../../core/server/mocks';
import { createCollectorFetchContextMock } from 'src/plugins/usage_collection/server/mocks';
import { httpServiceMock, loggingSystemMock } from '../../../../../core/server/mocks';
import {
Collector,
createCollectorFetchContextMock,
} from 'src/plugins/usage_collection/server/mocks';

const logger = loggingSystemMock.createLogger();

describe('csp collector', () => {
let httpMock: ReturnType<typeof httpServiceMock.createSetupContract>;
Expand All @@ -36,7 +41,7 @@ describe('csp collector', () => {
});

test('fetches whether strict mode is enabled', async () => {
const collector = createCspCollector(httpMock);
const collector = new Collector(logger, createCspCollector(httpMock));

expect((await collector.fetch(mockedFetchContext)).strict).toEqual(true);

Expand All @@ -45,7 +50,7 @@ describe('csp collector', () => {
});

test('fetches whether the legacy browser warning is enabled', async () => {
const collector = createCspCollector(httpMock);
const collector = new Collector(logger, createCspCollector(httpMock));

expect((await collector.fetch(mockedFetchContext)).warnLegacyBrowsers).toEqual(true);

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

test('fetches whether the csp rules have been changed or not', async () => {
const collector = createCspCollector(httpMock);
const collector = new Collector(logger, createCspCollector(httpMock));

expect((await collector.fetch(mockedFetchContext)).rulesChangedFromDefault).toEqual(false);

Expand All @@ -63,7 +68,7 @@ describe('csp collector', () => {
});

test('does not include raw csp rules under any property names', async () => {
const collector = createCspCollector(httpMock);
const collector = new Collector(logger, createCspCollector(httpMock));

// It's important that we do not send the value of csp.rules here as it
// can be customized with values that can be identifiable to given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,25 @@
* under the License.
*/

import { pluginInitializerContextConfigMock } from '../../../../../core/server/mocks';
import {
CollectorOptions,
loggingSystemMock,
pluginInitializerContextConfigMock,
} from '../../../../../core/server/mocks';
import {
Collector,
createUsageCollectionSetupMock,
} from '../../../../usage_collection/server/usage_collection.mock';
import { createCollectorFetchContextMock } from '../../../../usage_collection/server/mocks';
import { registerKibanaUsageCollector } from './';

const logger = loggingSystemMock.createLogger();

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

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
collector = config;
collector = new Collector(logger, config);
return createUsageCollectionSetupMock().makeUsageCollector(config);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,23 @@
* under the License.
*/

import { uiSettingsServiceMock } from '../../../../../core/server/mocks';
import { loggingSystemMock, uiSettingsServiceMock } from '../../../../../core/server/mocks';
import {
CollectorOptions,
Collector,
createUsageCollectionSetupMock,
createCollectorFetchContextMock,
} from '../../../../usage_collection/server/usage_collection.mock';

import { registerManagementUsageCollector } from './';

const logger = loggingSystemMock.createLogger();

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

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
collector = config;
collector = new Collector(logger, config);
return createUsageCollectionSetupMock().makeUsageCollector(config);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,23 @@

import { Subject } from 'rxjs';
import {
CollectorOptions,
Collector,
createUsageCollectionSetupMock,
createCollectorFetchContextMock,
} from '../../../../usage_collection/server/usage_collection.mock';

import { registerOpsStatsCollector } from './';
import { OpsMetrics } from '../../../../../core/server';
import { loggingSystemMock } from '../../../../../core/server/mocks';

const logger = loggingSystemMock.createLogger();

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

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeStatsCollector.mockImplementation((config) => {
collector = config;
collector = new Collector(logger, config);
return createUsageCollectionSetupMock().makeStatsCollector(config);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,23 @@
* under the License.
*/

import { savedObjectsRepositoryMock } from '../../../../../core/server/mocks';
import { loggingSystemMock, savedObjectsRepositoryMock } from '../../../../../core/server/mocks';
import {
CollectorOptions,
Collector,
createUsageCollectionSetupMock,
createCollectorFetchContextMock,
} from '../../../../usage_collection/server/usage_collection.mock';

import { registerUiMetricUsageCollector } from './';

const logger = loggingSystemMock.createLogger();

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

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
collector = config;
collector = new Collector(logger, config);
return createUsageCollectionSetupMock().makeUsageCollector(config);
});

Expand Down
11 changes: 9 additions & 2 deletions src/plugins/telemetry/server/telemetry_collection/get_kibana.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { omit } from 'lodash';
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import {
ISavedObjectsRepository,
KibanaRequest,
LegacyAPICaller,
SavedObjectsClientContract,
} from 'kibana/server';
Expand Down Expand Up @@ -89,8 +90,14 @@ export async function getKibana(
usageCollection: UsageCollectionSetup,
callWithInternalUser: LegacyAPICaller,
asInternalUser: ElasticsearchClient,
soClient: SavedObjectsClientContract | ISavedObjectsRepository
soClient: SavedObjectsClientContract | ISavedObjectsRepository,
kibanaRequest: KibanaRequest | undefined // intentionally `| undefined` to enforce providing the parameter
): Promise<KibanaUsageStats> {
const usage = await usageCollection.bulkFetch(callWithInternalUser, asInternalUser, soClient);
const usage = await usageCollection.bulkFetch(
callWithInternalUser,
asInternalUser,
soClient,
kibanaRequest
);
return usageCollection.toObject(usage);
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
usageCollectionPluginMock,
createCollectorFetchContextMock,
} from '../../../usage_collection/server/mocks';
import { elasticsearchServiceMock } from '../../../../../src/core/server/mocks';
import { elasticsearchServiceMock, httpServerMock } from '../../../../../src/core/server/mocks';

function mockUsageCollection(kibanaUsage = {}) {
const usageCollection = usageCollectionPluginMock.createSetupContract();
Expand Down Expand Up @@ -87,6 +87,7 @@ function mockStatsCollectionConfig(clusterInfo: any, clusterStats: any, kibana:
...createCollectorFetchContextMock(),
esClient: mockGetLocalStats(clusterInfo, clusterStats),
usageCollection: mockUsageCollection(kibana),
kibanaRequest: httpServerMock.createKibanaRequest(),
timestamp: Date.now(),
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,24 +62,24 @@ export type TelemetryLocalStats = ReturnType<typeof handleLocalStats>;

/**
* Get statistics for all products joined by Elasticsearch cluster.
* @param {Array} cluster uuids
* @param {Object} config contains the new esClient already scoped contains usageCollection, callCluster, esClient, start, end
* @param {Array} cluster uuids array of cluster uuid's
* @param {Object} config contains the usageCollection, callCluster (deprecated), the esClient and Saved Objects client scoped to the request or the internal repository, and the kibana request
* @param {Object} StatsCollectionContext contains logger and version (string)
*/
export const getLocalStats: StatsGetter<{}, TelemetryLocalStats> = async (
clustersDetails, // array of cluster uuid's
config, // contains the new esClient already scoped contains usageCollection, callCluster, esClient, start, end and the saved objects client scoped to the request or the internal repository
context // StatsCollectionContext contains logger and version (string)
clustersDetails,
config,
context
) => {
const { callCluster, usageCollection, esClient, soClient } = config;
const { callCluster, usageCollection, esClient, soClient, kibanaRequest } = config;

return await Promise.all(
clustersDetails.map(async (clustersDetail) => {
const [clusterInfo, clusterStats, nodesUsage, kibana, dataTelemetry] = await Promise.all([
getClusterInfo(esClient), // cluster info
getClusterStats(esClient), // cluster stats (not to be confused with cluster _state_)
getNodesUsage(esClient), // nodes_usage info
getKibana(usageCollection, callCluster, esClient, soClient),
getKibana(usageCollection, callCluster, esClient, soClient, kibanaRequest),
getDataTelemetry(esClient),
]);
return handleLocalStats(
Expand Down
5 changes: 4 additions & 1 deletion src/plugins/telemetry_collection_manager/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ export class TelemetryCollectionManagerPlugin
const soClient = config.unencrypted
? collectionSoService.getScopedClient(config.request)
: collectionSoService.createInternalRepository();
return { callCluster, timestamp, usageCollection, esClient, soClient };
// Provide the kibanaRequest so opted-in plugins can scope their custom clients only if the request is not encrypted
const kibanaRequest = config.unencrypted ? request : void 0;

return { callCluster, timestamp, usageCollection, esClient, soClient, kibanaRequest };
}

private async getOptInStats(optInStatus: boolean, config: StatsGetterConfig) {
Expand Down
1 change: 1 addition & 0 deletions src/plugins/telemetry_collection_manager/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export interface StatsCollectionConfig {
timestamp: number;
esClient: ElasticsearchClient;
soClient: SavedObjectsClientContract | ISavedObjectsRepository;
kibanaRequest: KibanaRequest | undefined; // intentionally `| undefined` to enforce providing the parameter
}

export interface BasicStatsPayload {
Expand Down
10 changes: 5 additions & 5 deletions src/plugins/usage_collection/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ Then you need to make the Telemetry service aware of the collector by registerin
```ts
// server/plugin.ts
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { CoreSetup, CoreStart } from 'kibana/server';
import { CoreSetup, CoreStart } from 'src/core/server';

class Plugin {
public setup(core: CoreSetup, plugins: { usageCollection?: UsageCollectionSetup }) {
Expand All @@ -46,7 +46,7 @@ Then you need to make the Telemetry service aware of the collector by registerin
```ts
// server/collectors/register.ts
import { UsageCollectionSetup, CollectorFetchContext } from 'src/plugins/usage_collection/server';
import { APICluster } from 'kibana/server';
import { APICluster } from 'src/core/server';

interface Usage {
my_objects: {
Expand Down Expand Up @@ -95,8 +95,8 @@ Some background:

- `isReady` (added in v7.2.0 and v6.8.4) is a way for a usage collector to announce that some async process must finish first before it can return data in the `fetch` method (e.g. a client needs to ne initialized, or the task manager needs to run a task first). If any collector reports that it is not ready when we call its `fetch` method, we reset a flag to try again and, after a set amount of time, collect data from those collectors that are ready and skip any that are not. This means that if a collector returns `true` for `isReady` and it actually isn't ready to return data, there won't be telemetry data from that collector in that telemetry report (usually once per day). You should consider what it means if your collector doesn't return data in the first few documents when Kibana starts or, if we should wait for any other reason (e.g. the task manager needs to run your task first). If you need to tell telemetry collection to wait, you should implement this function with custom logic. If your `fetch` method can run without the need of any previous dependencies, then you can return true for `isReady` as shown in the example below.

- The `fetch` method needs to support multiple contexts in which it is called. For example, when stats are pulled from a Kibana Metricbeat module, the Beat calls Kibana's stats API to invoke usage collection.
In this case, the `fetch` method is called as a result of an HTTP API request and `callCluster` wraps `callWithRequest` or `esClient` wraps `asCurrentUser`, where the request headers are expected to have read privilege on the entire `.kibana' index. The `fetch` method also exposes the saved objects client that will have the correct scope when the collectors' `fetch` method is called.
- The `fetch` method needs to support multiple contexts in which it is called. For example, when a user requests the example of what we collect in the **Kibana>Advanced Settings>Usage data** section, the clients provided in the context of the function (`CollectorFetchContext`) are scoped to that user's privileges. The reason is to avoid exposing via telemetry any data that user should not have access to (i.e.: if the user does not have access to certain indices, they shouldn't be allowed to see the number of documents that exists in it). In this case, the `fetch` method receives the clients `callCluster`, `esClient` and `soClient` scoped to the user who performed the HTTP API request. Alternatively, when requesting the usage data to be reported to the Remote Telemetry Service, the clients are scoped to the internal Kibana user (`kibana_system`). Please, mind it might have lower-level access than the default super-admin `elastic` test user.
In some scenarios, your collector might need to maintain its own client. An example of that is the `monitoring` plugin, that maintains a connection to the Remote Monitoring Cluster to push its monitoring data. If that's the case, your plugin can opt-in to receive the additional `kibanaRequest` parameter by adding `extendFetchContext.kibanaRequest: true` to the collector's config: it will be appended to the context of the `fetch` method only if the request needs to be scoped to a user other than Kibana Internal, so beware that your collector will need to work for both scenarios (especially for the scenario when `kibanaRequest` is missing).

Note: there will be many cases where you won't need to use the `callCluster`, `esClient` or `soClient` function that gets passed in to your `fetch` method at all. Your feature might have an accumulating value in server memory, or read something from the OS.

Expand All @@ -105,7 +105,7 @@ In the case of using a custom SavedObjects client, it is up to the plugin to ini
```ts
// server/plugin.ts
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { CoreSetup, CoreStart } from 'kibana/server';
import { CoreSetup, CoreStart } from 'src/core/server';

class Plugin {
private savedObjectsRepository?: ISavedObjectsRepository;
Expand Down
Loading

0 comments on commit 484437f

Please sign in to comment.