Skip to content

Commit

Permalink
[APM] Fix anomalies not showing up on transaction charts (#76930)
Browse files Browse the repository at this point in the history
* [APM] Fix anomalies not showing up on transaction charts

* Added API tests to check transaction groups charts for anomaly data

* Improve test names and assertions from PR feedback

* Updated the transaction groups chart API to make `environment` a
required param while making `uiFilters` optional

* updates the basic API tests for transaction_groups/charts with the
required `environment` param

* makes uiFIltersES default to [] on core setup and removes SetupUIFilters type

* fixes vertical shade

* - replaces uiFiltersES with esFilter & uiFilters and cleans up related code around these
- deduplicates the required environment in the transaction_groups/charts API

* updates basic apm_api_integration tests

* pr feedback

* updates api test snapshots with correct anomaly data

* removed environment query param from useTransactionCharts and ensures
it's included in uiFilters returned from useUrlParams

Co-authored-by: Oliver Gupte <[email protected]>
  • Loading branch information
sorenlouv and ogupte authored Oct 2, 2020
1 parent 1b61cc6 commit 4ddcd1d
Show file tree
Hide file tree
Showing 87 changed files with 528 additions and 548 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
"id": "opbeans-go~>postgresql",
"sourceData": {
"id": "opbeans-go",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-go",
"agent.name": "go"
},
Expand All @@ -103,7 +103,7 @@
"id": "opbeans-go~opbeans-java",
"sourceData": {
"id": "opbeans-go",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-go",
"agent.name": "go"
},
Expand All @@ -123,13 +123,13 @@
"id": "opbeans-go~opbeans-node",
"sourceData": {
"id": "opbeans-go",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-go",
"agent.name": "go"
},
"targetData": {
"id": "opbeans-node",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-node",
"agent.name": "nodejs"
},
Expand All @@ -143,7 +143,7 @@
"id": "opbeans-go~opbeans-ruby",
"sourceData": {
"id": "opbeans-go",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-go",
"agent.name": "go"
},
Expand Down Expand Up @@ -189,7 +189,7 @@
},
"targetData": {
"id": "opbeans-go",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-go",
"agent.name": "go"
},
Expand All @@ -209,7 +209,7 @@
},
"targetData": {
"id": "opbeans-node",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-node",
"agent.name": "nodejs"
}
Expand Down Expand Up @@ -242,7 +242,7 @@
"id": "opbeans-node~>postgresql",
"sourceData": {
"id": "opbeans-node",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-node",
"agent.name": "nodejs"
},
Expand All @@ -262,7 +262,7 @@
"id": "opbeans-node~>redis",
"sourceData": {
"id": "opbeans-node",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-node",
"agent.name": "nodejs"
},
Expand All @@ -282,13 +282,13 @@
"id": "opbeans-node~opbeans-go",
"sourceData": {
"id": "opbeans-node",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-node",
"agent.name": "nodejs"
},
"targetData": {
"id": "opbeans-go",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-go",
"agent.name": "go"
},
Expand All @@ -302,7 +302,7 @@
"id": "opbeans-node~opbeans-python",
"sourceData": {
"id": "opbeans-node",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-node",
"agent.name": "nodejs"
},
Expand All @@ -322,7 +322,7 @@
"id": "opbeans-node~opbeans-ruby",
"sourceData": {
"id": "opbeans-node",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-node",
"agent.name": "nodejs"
},
Expand Down Expand Up @@ -408,7 +408,7 @@
},
"targetData": {
"id": "opbeans-go",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-go",
"agent.name": "go"
}
Expand All @@ -427,7 +427,7 @@
},
"targetData": {
"id": "opbeans-node",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-node",
"agent.name": "nodejs"
},
Expand Down Expand Up @@ -487,7 +487,7 @@
},
"targetData": {
"id": "opbeans-go",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-go",
"agent.name": "go"
},
Expand Down Expand Up @@ -527,7 +527,7 @@
},
"targetData": {
"id": "opbeans-node",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-node",
"agent.name": "nodejs"
},
Expand Down Expand Up @@ -566,7 +566,7 @@
},
"targetData": {
"id": "opbeans-go",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-go",
"agent.name": "go"
}
Expand Down Expand Up @@ -602,7 +602,7 @@
},
"targetData": {
"id": "opbeans-node",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-node",
"agent.name": "nodejs"
}
Expand Down Expand Up @@ -673,7 +673,7 @@
{
"data": {
"id": "opbeans-node",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-node",
"agent.name": "nodejs",
"anomaly_score": 41.31593099784474,
Expand Down Expand Up @@ -733,7 +733,7 @@
{
"data": {
"id": "opbeans-go",
"service.environment": "testing",
"service.environment": "test",
"service.name": "opbeans-go",
"agent.name": "go",
"anomaly_score": 0.2633884161762746,
Expand Down
7 changes: 6 additions & 1 deletion x-pack/plugins/apm/public/context/UrlParamsContext/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
import { pickKeys } from '../../../common/utils/pick_keys';
import { useDeepObjectIdentity } from '../../hooks/useDeepObjectIdentity';
import { LocalUIFilterName } from '../../../common/ui_filter';
import { ENVIRONMENT_ALL } from '../../../common/environment_filter_values';

interface TimeRange {
rangeFrom: string;
Expand All @@ -38,7 +39,11 @@ function useUiFilters(params: IUrlParams): UIFilters {
(val) => (val ? val.split(',') : [])
) as Partial<Record<LocalUIFilterName, string[]>>;

return useDeepObjectIdentity({ kuery, environment, ...localUiFilters });
return useDeepObjectIdentity({
kuery,
environment: environment || ENVIRONMENT_ALL.value,
...localUiFilters,
});
}

const defaultRefresh = (_time: TimeRange) => {};
Expand Down
7 changes: 5 additions & 2 deletions x-pack/plugins/apm/public/utils/testHelpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
} from '../../typings/elasticsearch';
import { MockApmPluginContextWrapper } from '../context/ApmPluginContext/MockApmPluginContext';
import { UrlParamsProvider } from '../context/UrlParamsContext';
import { UIFilters } from '../../typings/ui_filters';

const originalConsoleWarn = console.warn; // eslint-disable-line no-console
/**
Expand Down Expand Up @@ -118,7 +119,8 @@ interface MockSetup {
apmEventClient: any;
internalClient: any;
config: APMConfig;
uiFiltersES: ESFilter[];
uiFilters: UIFilters;
esFilter: ESFilter[];
indices: {
/* eslint-disable @typescript-eslint/naming-convention */
'apm_oss.sourcemapIndices': string;
Expand Down Expand Up @@ -179,7 +181,8 @@ export async function inspectSearchParams(
},
}
) as APMConfig,
uiFiltersES: [{ term: { 'my.custom.ui.filter': 'foo-bar' } }],
uiFilters: { environment: 'test' },
esFilter: [{ term: { 'service.environment': 'test' } }],
indices: {
/* eslint-disable @typescript-eslint/naming-convention */
'apm_oss.sourcemapIndices': 'myIndex',
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ describe('timeseriesFetcher', () => {
get: () => 'myIndex',
}
) as APMConfig,
uiFiltersES: [
uiFilters: {
environment: 'prod',
},
esFilter: [
{
term: { 'service.environment': 'prod' },
},
Expand Down
12 changes: 4 additions & 8 deletions x-pack/plugins/apm/server/lib/errors/distribution/get_buckets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,7 @@ import {
SERVICE_NAME,
} from '../../../../common/elasticsearch_fieldnames';
import { rangeFilter } from '../../../../common/utils/range_filter';
import {
Setup,
SetupTimeRange,
SetupUIFilters,
} from '../../helpers/setup_request';
import { Setup, SetupTimeRange } from '../../helpers/setup_request';

export async function getBuckets({
serviceName,
Expand All @@ -26,13 +22,13 @@ export async function getBuckets({
serviceName: string;
groupId?: string;
bucketSize: number;
setup: Setup & SetupTimeRange & SetupUIFilters;
setup: Setup & SetupTimeRange;
}) {
const { start, end, uiFiltersES, apmEventClient } = setup;
const { start, end, esFilter, apmEventClient } = setup;
const filter: ESFilter[] = [
{ term: { [SERVICE_NAME]: serviceName } },
{ range: rangeFilter(start, end) },
...uiFiltersES,
...esFilter,
];

if (groupId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@
*/

import { PromiseReturnType } from '../../../../typings/common';
import {
Setup,
SetupTimeRange,
SetupUIFilters,
} from '../../helpers/setup_request';
import { Setup, SetupTimeRange } from '../../helpers/setup_request';
import { getBuckets } from './get_buckets';
import { BUCKET_TARGET_COUNT } from '../../transactions/constants';

Expand All @@ -28,7 +24,7 @@ export async function getErrorDistribution({
}: {
serviceName: string;
groupId?: string;
setup: Setup & SetupTimeRange & SetupUIFilters;
setup: Setup & SetupTimeRange;
}) {
const bucketSize = getBucketSize({ start: setup.start, end: setup.end });
const { buckets, noHits } = await getBuckets({
Expand Down
12 changes: 4 additions & 8 deletions x-pack/plugins/apm/server/lib/errors/get_error_group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ import {
} from '../../../common/elasticsearch_fieldnames';
import { PromiseReturnType } from '../../../typings/common';
import { rangeFilter } from '../../../common/utils/range_filter';
import {
Setup,
SetupTimeRange,
SetupUIFilters,
} from '../helpers/setup_request';
import { Setup, SetupTimeRange } from '../helpers/setup_request';
import { getTransaction } from '../transactions/get_transaction';

export type ErrorGroupAPIResponse = PromiseReturnType<typeof getErrorGroup>;
Expand All @@ -29,9 +25,9 @@ export async function getErrorGroup({
}: {
serviceName: string;
groupId: string;
setup: Setup & SetupTimeRange & SetupUIFilters;
setup: Setup & SetupTimeRange;
}) {
const { start, end, uiFiltersES, apmEventClient } = setup;
const { start, end, esFilter, apmEventClient } = setup;

const params = {
apm: {
Expand All @@ -45,7 +41,7 @@ export async function getErrorGroup({
{ term: { [SERVICE_NAME]: serviceName } },
{ term: { [ERROR_GROUP_ID]: groupId } },
{ range: rangeFilter(start, end) },
...uiFiltersES,
...esFilter,
],
should: [{ term: { [TRANSACTION_SAMPLED]: true } }],
},
Expand Down
8 changes: 2 additions & 6 deletions x-pack/plugins/apm/server/lib/errors/get_error_groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ import {
ERROR_LOG_MESSAGE,
} from '../../../common/elasticsearch_fieldnames';
import { PromiseReturnType } from '../../../typings/common';
import {
Setup,
SetupTimeRange,
SetupUIFilters,
} from '../helpers/setup_request';
import { Setup, SetupTimeRange } from '../helpers/setup_request';
import { getErrorGroupsProjection } from '../../projections/errors';
import { mergeProjection } from '../../projections/util/merge_projection';
import { SortOptions } from '../../../typings/elasticsearch/aggregations';
Expand All @@ -35,7 +31,7 @@ export async function getErrorGroups({
serviceName: string;
sortField?: string;
sortDirection?: 'asc' | 'desc';
setup: Setup & SetupTimeRange & SetupUIFilters;
setup: Setup & SetupTimeRange;
}) {
const { apmEventClient } = setup;

Expand Down
Loading

0 comments on commit 4ddcd1d

Please sign in to comment.