maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[APM] Fix missing ML data, and NaN issue (#34333)

Browse source
This commit is contained in:
Søren Louv-Jansen 2019-04-02 16:39:19 +02:00 committed by GitHub
parent 2bba68f858
commit 949a267672
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 268 additions and 101 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

230
x-pack/plugins/apm/server/lib/helpers/setup_request.test.ts
View file

@ -4,35 +4,35 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { setupRequest } from './setup_request';
import { Legacy } from 'kibana';
import { isApmIndex, setupRequest } from './setup_request';
function getMockRequest() {
const callWithRequestSpy = jest.fn();
const mockRequest = ({
params: {},
query: {},
server: {
config: () => ({ get: () => 'apm-*' }),
plugins: {
elasticsearch: {
getCluster: () => ({ callWithRequest: callWithRequestSpy })
}
}
},
getUiSettingsService: () => ({ get: async () => false })
} as any) as Legacy.Request;
return { callWithRequestSpy, mockRequest };
}
describe('setupRequest', () => {
let callWithRequestSpy: jest.Mock;
let mockReq: any;
beforeEach(() => {
callWithRequestSpy = jest.fn();
mockReq = {
params: {},
query: {},
server: {
config: () => 'myConfig',
plugins: {
elasticsearch: {
getCluster: () => ({ callWithRequest: callWithRequestSpy })
}
}
},
getUiSettingsService: jest.fn(() => ({
get: jest.fn(() => Promise.resolve(false))
}))
};
});
it('should call callWithRequest with default args', async () => {
const setup = setupRequest(mockReq);
await setup.client('myType', { body: { foo: 'bar' } });
expect(callWithRequestSpy).toHaveBeenCalledWith(mockReq, 'myType', {
const { mockRequest, callWithRequestSpy } = getMockRequest();
const setup = setupRequest(mockRequest);
await setup.client('myType', { index: 'apm-*', body: { foo: 'bar' } });
expect(callWithRequestSpy).toHaveBeenCalledWith(mockRequest, 'myType', {
index: 'apm-*',
body: {
foo: 'bar',
query: {
@ -47,51 +47,82 @@ describe('setupRequest', () => {
});
describe('omitLegacyData', () => {
it('should add `observer.version_major` filter if `omitLegacyData=true` ', async () => {
const setup = setupRequest(mockReq);
await setup.client('myType', {
omitLegacyData: true,
body: { query: { bool: { filter: [{ term: 'someTerm' }] } } }
});
expect(callWithRequestSpy.mock.calls[0][2].body).toEqual({
query: {
bool: {
filter: [
{ term: 'someTerm' },
{ range: { 'observer.version_major': { gte: 7 } } }
]
describe('if index is apm-*', () => {
it('should add `observer.version_major` filter if `omitLegacyData=true` ', async () => {
const { mockRequest, callWithRequestSpy } = getMockRequest();
const setup = setupRequest(mockRequest);
await setup.client('myType', {
index: 'apm-*',
omitLegacyData: true,
body: { query: { bool: { filter: [{ term: 'someTerm' }] } } }
});
expect(callWithRequestSpy.mock.calls[0][2].body).toEqual({
query: {
bool: {
filter: [
{ term: 'someTerm' },
{ range: { 'observer.version_major': { gte: 7 } } }
]
}
}
}
});
});
});
it('should not add `observer.version_major` filter if `omitLegacyData=false` ', async () => {
const setup = setupRequest(mockReq);
await setup.client('myType', {
omitLegacyData: false,
body: { query: { bool: { filter: [{ term: 'someTerm' }] } } }
it('should not add `observer.version_major` filter if `omitLegacyData=false` ', async () => {
const { mockRequest, callWithRequestSpy } = getMockRequest();
const setup = setupRequest(mockRequest);
await setup.client('myType', {
index: 'apm-*',
omitLegacyData: false,
body: { query: { bool: { filter: [{ term: 'someTerm' }] } } }
});
expect(callWithRequestSpy.mock.calls[0][2].body).toEqual({
query: { bool: { filter: [{ term: 'someTerm' }] } }
});
});
expect(callWithRequestSpy.mock.calls[0][2].body).toEqual({
query: { bool: { filter: [{ term: 'someTerm' }] } }
});
});
it('should set filter if none exists', async () => {
const setup = setupRequest(mockReq);
await setup.client('myType', {});
const params = callWithRequestSpy.mock.calls[0][2];
expect(params.body).toEqual({
query: {
bool: {
filter: [{ range: { 'observer.version_major': { gte: 7 } } }]
it('should add `observer.version_major` filter if none exists', async () => {
const { mockRequest, callWithRequestSpy } = getMockRequest();
const setup = setupRequest(mockRequest);
await setup.client('myType', { index: 'apm-*' });
const params = callWithRequestSpy.mock.calls[0][2];
expect(params.body).toEqual({
query: {
bool: {
filter: [{ range: { 'observer.version_major': { gte: 7 } } }]
}
}
}
});
});
it('should have `omitLegacyData=true` as default and merge boolean filters', async () => {
const { mockRequest, callWithRequestSpy } = getMockRequest();
const setup = setupRequest(mockRequest);
await setup.client('myType', {
index: 'apm-*',
body: {
query: { bool: { filter: [{ term: 'someTerm' }] } }
}
});
const params = callWithRequestSpy.mock.calls[0][2];
expect(params.body).toEqual({
query: {
bool: {
filter: [
{ term: 'someTerm' },
{ range: { 'observer.version_major': { gte: 7 } } }
]
}
}
});
});
});
it('should have `omitLegacyData=true` as default and merge boolean filters', async () => {
const setup = setupRequest(mockReq);
it('if index is not an APM index, it should not add `observer.version_major` filter ', async () => {
const { mockRequest, callWithRequestSpy } = getMockRequest();
const setup = setupRequest(mockRequest);
await setup.client('myType', {
index: '.ml-*',
body: {
query: { bool: { filter: [{ term: 'someTerm' }] } }
}
@ -100,24 +131,75 @@ describe('setupRequest', () => {
expect(params.body).toEqual({
query: {
bool: {
filter: [
{ term: 'someTerm' },
{ range: { 'observer.version_major': { gte: 7 } } }
]
filter: [{ term: 'someTerm' }]
}
}
});
});
});
it('should set ignore_throttled to false if includeFrozen is true', async () => {
// mock includeFrozen to return true
mockReq.getUiSettingsService.mockImplementation(() => ({
get: jest.fn(() => Promise.resolve(true))
}));
const setup = setupRequest(mockReq);
await setup.client('myType', {});
const params = callWithRequestSpy.mock.calls[0][2];
expect(params.ignore_throttled).toBe(false);
describe('ignore_throttled', () => {
it('should set `ignore_throttled=true` if `includeFrozen=false`', async () => {
const { mockRequest, callWithRequestSpy } = getMockRequest();
// mock includeFrozen to return false
mockRequest.getUiSettingsService = () => ({ get: async () => false });
const setup = setupRequest(mockRequest);
await setup.client('myType', {});
const params = callWithRequestSpy.mock.calls[0][2];
expect(params.ignore_throttled).toBe(true);
});
it('should set `ignore_throttled=false` if `includeFrozen=true`', async () => {
const { mockRequest, callWithRequestSpy } = getMockRequest();
// mock includeFrozen to return true
mockRequest.getUiSettingsService = () => ({ get: async () => true });
const setup = setupRequest(mockRequest);
await setup.client('myType', {});
const params = callWithRequestSpy.mock.calls[0][2];
expect(params.ignore_throttled).toBe(false);
});
});
describe('isApmIndex', () => {
const apmIndices = [
'apm-*-metric-*',
'apm-*-onboarding-*',
'apm-*-span-*',
'apm-*-transaction-*',
'apm-*-error-*'
];
describe('when indexParam is a string', () => {
it('should return true if it matches any of the items in apmIndices', () => {
const indexParam = 'apm-*-transaction-*';
expect(isApmIndex(apmIndices, indexParam)).toBe(true);
});
it('should return false if it does not match any of the items in `apmIndices`', () => {
const indexParam = '.ml-anomalies-*';
expect(isApmIndex(apmIndices, indexParam)).toBe(false);
});
});
describe('when indexParam is an array', () => {
it('should return true if all values in `indexParam` matches values in `apmIndices`', () => {
const indexParam = ['apm-*-transaction-*', 'apm-*-span-*'];
expect(isApmIndex(apmIndices, indexParam)).toBe(true);
});
it("should return false if some of the values don't match with `apmIndices`", () => {
const indexParam = ['apm-*-transaction-*', '.ml-anomalies-*'];
expect(isApmIndex(apmIndices, indexParam)).toBe(false);
});
});
describe('when indexParam is neither a string or an array', () => {
it('should return false', () => {
[true, false, undefined].forEach(indexParam => {
expect(isApmIndex(apmIndices, indexParam)).toBe(false);
});
});
});
});
});

42
x-pack/plugins/apm/server/lib/helpers/setup_request.ts
View file

@ -11,7 +11,7 @@ import {
SearchParams
} from 'elasticsearch';
import { Legacy } from 'kibana';
import { cloneDeep, has, set } from 'lodash';
import { cloneDeep, has, isString, set } from 'lodash';
import moment from 'moment';
import { OBSERVER_VERSION_MAJOR } from 'x-pack/plugins/apm/common/elasticsearch_fieldnames';
@ -43,12 +43,36 @@ interface APMRequestQuery {
esFilterQuery: string;
}
function addFilterForLegacyData({
omitLegacyData = true,
...params
}: APMSearchParams): SearchParams {
function getApmIndices(config: Legacy.KibanaConfig) {
return [
config.get<string>('apm_oss.errorIndices'),
config.get<string>('apm_oss.metricsIndices'),
config.get<string>('apm_oss.onboardingIndices'),
config.get<string>('apm_oss.sourcemapIndices'),
config.get<string>('apm_oss.spanIndices'),
config.get<string>('apm_oss.transactionIndices')
];
}
export function isApmIndex(
apmIndices: string[],
indexParam: SearchParams['index']
) {
if (isString(indexParam)) {
return apmIndices.includes(indexParam);
} else if (Array.isArray(indexParam)) {
// return false if at least one of the indices is not an APM index
return indexParam.every(index => apmIndices.includes(index));
}
return false;
}
function addFilterForLegacyData(
apmIndices: string[],
{ omitLegacyData = true, ...params }: APMSearchParams
): SearchParams {
// search across all data (including data)
if (!omitLegacyData) {
if (!omitLegacyData || !isApmIndex(apmIndices, params.index)) {
return params;
}
@ -69,12 +93,14 @@ export function setupRequest(req: Legacy.Request): Setup {
const query = (req.query as unknown) as APMRequestQuery;
const cluster = req.server.plugins.elasticsearch.getCluster('data');
const uiSettings = req.getUiSettingsService();
const config = req.server.config();
const apmIndices = getApmIndices(config);
const client: ESClient = async (type, params) => {
const includeFrozen = await uiSettings.get('search:includeFrozen');
const nextParams = {
...addFilterForLegacyData(params), // filter out pre-7.0 data
...addFilterForLegacyData(apmIndices, params), // filter out pre-7.0 data
ignore_throttled: !includeFrozen, // whether to query frozen indices or not
rest_total_hits_as_int: true // ensure that ES returns accurate hits.total with pre-6.6 format
};
@ -99,6 +125,6 @@ export function setupRequest(req: Legacy.Request): Setup {
end: moment.utc(query.end).valueOf(),
esFilterQuery: decodeEsQuery(query.esFilterQuery),
client,
config: req.server.config()
config
};
}

56
x-pack/plugins/apm/server/lib/transactions/charts/get_timeseries_data/__snapshots__/transform.test.ts.snap
View file

@ -5,6 +5,10 @@ Object {
"overallAvgDuration": 32861.15660262639,
"responseTimes": Object {
"avg": Array [
Object {
"x": 1528113600000,
"y": 26310.63483891513,
},
Object {
"x": 1528124400000,
"y": 26193.277795595466,
@ -321,8 +325,16 @@ Object {
"x": 1528966800000,
"y": 52360.30017052116,
},
Object {
"x": 1528977600000,
"y": null,
},
],
"p95": Array [
Object {
"x": 1528113600000,
"y": 82172.85648714812,
},
Object {
"x": 1528124400000,
"y": 80738.78571428556,
@ -639,8 +651,16 @@ Object {
"x": 1528966800000,
"y": 194970.75667682925,
},
Object {
"x": 1528977600000,
"y": null,
},
],
"p99": Array [
Object {
"x": 1528113600000,
"y": 293866.3866666665,
},
Object {
"x": 1528124400000,
"y": 293257.27333333343,
@ -957,12 +977,20 @@ Object {
"x": 1528966800000,
"y": 473485.4199999998,
},
Object {
"x": 1528977600000,
"y": null,
},
],
},
"totalHits": 1297673,
"tpmBuckets": Array [
Object {
"dataPoints": Array [
Object {
"x": 1528113600000,
"y": 82230,
},
Object {
"x": 1528124400000,
"y": 81460,
@ -1279,11 +1307,19 @@ Object {
"x": 1528966800000,
"y": 81845,
},
Object {
"x": 1528977600000,
"y": 0,
},
],
"key": "HTTP 2xx",
},
Object {
"dataPoints": Array [
Object {
"x": 1528113600000,
"y": 0,
},
Object {
"x": 1528124400000,
"y": 0,
@ -1600,11 +1636,19 @@ Object {
"x": 1528966800000,
"y": 0,
},
Object {
"x": 1528977600000,
"y": 0,
},
],
"key": "HTTP 3xx",
},
Object {
"dataPoints": Array [
Object {
"x": 1528113600000,
"y": 5930,
},
Object {
"x": 1528124400000,
"y": 6065,
@ -1921,11 +1965,19 @@ Object {
"x": 1528966800000,
"y": 6070,
},
Object {
"x": 1528977600000,
"y": 0,
},
],
"key": "HTTP 4xx",
},
Object {
"dataPoints": Array [
Object {
"x": 1528113600000,
"y": 6045,
},
Object {
"x": 1528124400000,
"y": 6015,
@ -2242,6 +2294,10 @@ Object {
"x": 1528966800000,
"y": 5915,
},
Object {
"x": 1528977600000,
"y": 0,
},
],
"key": "HTTP 5xx",
},

37
x-pack/plugins/apm/server/lib/transactions/charts/get_timeseries_data/transform.test.ts
View file

@ -4,7 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { first, last } from 'lodash';
import { timeseriesResponse } from './mock-responses/timeseries_response';
import {
ApmTimeSeriesResponse,
@ -21,18 +20,6 @@ describe('timeseriesTransformer', () => {
});
});
it('should not contain first and last bucket', () => {
const mockDates = timeseriesResponse.aggregations.transaction_results.buckets[0].timeseries.buckets.map(
bucket => bucket.key
);
expect(first(res.responseTimes.avg).x).not.toBe(first(mockDates));
expect(last(res.responseTimes.avg).x).not.toBe(last(mockDates));
expect(first(res.tpmBuckets[0].dataPoints).x).not.toBe(first(mockDates));
expect(last(res.tpmBuckets[0].dataPoints).x).not.toBe(last(mockDates));
});
it('should have correct order', () => {
expect(res.tpmBuckets.map(bucket => bucket.key)).toEqual([
'HTTP 2xx',
@ -74,7 +61,7 @@ describe('getTpmBuckets', () => {
{
key_as_string: '',
key: 3,
doc_count: 1337
doc_count: 400
}
]
}
@ -102,7 +89,7 @@ describe('getTpmBuckets', () => {
{
key_as_string: '',
key: 3,
doc_count: 1337
doc_count: 300
}
]
}
@ -110,8 +97,24 @@ describe('getTpmBuckets', () => {
];
const bucketSize = 10;
expect(getTpmBuckets(buckets, bucketSize)).toEqual([
{ dataPoints: [{ x: 1, y: 1200 }, { x: 2, y: 1800 }], key: 'HTTP 4xx' },
{ dataPoints: [{ x: 1, y: 3000 }, { x: 2, y: 600 }], key: 'HTTP 5xx' }
{
dataPoints: [
{ x: 0, y: 0 },
{ x: 1, y: 1200 },
{ x: 2, y: 1800 },
{ x: 3, y: 2400 }
],
key: 'HTTP 4xx'
},
{
dataPoints: [
{ x: 0, y: 0 },
{ x: 1, y: 3000 },
{ x: 2, y: 600 },
{ x: 3, y: 1800 }
],
key: 'HTTP 5xx'
}
]);
});
});

4
x-pack/plugins/apm/server/lib/transactions/charts/get_timeseries_data/transform.ts
View file

@ -59,7 +59,7 @@ export function getTpmBuckets(
) {
const buckets = transactionResultBuckets.map(
({ key: resultKey, timeseries }) => {
const dataPoints = timeseries.buckets.slice(1, -1).map(bucket => {
const dataPoints = timeseries.buckets.map(bucket => {
return {
x: bucket.key,
y: round(bucket.doc_count * (60 / bucketSize), 1)
@ -82,7 +82,7 @@ export function getTpmBuckets(
function getResponseTime(
responseTimeBuckets: ESResponse['aggregations']['response_times']['buckets'] = []
) {
return responseTimeBuckets.slice(1, -1).reduce(
return responseTimeBuckets.reduce(
(acc, bucket) => {
const { '95.0': p95, '99.0': p99 } = bucket.pct.values;