Skip to content

Commit

Permalink
Address PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
ogupte committed Nov 12, 2019
1 parent 3b55d11 commit b3a4c08
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 32 deletions.
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/apm/index.ts
Expand Up @@ -48,7 +48,7 @@ export const apm: LegacyPluginInitializer = kibana => {
},
hacks: ['plugins/apm/hacks/toggle_app_link_in_nav'],
savedObjectSchemas: {
'apm-telemetry': {
'apm-services-telemetry': {
isNamespaceAgnostic: true
},
'apm-indices': {
Expand Down
Expand Up @@ -8,7 +8,8 @@ import { SavedObjectAttributes } from 'src/core/server';
import {
APM_TELEMETRY_DOC_ID,
createApmTelementry,
storeApmTelemetry
storeApmServicesTelemetry,
APM_TELEMETRY_SAVED_OBJ_TYPE
} from '../index';

describe('apm_telemetry', () => {
Expand Down Expand Up @@ -44,7 +45,7 @@ describe('apm_telemetry', () => {
});
});

describe('storeApmTelemetry', () => {
describe('storeApmServicesTelemetry', () => {
let server: any;
let apmTelemetry: SavedObjectAttributes;
let savedObjectsClientInstance: any;
Expand Down Expand Up @@ -75,24 +76,24 @@ describe('apm_telemetry', () => {
});

it('should call savedObjectsClient create with the given ApmTelemetry object', () => {
storeApmTelemetry(server, apmTelemetry);
storeApmServicesTelemetry(server, apmTelemetry);
expect(savedObjectsClientInstance.create.mock.calls[0][1]).toBe(
apmTelemetry
);
});

it('should call savedObjectsClient create with the apm-telemetry document type and ID', () => {
storeApmTelemetry(server, apmTelemetry);
storeApmServicesTelemetry(server, apmTelemetry);
expect(savedObjectsClientInstance.create.mock.calls[0][0]).toBe(
'apm-telemetry'
APM_TELEMETRY_SAVED_OBJ_TYPE
);
expect(savedObjectsClientInstance.create.mock.calls[0][2].id).toBe(
APM_TELEMETRY_DOC_ID
);
});

it('should call savedObjectsClient create with overwrite: true', () => {
storeApmTelemetry(server, apmTelemetry);
storeApmServicesTelemetry(server, apmTelemetry);
expect(savedObjectsClientInstance.create.mock.calls[0][2].overwrite).toBe(
true
);
Expand Down
25 changes: 15 additions & 10 deletions x-pack/legacy/plugins/apm/server/lib/apm_telemetry/index.ts
Expand Up @@ -9,8 +9,9 @@ import { countBy } from 'lodash';
import { SavedObjectAttributes } from 'src/core/server';
import { InternalCoreSetup } from 'src/core/server';
import { isAgentName } from '../../../common/agent_name';
import { getSavedObjectsClient } from '../helpers/saved_objects_client';
import { getInternalSavedObjectsClient } from '../helpers/saved_objects_client';
export const APM_TELEMETRY_DOC_ID = 'apm-telemetry';
export const APM_TELEMETRY_SAVED_OBJ_TYPE = 'apm-telemetry';

export function createApmTelementry(
agentNames: string[] = []
Expand All @@ -22,16 +23,20 @@ export function createApmTelementry(
};
}

export async function storeApmTelemetry(
export async function storeApmServicesTelemetry(
server: Server,
apmTelemetry: SavedObjectAttributes
) {
try {
const savedObjectsClient = getSavedObjectsClient(server);
await savedObjectsClient.create('apm-telemetry', apmTelemetry, {
id: APM_TELEMETRY_DOC_ID,
overwrite: true
});
const internalSavedObjectsClient = getInternalSavedObjectsClient(server);
await internalSavedObjectsClient.create(
APM_TELEMETRY_SAVED_OBJ_TYPE,
apmTelemetry,
{
id: APM_TELEMETRY_DOC_ID,
overwrite: true
}
);
} catch (e) {
server.log(['error'], `Unable to save APM telemetry data: ${e.message}`);
}
Expand All @@ -43,10 +48,10 @@ export function makeApmUsageCollector(core: InternalCoreSetup) {
const apmUsageCollector = server.usage.collectorSet.makeUsageCollector({
type: 'apm',
fetch: async () => {
const savedObjectsClient = getSavedObjectsClient(server);
const internalSavedObjectsClient = getInternalSavedObjectsClient(server);
try {
const apmTelemetrySavedObject = await savedObjectsClient.get(
'apm-telemetry',
const apmTelemetrySavedObject = await internalSavedObjectsClient.get(
APM_TELEMETRY_SAVED_OBJ_TYPE,
APM_TELEMETRY_DOC_ID
);
return apmTelemetrySavedObject.attributes;
Expand Down
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { getSavedObjectsClient } from './saved_objects_client';
import { getInternalSavedObjectsClient } from './saved_objects_client';

describe('saved_objects/client', () => {
describe('getSavedObjectsClient', () => {
Expand All @@ -31,25 +31,25 @@ describe('saved_objects/client', () => {
});

it('should use internal user "admin"', () => {
getSavedObjectsClient(server);
getInternalSavedObjectsClient(server);

expect(server.plugins.elasticsearch.getCluster).toHaveBeenCalledWith(
'admin'
);
});

it('should call getSavedObjectsRepository with a cluster using the internal user context', () => {
getSavedObjectsClient(server);
getInternalSavedObjectsClient(server);

expect(
server.savedObjects.getSavedObjectsRepository
).toHaveBeenCalledWith(callWithInternalUser);
});

it('should return a SavedObjectsClient initialized with the saved objects internal repository', () => {
const result = getSavedObjectsClient(server);
const internalSavedObjectsClient = getInternalSavedObjectsClient(server);

expect(result).toBe(savedObjectsClientInstance);
expect(internalSavedObjectsClient).toBe(savedObjectsClientInstance);
expect(server.savedObjects.SavedObjectsClient).toHaveBeenCalledWith(
internalRepository
);
Expand Down
Expand Up @@ -6,7 +6,10 @@

import { Server } from 'hapi';

export function getSavedObjectsClient(server: Server, clusterName = 'admin') {
export function getInternalSavedObjectsClient(
server: Server,
clusterName = 'admin'
) {
const { SavedObjectsClient, getSavedObjectsRepository } = server.savedObjects;
const { callWithInternalUser } = server.plugins.elasticsearch.getCluster(
clusterName
Expand Down
11 changes: 7 additions & 4 deletions x-pack/legacy/plugins/apm/server/lib/index_pattern/index.ts
Expand Up @@ -4,18 +4,21 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { Server } from 'hapi';
import { getSavedObjectsClient } from '../helpers/saved_objects_client';
import { getInternalSavedObjectsClient } from '../helpers/saved_objects_client';
import apmIndexPattern from '../../../../../../../src/legacy/core_plugins/kibana/server/tutorials/apm/index_pattern.json';

export async function getAPMIndexPattern(server: Server) {
const config = server.config();
const apmIndexPatternTitle = config.get('apm_oss.indexPattern');
const savedObjectsClient = getSavedObjectsClient(server);
const internalSavedObjectsClient = getInternalSavedObjectsClient(server);
try {
return await savedObjectsClient.get('index-pattern', apmIndexPattern.id);
return await internalSavedObjectsClient.get(
'index-pattern',
apmIndexPattern.id
);
} catch (error) {
// if GET fails, then create a new index pattern saved object
return await savedObjectsClient.create(
return await internalSavedObjectsClient.create(
'index-pattern',
{
...apmIndexPattern.attributes,
Expand Down
Expand Up @@ -7,16 +7,19 @@
import { InternalCoreSetup } from 'src/core/server';
import { CallCluster } from '../../../../../../../../src/legacy/core_plugins/elasticsearch';
import { getApmIndices } from '../apm_indices/get_apm_indices';
import { getSavedObjectsClient } from '../../helpers/saved_objects_client';
import { getInternalSavedObjectsClient } from '../../helpers/saved_objects_client';

export async function createApmAgentConfigurationIndex(
core: InternalCoreSetup
) {
try {
const { server } = core.http;
const config = server.config();
const savedObjectsClient = getSavedObjectsClient(server);
const indices = await getApmIndices({ savedObjectsClient, config });
const internalSavedObjectsClient = getInternalSavedObjectsClient(server);
const indices = await getApmIndices({
savedObjectsClient: internalSavedObjectsClient,
config
});
const index = indices['apm_oss.apmAgentConfigurationIndex'];
const { callWithInternalUser } = server.plugins.elasticsearch.getCluster(
'admin'
Expand Down
7 changes: 5 additions & 2 deletions x-pack/legacy/plugins/apm/server/routes/services.ts
Expand Up @@ -6,7 +6,10 @@

import * as t from 'io-ts';
import { AgentName } from '../../typings/es_schemas/ui/fields/Agent';
import { createApmTelementry, storeApmTelemetry } from '../lib/apm_telemetry';
import {
createApmTelementry,
storeApmServicesTelemetry
} from '../lib/apm_telemetry';
import { setupRequest } from '../lib/helpers/setup_request';
import { getServiceAgentName } from '../lib/services/get_service_agent_name';
import { getServices } from '../lib/services/get_services';
Expand All @@ -31,7 +34,7 @@ export const servicesRoute = createRoute(core => ({
({ agentName }) => agentName as AgentName
);
const apmTelemetry = createApmTelementry(agentNames);
storeApmTelemetry(server, apmTelemetry);
storeApmServicesTelemetry(server, apmTelemetry);

return services;
}
Expand Down

0 comments on commit b3a4c08

Please sign in to comment.