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

[Usage Collection] Usage collection add saved objects client to collector fetch context #80554

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d58cd0c
vega snapshot
TinaHeiligers Sep 21, 2020
9c16f82
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Sep 22, 2020
540cac0
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Sep 23, 2020
184ccac
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Sep 24, 2020
64e4718
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Sep 28, 2020
72046b8
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Sep 28, 2020
3e84011
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Sep 29, 2020
fe7a768
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Sep 30, 2020
6e7d21d
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 1, 2020
1ee9535
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 2, 2020
b7a04a9
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 2, 2020
81b20c1
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 2, 2020
2422be1
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 4, 2020
8db7eb0
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 5, 2020
ed62b62
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 5, 2020
a42583e
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 6, 2020
770cc6f
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 7, 2020
8aa0273
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 7, 2020
787875d
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 8, 2020
bfaf829
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 8, 2020
b7bcbe3
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 10, 2020
6afd633
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 12, 2020
9a7e26a
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 13, 2020
d9bef25
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 13, 2020
7209b1f
Merge branch 'master' of github.com:elastic/kibana
TinaHeiligers Oct 13, 2020
3dc502b
Adds scoped saved objects client to collector fetch context
TinaHeiligers Oct 14, 2020
fd2ccfa
Updates README, cleans up code
TinaHeiligers Oct 14, 2020
697cfe7
Merge branch 'master' of github.com:elastic/kibana into usage-collect…
TinaHeiligers Oct 14, 2020
4d2588c
Handles CollectorFetchContext changes bubbling up to the data plugin'…
TinaHeiligers Oct 14, 2020
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
1 change: 1 addition & 0 deletions src/plugins/data/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { EnvironmentMode } from '@kbn/config';
import { ErrorToastOptions } from 'src/core/public/notifications';
import { ExpressionAstFunction } from 'src/plugins/expressions/common';
import { ExpressionsServerSetup } from 'src/plugins/expressions/server';
import { ISavedObjectsRepository } from 'kibana/server';
import { ISearchOptions as ISearchOptions_2 } from 'src/plugins/data/public';
import { ISearchSource } from 'src/plugins/data/public';
import { KibanaRequest } from 'src/core/server';
Expand Down
6 changes: 5 additions & 1 deletion src/plugins/telemetry/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
Logger,
IClusterClient,
UiSettingsServiceStart,
SavedObjectsServiceStart,
} from '../../../core/server';
import { registerRoutes } from './routes';
import { registerCollection } from './telemetry_collection';
Expand Down Expand Up @@ -88,6 +89,7 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
private readonly oldUiSettingsHandled$ = new AsyncSubject();
private savedObjectsClient?: ISavedObjectsRepository;
private elasticsearchClient?: IClusterClient;
private savedObjectsService?: SavedObjectsServiceStart;

constructor(initializerContext: PluginInitializerContext<TelemetryConfigType>) {
this.logger = initializerContext.logger.get();
Expand All @@ -110,7 +112,8 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
registerCollection(
telemetryCollectionManager,
elasticsearch.legacy.client,
() => this.elasticsearchClient
() => this.elasticsearchClient,
() => this.savedObjectsService
);
const router = http.createRouter();

Expand Down Expand Up @@ -139,6 +142,7 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
const savedObjectsInternalRepository = savedObjects.createInternalRepository();
this.savedObjectsClient = savedObjectsInternalRepository;
this.elasticsearchClient = elasticsearch.client;
this.savedObjectsService = savedObjects;

// Not catching nor awaiting these promises because they should never reject
this.handleOldUiSettings(uiSettings);
Expand Down
11 changes: 8 additions & 3 deletions src/plugins/telemetry/server/telemetry_collection/get_kibana.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@

import { omit } from 'lodash';
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { LegacyAPICaller } from 'kibana/server';
import {
ISavedObjectsRepository,
LegacyAPICaller,
SavedObjectsClientContract,
} from 'kibana/server';
import { StatsCollectionContext } from 'src/plugins/telemetry_collection_manager/server';
import { ElasticsearchClient } from 'src/core/server';

Expand Down Expand Up @@ -84,8 +88,9 @@ export function handleKibanaStats(
export async function getKibana(
usageCollection: UsageCollectionSetup,
callWithInternalUser: LegacyAPICaller,
asInternalUser: ElasticsearchClient
asInternalUser: ElasticsearchClient,
soClient: SavedObjectsClientContract | ISavedObjectsRepository
): Promise<KibanaUsageStats> {
const usage = await usageCollection.bulkFetch(callWithInternalUser, asInternalUser);
const usage = await usageCollection.bulkFetch(callWithInternalUser, asInternalUser, soClient);
return usageCollection.toObject(usage);
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
import { merge, omit } from 'lodash';

import { getLocalStats, handleLocalStats } from './get_local_stats';
import { usageCollectionPluginMock } from '../../../usage_collection/server/mocks';
import {
usageCollectionPluginMock,
createCollectorFetchContextMock,
} from '../../../usage_collection/server/mocks';
import { elasticsearchServiceMock } from '../../../../../src/core/server/mocks';

function mockUsageCollection(kibanaUsage = {}) {
Expand Down Expand Up @@ -79,6 +82,16 @@ function mockGetLocalStats(clusterInfo: any, clusterStats: any) {
return esClient;
}

function mockStatsCollectionConfig(clusterInfo: any, clusterStats: any, kibana: {}) {
return {
...createCollectorFetchContextMock(),
esClient: mockGetLocalStats(clusterInfo, clusterStats),
usageCollection: mockUsageCollection(kibana),
start: '',
end: '',
};
}

describe('get_local_stats', () => {
const clusterUuid = 'abc123';
const clusterName = 'my-cool-cluster';
Expand Down Expand Up @@ -224,12 +237,10 @@ describe('get_local_stats', () => {

describe('getLocalStats', () => {
it('returns expected object with kibana data', async () => {
const callCluster = jest.fn();
const usageCollection = mockUsageCollection(kibana);
const esClient = mockGetLocalStats(clusterInfo, clusterStats);
const statsCollectionConfig = mockStatsCollectionConfig(clusterInfo, clusterStats, kibana);
const response = await getLocalStats(
[{ clusterUuid: 'abc123' }],
{ callCluster, usageCollection, esClient, start: '', end: '' },
{ ...statsCollectionConfig },
context
);
const result = response[0];
Expand All @@ -244,14 +255,8 @@ describe('get_local_stats', () => {
});

it('returns an empty array when no cluster uuid is provided', async () => {
const callCluster = jest.fn();
const usageCollection = mockUsageCollection(kibana);
const esClient = mockGetLocalStats(clusterInfo, clusterStats);
const response = await getLocalStats(
[],
{ callCluster, usageCollection, esClient, start: '', end: '' },
context
);
const statsCollectionConfig = mockStatsCollectionConfig(clusterInfo, clusterStats, kibana);
const response = await getLocalStats([], { ...statsCollectionConfig }, context);
expect(response).toBeDefined();
expect(response.length).toEqual(0);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,18 +68,18 @@ export type TelemetryLocalStats = ReturnType<typeof handleLocalStats>;
*/
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
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)
) => {
const { callCluster, usageCollection, esClient } = config;
const { callCluster, usageCollection, esClient, soClient } = 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),
getKibana(usageCollection, callCluster, esClient, soClient),
getDataTelemetry(esClient),
]);
return handleLocalStats(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
* under the License.
*/

import { ILegacyClusterClient } from 'kibana/server';
import { ILegacyClusterClient, SavedObjectsServiceStart } from 'kibana/server';
import { TelemetryCollectionManagerPluginSetup } from 'src/plugins/telemetry_collection_manager/server';
import { IClusterClient } from '../../../../../src/core/server';
import { getLocalStats } from './get_local_stats';
Expand All @@ -46,11 +46,13 @@ import { getLocalLicense } from './get_local_license';
export function registerCollection(
telemetryCollectionManager: TelemetryCollectionManagerPluginSetup,
esCluster: ILegacyClusterClient,
esClientGetter: () => IClusterClient | undefined
esClientGetter: () => IClusterClient | undefined,
soServiceGetter: () => SavedObjectsServiceStart | undefined
) {
telemetryCollectionManager.setCollection({
esCluster,
esClientGetter,
soServiceGetter,
title: 'local',
priority: 0,
statsGetter: getLocalStats,
Expand Down
21 changes: 18 additions & 3 deletions src/plugins/telemetry_collection_manager/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
Plugin,
Logger,
IClusterClient,
SavedObjectsServiceStart,
} from '../../../core/server';

import {
Expand Down Expand Up @@ -90,6 +91,7 @@ export class TelemetryCollectionManagerPlugin
priority,
esCluster,
esClientGetter,
soServiceGetter,
statsGetter,
clusterDetailsGetter,
licenseGetter,
Expand All @@ -112,6 +114,9 @@ export class TelemetryCollectionManagerPlugin
if (!esClientGetter) {
throw Error('esClientGetter method not set.');
}
if (!soServiceGetter) {
throw Error('soServiceGetter method not set.');
}
if (!clusterDetailsGetter) {
throw Error('Cluster UUIds method is not set.');
}
Expand All @@ -126,6 +131,7 @@ export class TelemetryCollectionManagerPlugin
esCluster,
title,
esClientGetter,
soServiceGetter,
});
this.usageGetterMethodPriority = priority;
}
Expand All @@ -135,6 +141,7 @@ export class TelemetryCollectionManagerPlugin
config: StatsGetterConfig,
collection: Collection,
collectionEsClient: IClusterClient,
collectionSoService: SavedObjectsServiceStart,
usageCollection: UsageCollectionSetup
): StatsCollectionConfig {
const { start, end, request } = config;
Expand All @@ -146,7 +153,11 @@ export class TelemetryCollectionManagerPlugin
const esClient = config.unencrypted
? collectionEsClient.asScoped(config.request).asCurrentUser
: collectionEsClient.asInternalUser;
return { callCluster, start, end, usageCollection, esClient };
// Scope the saved objects client appropriately and pass to the stats collection config
const soClient = config.unencrypted
? collectionSoService.getScopedClient(config.request)
: collectionSoService.createInternalRepository();
return { callCluster, start, end, usageCollection, esClient, soClient };
}

private async getOptInStats(optInStatus: boolean, config: StatsGetterConfig) {
Expand All @@ -156,11 +167,13 @@ export class TelemetryCollectionManagerPlugin
for (const collection of this.collections) {
// first fetch the client and make sure it's not undefined.
const collectionEsClient = collection.esClientGetter();
if (collectionEsClient !== undefined) {
const collectionSoService = collection.soServiceGetter();
if (collectionEsClient !== undefined && collectionSoService !== undefined) {
const statsCollectionConfig = this.getStatsCollectionConfig(
config,
collection,
collectionEsClient,
collectionSoService,
this.usageCollection
);

Expand Down Expand Up @@ -215,11 +228,13 @@ export class TelemetryCollectionManagerPlugin
}
for (const collection of this.collections) {
const collectionEsClient = collection.esClientGetter();
if (collectionEsClient !== undefined) {
const collectionSavedObjectsService = collection.soServiceGetter();
if (collectionEsClient !== undefined && collectionSavedObjectsService !== undefined) {
const statsCollectionConfig = this.getStatsCollectionConfig(
config,
collection,
collectionEsClient,
collectionSavedObjectsService,
this.usageCollection
);
try {
Expand Down
6 changes: 6 additions & 0 deletions src/plugins/telemetry_collection_manager/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import {
KibanaRequest,
ILegacyClusterClient,
IClusterClient,
SavedObjectsServiceStart,
SavedObjectsClientContract,
ISavedObjectsRepository,
} from 'kibana/server';
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { ElasticsearchClient } from '../../../../src/core/server';
Expand Down Expand Up @@ -77,6 +80,7 @@ export interface StatsCollectionConfig {
start: string | number;
end: string | number;
esClient: ElasticsearchClient;
soClient: SavedObjectsClientContract | ISavedObjectsRepository;
}

export interface BasicStatsPayload {
Expand Down Expand Up @@ -141,6 +145,7 @@ export interface CollectionConfig<
priority: number;
esCluster: ILegacyClusterClient;
esClientGetter: () => IClusterClient | undefined; // --> by now we know that the client getter will return the IClusterClient but we assure that through a code check
soServiceGetter: () => SavedObjectsServiceStart | undefined; // --> by now we know that the service getter will return the SavedObjectsServiceStart but we assure that through a code check
statsGetter: StatsGetter<CustomContext, T>;
clusterDetailsGetter: ClusterDetailsGetter<CustomContext>;
licenseGetter: LicenseGetter<CustomContext>;
Expand All @@ -157,5 +162,6 @@ export interface Collection<
clusterDetailsGetter: ClusterDetailsGetter<CustomContext>;
esCluster: ILegacyClusterClient;
esClientGetter: () => IClusterClient | undefined; // the collection could still return undefined for the es client getter.
soServiceGetter: () => SavedObjectsServiceStart | undefined; // the collection could still return undefined for the Saved Objects Service getter.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

irrelated to PR: in what cases it would return undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The saved objects client is only available during the start cycle, so if it's called before it's initialized, we'll return undefined. It's the same as for the new elasticsearch client.

title: string;
}
10 changes: 6 additions & 4 deletions src/plugins/usage_collection/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ All you need to provide is a `type` for organizing your fields, `schema` field t
},
fetch: async (collectorFetchContext: CollectorFetchContext) => {

// query ES and get some data
// query ES or saved objects and get some data
// summarize the data into a model
// return the modeled object that includes whatever you want to track

Expand All @@ -85,9 +85,11 @@ Some background:

- `MY_USAGE_TYPE` can be any string. It usually matches the plugin name. As a safety mechanism, we double check there are no duplicates at the moment of registering the collector.
- 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.
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.

Note: there will be many cases where you won't need to use the `callCluster` (or `esClient`) 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, or use other clients like a custom SavedObjects client. In that case it's up to the plugin to initialize those clients like the example below:
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.

In the case of using a custom SavedObjects client, it is up to the plugin to initialize the client to save the data and it is strongly recommended to scope that client to the `kibana_system` user.

```ts
// server/plugin.ts
Expand All @@ -98,7 +100,7 @@ class Plugin {
private savedObjectsRepository?: ISavedObjectsRepository;

public setup(core: CoreSetup, plugins: { usageCollection?: UsageCollectionSetup }) {
registerMyPluginUsageCollector(() => this.savedObjectsRepository, plugins.usageCollection);
registerMyPluginUsageCollector(plugins.usageCollection);
}

public start(core: CoreStart) {
Expand Down
15 changes: 14 additions & 1 deletion src/plugins/usage_collection/server/collector/collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@
* under the License.
*/

import { Logger, LegacyAPICaller, ElasticsearchClient } from 'kibana/server';
import {
Logger,
LegacyAPICaller,
ElasticsearchClient,
ISavedObjectsRepository,
SavedObjectsClientContract,
} from 'kibana/server';

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

Expand Down Expand Up @@ -56,7 +62,14 @@ export interface CollectorFetchContext {
* - When building the telemetry data payload to report to the remote cluster, the requests are scoped to the `kibana` internal user
*/
esClient: ElasticsearchClient;
/**
* Request-scoped Saved Objects client:
* - When users are requesting a sample of data, it is scoped to their role to avoid exposing data they should't read
* - When building the telemetry data payload to report to the remote cluster, the requests are scoped to the `kibana` internal user
*/
soClient: SavedObjectsClientContract | ISavedObjectsRepository;
}

export interface CollectorOptions<T = unknown, U = T> {
type: string;
init?: Function;
Expand Down
Loading