Skip to content

Commit

Permalink
[APM] Type checking of query params
Browse files Browse the repository at this point in the history
  • Loading branch information
sorenlouv committed Jul 31, 2019
1 parent fc5b0fb commit 2e25b6a
Show file tree
Hide file tree
Showing 20 changed files with 304 additions and 221 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { EuiIconTip, EuiTitle } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import d3 from 'd3';
import React, { FunctionComponent, useCallback } from 'react';
import { TransactionDistributionAPIResponse } from '../../../../../server/lib/transactions/distribution';
import { IBucket } from '../../../../../server/lib/transactions/distribution/get_buckets/transform';
import { IUrlParams } from '../../../../context/UrlParamsContext/types';
import { getTimeFormatter, timeUnit } from '../../../../utils/formatters';
Expand All @@ -18,6 +17,7 @@ import { EmptyMessage } from '../../../shared/EmptyMessage';
import { fromQuery, toQuery } from '../../../shared/Links/url_helpers';
import { history } from '../../../../utils/history';
import { LoadingStatePrompt } from '../../../shared/LoadingStatePrompt';
import { TransactionDistributionAPIResponse } from '../../../../../server/routes/transaction_groups/transaction_distribution_route';

interface IChartPoint {
sample?: IBucket['sample'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { loadTransactionList } from '../services/rest/apm/transaction_groups';
import { IUrlParams } from '../context/UrlParamsContext/types';
import { useUiFilters } from '../context/UrlParamsContext';
import { useFetcher } from './useFetcher';
import { TransactionGroupListAPIResponse } from '../../server/lib/transaction_groups';
import { TransactionGroupListAPIResponse } from '../../server/routes/transaction_groups/transaction_group_list_route';

const getRelativeImpact = (
impact: number,
Expand Down
9 changes: 6 additions & 3 deletions x-pack/legacy/plugins/apm/public/selectors/chartSelectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { i18n } from '@kbn/i18n';
import { difference, zipObject } from 'lodash';
import mean from 'lodash.mean';
import { rgba } from 'polished';
import { TimeSeriesAPIResponse } from '../../server/lib/transactions/charts';
import { ApmTimeSeriesResponse } from '../../server/lib/transactions/charts/get_timeseries_data/transform';
import { StringMap } from '../../typings/common';
import {
Expand All @@ -20,6 +19,7 @@ import {
import { asDecimal, asMillis, tpmUnit } from '../utils/formatters';
import { IUrlParams } from '../context/UrlParamsContext/types';
import { getEmptySeries } from '../components/shared/charts/CustomPlot/getEmptySeries';
import { TransactionChartsAPIResponse } from '../../server/routes/transaction_groups/transaction_charts_route';

export interface ITpmBucket {
title: string;
Expand Down Expand Up @@ -49,7 +49,10 @@ const INITIAL_DATA = {

export function getTransactionCharts(
{ transactionType }: IUrlParams,
{ apmTimeseries, anomalyTimeseries }: TimeSeriesAPIResponse = INITIAL_DATA
{
apmTimeseries,
anomalyTimeseries
}: TransactionChartsAPIResponse = INITIAL_DATA
): ITransactionChartData {
const tpmSeries = getTpmSeries(apmTimeseries, transactionType);

Expand All @@ -67,7 +70,7 @@ export function getTransactionCharts(
export function getResponseTimeSeries({
apmTimeseries,
anomalyTimeseries
}: TimeSeriesAPIResponse) {
}: TransactionChartsAPIResponse) {
const { overallAvgDuration } = apmTimeseries;
const { avg, p95, p99 } = apmTimeseries.responseTimes;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { TransactionBreakdownAPIResponse } from '../../../../server/lib/transactions/breakdown';
import { TimeSeriesAPIResponse } from '../../../../server/lib/transactions/charts';
import { TransactionDistributionAPIResponse } from '../../../../server/lib/transactions/distribution';
import { callApi } from '../callApi';
import { UIFilters } from '../../../../typings/ui-filters';
import { TransactionGroupListAPIResponse } from '../../../../server/lib/transaction_groups';
import { transactionChartsRoute } from '../../../../server/routes/transaction_groups/transaction_charts_route';
import { transactionGroupListRoute } from '../../../../server/routes/transaction_groups/transaction_group_list_route';
import { transactionDistributionRoute } from '../../../../server/routes/transaction_groups/transactionDistributionRoute';
import { transactionBreakdownRoute } from '../../../../server/routes/transaction_groups/transaction_breakdown_route';

export async function loadTransactionList({
serviceName,
Expand All @@ -24,7 +24,7 @@ export async function loadTransactionList({
transactionType: string;
uiFilters: UIFilters;
}) {
return await callApi<TransactionGroupListAPIResponse>({
return await callApi<typeof transactionGroupListRoute>({
pathname: `/api/apm/services/${serviceName}/transaction_groups`,
query: {
start,
Expand Down Expand Up @@ -54,7 +54,7 @@ export async function loadTransactionDistribution({
traceId?: string;
uiFilters: UIFilters;
}) {
return callApi<TransactionDistributionAPIResponse>({
return callApi<typeof transactionDistributionRoute>({
pathname: `/api/apm/services/${serviceName}/transaction_groups/distribution`,
query: {
start,
Expand Down Expand Up @@ -83,7 +83,7 @@ export async function loadTransactionCharts({
transactionName?: string;
uiFilters: UIFilters;
}) {
return callApi<TimeSeriesAPIResponse>({
return callApi<typeof transactionChartsRoute>({
pathname: `/api/apm/services/${serviceName}/transaction_groups/charts`,
query: {
start,
Expand All @@ -110,7 +110,7 @@ export async function loadTransactionBreakdown({
transactionType: string;
uiFilters: UIFilters;
}) {
return callApi<TransactionBreakdownAPIResponse>({
return callApi<typeof transactionBreakdownRoute>({
pathname: `/api/apm/services/${serviceName}/transaction_groups/breakdown`,
query: {
start,
Expand Down
14 changes: 11 additions & 3 deletions x-pack/legacy/plugins/apm/public/services/rest/callApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,18 @@ export function _clearCache() {
cache.reset();
}

export async function callApi<T = void>(
fetchOptions: KFetchOptions,
interface CallApiOptions<Q extends Record<string, any>> extends KFetchOptions {
query?: Q;
}

interface Route {
method: string;
handler: (req: any) => any;
}
export async function callApi<R extends Route>(
fetchOptions: CallApiOptions<Parameters<R['handler']>[0]['query']>,
options?: KFetchKibanaOptions
): Promise<T> {
): Promise<ReturnType<R['handler']>> {
const cacheKey = getCacheKey(fetchOptions);
const cacheResponse = cache.get(cacheKey);
if (cacheResponse) {
Expand Down
5 changes: 3 additions & 2 deletions x-pack/legacy/plugins/apm/server/lib/helpers/es_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { Legacy } from 'kibana';
import { cloneDeep, has, isString, set } from 'lodash';
import { OBSERVER_VERSION_MAJOR } from '../../../common/elasticsearch_fieldnames';
import { StringMap } from '../../../typings/common';
import { APMRequest } from './setup_request';

function getApmIndices(config: Legacy.KibanaConfig) {
return [
Expand Down Expand Up @@ -66,7 +67,7 @@ function addFilterForLegacyData(

// add additional params for search (aka: read) requests
async function getParamsForSearchRequest(
req: Legacy.Request,
req: APMRequest,
params: SearchParams,
apmOptions?: APMOptions
) {
Expand All @@ -85,7 +86,7 @@ interface APMOptions {
includeLegacyData: boolean;
}

export function getESClient(req: Legacy.Request) {
export function getESClient(req: APMRequest) {
const cluster = req.server.plugins.elasticsearch.getCluster('data');
const query = req.query as StringMap;

Expand Down
17 changes: 14 additions & 3 deletions x-pack/legacy/plugins/apm/server/lib/helpers/setup_request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,27 @@ function decodeUiFilters(server: Server, uiFiltersEncoded?: string) {
return getUiFiltersES(server, uiFilters);
}

export interface APMRequestQuery {
// overwrite "query" on Legacy.Request
export type APMRequest<Q extends Record<string, any> | unknown = unknown> = {
query: Q;
} & Omit<Legacy.Request, 'query'>;

export interface DefaultQueryParams {
start: string;
end: string;
uiFilters: string;
}

interface APMRequestQuery {
_debug?: string;
start?: string;
end?: string;
uiFilters?: string;
}

export type Setup = PromiseReturnType<typeof setupRequest>;
export async function setupRequest(req: Legacy.Request) {
const query = (req.query as unknown) as APMRequestQuery;
export async function setupRequest(req: APMRequest<unknown>) {
const query = req.query as APMRequestQuery;
const { server } = req;
const config = server.config();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@
import { Setup } from '../helpers/setup_request';
import { transactionGroupsFetcher, Options } from './fetcher';
import { transactionGroupsTransformer } from './transform';
import { PromiseReturnType } from '../../../typings/common';

export type TransactionGroupListAPIResponse = PromiseReturnType<
typeof getTransactionGroupList
>;
export async function getTransactionGroupList(options: Options, setup: Setup) {
const { start, end } = setup;
const response = await transactionGroupsFetcher(options, setup);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,11 @@ import {
TRANSACTION_NAME,
TRANSACTION_BREAKDOWN_COUNT
} from '../../../../common/elasticsearch_fieldnames';
import { PromiseReturnType } from '../../../../typings/common';
import { Setup } from '../../helpers/setup_request';
import { rangeFilter } from '../../helpers/range_filter';
import { getMetricsDateHistogramParams } from '../../helpers/metrics';
import { MAX_KPIS, COLORS } from './constants';

export type TransactionBreakdownAPIResponse = PromiseReturnType<
typeof getTransactionBreakdown
>;

export async function getTransactionBreakdown({
setup,
serviceName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { PromiseReturnType } from '../../../../typings/common';
import { Setup } from '../../helpers/setup_request';
import { getAnomalySeries } from './get_anomaly_data';
import { getApmTimeseriesData } from './get_timeseries_data';
Expand All @@ -14,9 +13,6 @@ function getDates(apmTimeseries: ApmTimeSeriesResponse) {
return apmTimeseries.responseTimes.avg.map(p => p.x);
}

export type TimeSeriesAPIResponse = PromiseReturnType<
typeof getTransactionCharts
>;
export async function getTransactionCharts(options: {
serviceName: string;
transactionType: string | undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,23 @@ import {
import { rangeFilter } from '../../../helpers/range_filter';
import { Setup } from '../../../helpers/setup_request';

export function bucketFetcher(
serviceName: string,
transactionName: string,
transactionType: string,
transactionId: string,
traceId: string,
bucketSize: number,
setup: Setup
) {
export function bucketFetcher({
serviceName,
transactionName,
transactionType,
transactionId,
traceId,
bucketSize,
setup
}: {
serviceName: string;
transactionName: string;
transactionType: string;
transactionId?: string;
traceId?: string;
bucketSize: number;
setup: Setup;
}) {
const { start, end, uiFiltersES, client, config } = setup;
const bucketTargetCount = config.get<number>('xpack.apm.bucketTargetCount');

Expand All @@ -44,8 +52,8 @@ export function bucketFetcher(
...uiFiltersES
],
should: [
{ term: { [TRACE_ID]: traceId } },
{ term: { [TRANSACTION_ID]: transactionId } },
{ term: { [TRACE_ID]: traceId } }, // TODO: might fail when undefined
{ term: { [TRANSACTION_ID]: transactionId } }, // TODO: might fail when undefined
{ term: { [TRANSACTION_SAMPLED]: true } }
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,16 @@ import { Setup } from '../../../helpers/setup_request';
import { bucketFetcher } from './fetcher';
import { bucketTransformer } from './transform';

export async function getBuckets(
serviceName: string,
transactionName: string,
transactionType: string,
transactionId: string,
traceId: string,
bucketSize: number,
setup: Setup
) {
const response = await bucketFetcher(
serviceName,
transactionName,
transactionType,
transactionId,
traceId,
bucketSize,
setup
);
export async function getBuckets(options: {
serviceName: string;
transactionName: string;
transactionType: string;
transactionId?: string;
traceId?: string;
bucketSize: number;
setup: Setup;
}) {
const response = await bucketFetcher(options);

return bucketTransformer(response);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { PromiseReturnType } from '../../../../typings/common';
import { Setup } from '../../helpers/setup_request';
import { calculateBucketSize } from './calculate_bucket_size';
import { getBuckets } from './get_buckets';

export type TransactionDistributionAPIResponse = PromiseReturnType<
typeof getTransactionDistribution
>;
export async function getTransactionDistribution({
serviceName,
transactionName,
Expand All @@ -23,8 +19,8 @@ export async function getTransactionDistribution({
serviceName: string;
transactionName: string;
transactionType: string;
transactionId: string;
traceId: string;
transactionId?: string;
traceId?: string;
setup: Setup;
}) {
const bucketSize = await calculateBucketSize(
Expand All @@ -34,15 +30,15 @@ export async function getTransactionDistribution({
setup
);

const { buckets, totalHits } = await getBuckets(
const { buckets, totalHits } = await getBuckets({
serviceName,
transactionName,
transactionType,
transactionId,
traceId,
bucketSize,
setup
);
});

return {
totalHits,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/apm/server/new-platform/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { initErrorsApi } from '../routes/errors';
import { initMetricsApi } from '../routes/metrics';
import { initServicesApi } from '../routes/services';
import { initTracesApi } from '../routes/traces';
import { initTransactionGroupsApi } from '../routes/transaction_groups';
import { initTransactionGroupsApi } from '../routes/transaction_groups/init';
import { initUIFiltersApi } from '../routes/ui_filters';
import { initIndexPatternApi } from '../routes/index_pattern';
import { initSettingsApi } from '../routes/settings';
Expand Down
Loading

0 comments on commit 2e25b6a

Please sign in to comment.