Skip to content

Commit

Permalink
[APM] Address fallout from NP server migration (#51725) (#52116)
Browse files Browse the repository at this point in the history
* [APM] Address fallout from NP server migration

* Fix type issues in integration tests

* Await creation of agent configuration index
  • Loading branch information
dgieselaar authored Dec 10, 2019
1 parent 1c9c991 commit 3a53d1e
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 39 deletions.
13 changes: 3 additions & 10 deletions x-pack/legacy/plugins/apm/public/services/rest/callApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,11 @@ function fetchOptionsWithDebug(fetchOptions: FetchOptions) {
sessionStorage.getItem('apm_debug') === 'true' &&
startsWith(fetchOptions.pathname, '/api/apm');

const isGet = !fetchOptions.method || fetchOptions.method === 'GET';

// Need an empty body to pass route validation
const body = isGet
? {}
: {
body: JSON.stringify(fetchOptions.body || {})
};
const { body, ...rest } = fetchOptions;

return {
...fetchOptions,
...body,
...rest,
...(body !== undefined ? { body: JSON.stringify(body) } : {}),
query: {
...fetchOptions.query,
...(debugEnabled ? { _debug: true } : {})
Expand Down
4 changes: 2 additions & 2 deletions x-pack/legacy/plugins/apm/public/services/rest/ml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export async function startMLJob({
return callApi<StartedMLJobApiResponse>(http, {
method: 'POST',
pathname: `/api/ml/modules/setup/apm_transaction`,
body: JSON.stringify({
body: {
prefix: getMlPrefix(serviceName, transactionType),
groups,
indexPatternName: transactionIndices,
Expand All @@ -71,7 +71,7 @@ export async function startMLJob({
filter
}
}
})
}
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ describe('createApi', () => {
// stub default values
params: {},
query: {},
body: {},
body: null,
...requestMock
},
responseMock
Expand All @@ -144,7 +144,7 @@ describe('createApi', () => {
it('adds a _debug query parameter by default', async () => {
const { simulate, handlerMock, responseMock } = initApi({});

await simulate({ query: { _debug: true } });
await simulate({ query: { _debug: 'true' } });

expect(handlerMock).toHaveBeenCalledTimes(1);

Expand Down Expand Up @@ -288,7 +288,7 @@ describe('createApi', () => {
await simulate({
query: {
bar: '',
_debug: true
_debug: 'true'
}
});

Expand Down
39 changes: 27 additions & 12 deletions x-pack/legacy/plugins/apm/server/routes/create_api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ import {
Route,
Params
} from '../typings';
import { jsonRt } from '../../../common/runtime_types/json_rt';

const debugRt = t.partial({ _debug: t.boolean });
const debugRt = t.partial({ _debug: jsonRt.pipe(t.boolean) });

export function createApi() {
const factoryFns: Array<RouteFactoryFn<any, any, any, any>> = [];
Expand Down Expand Up @@ -53,28 +54,37 @@ export function createApi() {
| 'get'
| 'delete';

const bodyRt = params.body;
const fallbackBodyRt = bodyRt || t.strict({});
// For all runtime types with props, we create an exact
// version that will strip all keys that are unvalidated.

const bodyRt =
params.body && 'props' in params.body
? t.exact(params.body)
: params.body;

const rts = {
// add _debug query parameter to all routes
// Add _debug query parameter to all routes
query: params.query
? t.exact(t.intersection([params.query, debugRt]))
: t.exact(debugRt),
path: params.path ? t.exact(params.path) : t.strict({}),
body: bodyRt && 'props' in bodyRt ? t.exact(bodyRt) : fallbackBodyRt
body: bodyRt || t.null
};

const anyObject = schema.object({}, { allowUnknowns: true });

(router[routerMethod] as RouteRegistrar<typeof routerMethod>)(
{
path,
options,
validate: {
...(routerMethod === 'get'
? {}
: { body: schema.object({}, { allowUnknowns: true }) }),
params: schema.object({}, { allowUnknowns: true }),
query: schema.object({}, { allowUnknowns: true })
// `body` can be null, but `validate` expects non-nullable types
// if any validation is defined. Not having validation currently
// means we don't get the payload. See
// https://github.com/elastic/kibana/issues/50179
body: schema.nullable(anyObject) as typeof anyObject,
params: anyObject,
query: anyObject
}
},
async (context, request, response) => {
Expand All @@ -83,7 +93,7 @@ export function createApi() {
path: request.params,
body: request.body,
query: {
_debug: false,
_debug: 'false',
...request.query
}
};
Expand All @@ -100,6 +110,9 @@ export function createApi() {
throw Boom.badRequest(PathReporter.report(result)[0]);
}

// `io-ts` has stripped unvalidated keys, so we can compare
// the output with the input to see if all object keys are
// known and validated.
const strippedKeys = difference(
Object.keys(value || {}),
Object.keys(result.right || {})
Expand All @@ -124,7 +137,9 @@ export function createApi() {
context: {
...context,
__LEGACY,
// only return values for parameters that have runtime types
// Only return values for parameters that have runtime types,
// but always include query as _debug is always set even if
// it's not defined in the route.
params: pick(parsedParams, ...Object.keys(params), 'query'),
config,
logger
Expand Down
21 changes: 12 additions & 9 deletions x-pack/plugins/apm/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export class APMPlugin implements Plugin<APMPluginContract> {
this.legacySetup$ = new AsyncSubject();
}

public setup(
public async setup(
core: CoreSetup,
plugins: {
apm_oss: APMOSSPlugin extends Plugin<infer TSetup> ? TSetup : never;
Expand All @@ -46,14 +46,17 @@ export class APMPlugin implements Plugin<APMPluginContract> {
createApmApi().init(core, { config$: mergedConfig$, logger, __LEGACY });
});

combineLatest(mergedConfig$, core.elasticsearch.dataClient$).subscribe(
([config, dataClient]) => {
createApmAgentConfigurationIndex({
esClient: dataClient,
config,
});
}
);
await new Promise(resolve => {
combineLatest(mergedConfig$, core.elasticsearch.dataClient$).subscribe(
async ([config, dataClient]) => {
await createApmAgentConfigurationIndex({
esClient: dataClient,
config,
});
resolve();
}
);
});

return {
config$: mergedConfig$,
Expand Down
43 changes: 41 additions & 2 deletions x-pack/test/api_integration/apis/apm/agent_configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,56 @@ export default function agentConfigurationTests({ getService }: FtrProviderConte

function deleteCreatedConfigurations() {
const promises = Promise.all(createdConfigIds.map(deleteConfiguration));
createdConfigIds = [];
return promises;
}

function deleteConfiguration(configurationId: string) {
return supertest
.delete(`/api/apm/settings/agent-configuration/${configurationId}`)
.set('kbn-xsrf', 'foo');
.set('kbn-xsrf', 'foo')
.then((response: any) => {
createdConfigIds = createdConfigIds.filter(id => id === configurationId);
return response;
});
}

describe('agent configuration', () => {
describe('when creating one configuration', () => {
let createdConfigId: string;

const parameters = {
service: { name: 'myservice', environment: 'development' },
etag: '7312bdcc34999629a3d39df24ed9b2a7553c0c39',
};

before(async () => {
log.debug('creating agent configuration');

// all / all
const { body } = await createConfiguration({
service: {},
settings: { transaction_sample_rate: 0.1 },
});

createdConfigId = body._id;
});

it('returns the created configuration', async () => {
const { statusCode, body } = await searchConfigurations(parameters);

expect(statusCode).to.equal(200);
expect(body._id).to.equal(createdConfigId);
});

it('succesfully deletes the configuration', async () => {
await deleteConfiguration(createdConfigId);

const { statusCode } = await searchConfigurations(parameters);

expect(statusCode).to.equal(404);
});
});

describe('when creating four configurations', () => {
before(async () => {
log.debug('creating agent configuration');
Expand Down
5 changes: 4 additions & 1 deletion x-pack/test/api_integration/apis/apm/feature_controls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ export default function featureControlsTests({ getService }: FtrProviderContext)
}
const endpoints: Endpoint[] = [
{
req: { url: `/api/apm/services/foo/errors?start=${start}&end=${end}&uiFilters=%7B%7D` },
// this doubles as a smoke test for the _debug query parameter
req: {
url: `/api/apm/services/foo/errors?start=${start}&end=${end}&uiFilters=%7B%7D_debug=true`,
},
expectForbidden: expect404,
expectResponse: expect200,
},
Expand Down

0 comments on commit 3a53d1e

Please sign in to comment.