From 02021641537ae93db746f0143c8b2315da458c38 Mon Sep 17 00:00:00 2001 From: Dario Gieselaar Date: Tue, 15 Feb 2022 10:38:07 +0100 Subject: [PATCH] [APM] Lint rule for explicit return types (#124771) --- .eslintrc.js | 12 +++ .../apm/server/routes/backends/route.ts | 33 +++++--- .../apm/server/routes/correlations/route.ts | 26 +++++-- .../apm/server/routes/data_view/route.ts | 6 +- .../plugins/apm/server/routes/fleet/route.ts | 29 ++++--- .../routes/observability_overview/route.ts | 49 +++++++----- .../apm/server/routes/rum_client/route.ts | 1 + .../apm/server/routes/services/route.ts | 76 +++++++++++-------- .../settings/anomaly_detection/route.ts | 8 +- 9 files changed, 160 insertions(+), 80 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index ce7e2dea0a14f..6c98a016469f7 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -886,6 +886,18 @@ module.exports = { ], }, }, + { + // require explicit return types in route handlers for performance reasons + files: ['x-pack/plugins/apm/server/**/route.ts'], + rules: { + '@typescript-eslint/explicit-function-return-type': [ + 'error', + { + allowTypedFunctionExpressions: false, + }, + ], + }, + }, /** * Fleet overrides diff --git a/x-pack/plugins/apm/server/routes/backends/route.ts b/x-pack/plugins/apm/server/routes/backends/route.ts index 02dac877715a9..730ad672a26b7 100644 --- a/x-pack/plugins/apm/server/routes/backends/route.ts +++ b/x-pack/plugins/apm/server/routes/backends/route.ts @@ -21,6 +21,7 @@ import { getTopBackends } from './get_top_backends'; import { getUpstreamServicesForBackend } from './get_upstream_services_for_backend'; import { getThroughputChartsForBackend } from './get_throughput_charts_for_backend'; import { getErrorRateChartsForBackend } from './get_error_rate_charts_for_backend'; +import { ConnectionStatsItemWithImpact } from './../../../common/connections'; const topBackendsRoute = createApmServerRoute({ endpoint: 'GET /internal/apm/backends/top_backends', @@ -105,10 +106,11 @@ const topBackendsRoute = createApmServerRoute({ ]); return { + // eslint-disable-next-line @typescript-eslint/explicit-function-return-type backends: currentBackends.map((backend) => { const { stats, ...rest } = backend; const prev = previousBackends.find( - (item) => item.location.id === backend.location.id + (item): boolean => item.location.id === backend.location.id ); return { ...rest, @@ -221,17 +223,24 @@ const upstreamServicesForBackendRoute = createApmServerRoute({ ]); return { - services: currentServices.map((service) => { - const { stats, ...rest } = service; - const prev = previousServices.find( - (item) => item.location.id === service.location.id - ); - return { - ...rest, - currentStats: stats, - previousStats: prev?.stats ?? null, - }; - }), + services: currentServices.map( + ( + service + ): Omit & { + currentStats: ConnectionStatsItemWithImpact['stats']; + previousStats: ConnectionStatsItemWithImpact['stats'] | null; + } => { + const { stats, ...rest } = service; + const prev = previousServices.find( + (item): boolean => item.location.id === service.location.id + ); + return { + ...rest, + currentStats: stats, + previousStats: prev?.stats ?? null, + }; + } + ), }; }, }); diff --git a/x-pack/plugins/apm/server/routes/correlations/route.ts b/x-pack/plugins/apm/server/routes/correlations/route.ts index 0e1707cc55222..fd0bce7a62ff8 100644 --- a/x-pack/plugins/apm/server/routes/correlations/route.ts +++ b/x-pack/plugins/apm/server/routes/correlations/route.ts @@ -27,6 +27,13 @@ import { withApmSpan } from '../../utils/with_apm_span'; import { createApmServerRoute } from '../apm_routes/create_apm_server_route'; import { environmentRt, kueryRt, rangeRt } from '../default_api_types'; +import { LatencyCorrelation } from './../../../common/correlations/latency_correlations/types'; +import { + FieldStats, + TopValuesStats, +} from './../../../common/correlations/field_stats_types'; +import { FieldValuePair } from './../../../common/correlations/types'; +import { FailedTransactionsCorrelation } from './../../../common/correlations/failed_transactions_correlations/types'; const INVALID_LICENSE = i18n.translate('xpack.apm.correlations.license.text', { defaultMessage: @@ -59,7 +66,7 @@ const fieldCandidatesRoute = createApmServerRoute({ return withApmSpan( 'get_correlations_field_candidates', - async () => + async (): Promise<{ fieldCandidates: string[] }> => await fetchTransactionDurationFieldCandidates(esClient, { ...resources.params.query, index: indices.transaction, @@ -106,7 +113,7 @@ const fieldStatsRoute = createApmServerRoute({ return withApmSpan( 'get_correlations_field_stats', - async () => + async (): Promise<{ stats: FieldStats[]; errors: any[] }> => await fetchFieldsStats( esClient, { @@ -155,7 +162,7 @@ const fieldValueStatsRoute = createApmServerRoute({ return withApmSpan( 'get_correlations_field_value_stats', - async () => + async (): Promise => await fetchFieldValueFieldStats( esClient, { @@ -206,7 +213,7 @@ const fieldValuePairsRoute = createApmServerRoute({ return withApmSpan( 'get_correlations_field_value_pairs', - async () => + async (): Promise<{ errors: any[]; fieldValuePairs: FieldValuePair[] }> => await fetchTransactionDurationFieldValuePairs( esClient, { @@ -268,7 +275,11 @@ const significantCorrelationsRoute = createApmServerRoute({ return withApmSpan( 'get_significant_correlations', - async () => + async (): Promise<{ + latencyCorrelations: LatencyCorrelation[]; + ccsWarning: boolean; + totalDocCount: number; + }> => await fetchSignificantCorrelations( esClient, paramsWithIndex, @@ -321,7 +332,10 @@ const pValuesRoute = createApmServerRoute({ return withApmSpan( 'get_p_values', - async () => await fetchPValues(esClient, paramsWithIndex, fieldCandidates) + async (): Promise<{ + failedTransactionsCorrelations: FailedTransactionsCorrelation[]; + ccsWarning: boolean; + }> => await fetchPValues(esClient, paramsWithIndex, fieldCandidates) ); }, }); diff --git a/x-pack/plugins/apm/server/routes/data_view/route.ts b/x-pack/plugins/apm/server/routes/data_view/route.ts index b918e687bd7cd..01d835b149d2e 100644 --- a/x-pack/plugins/apm/server/routes/data_view/route.ts +++ b/x-pack/plugins/apm/server/routes/data_view/route.ts @@ -9,6 +9,7 @@ import { createStaticDataView } from './create_static_data_view'; import { setupRequest } from '../../lib/helpers/setup_request'; import { getDynamicDataView } from './get_dynamic_data_view'; import { createApmServerRoute } from '../apm_routes/create_apm_server_route'; +import { ISavedObjectsRepository } from '../../../../../../src/core/server'; const staticDataViewRoute = createApmServerRoute({ endpoint: 'POST /internal/apm/data_view/static', @@ -24,7 +25,10 @@ const staticDataViewRoute = createApmServerRoute({ const setupPromise = setupRequest(resources); const clientPromise = core .start() - .then((coreStart) => coreStart.savedObjects.createInternalRepository()); + .then( + (coreStart): ISavedObjectsRepository => + coreStart.savedObjects.createInternalRepository() + ); const setup = await setupPromise; const savedObjectsClient = await clientPromise; diff --git a/x-pack/plugins/apm/server/routes/fleet/route.ts b/x-pack/plugins/apm/server/routes/fleet/route.ts index 668d4e207208c..11753ab3ef12c 100644 --- a/x-pack/plugins/apm/server/routes/fleet/route.ts +++ b/x-pack/plugins/apm/server/routes/fleet/route.ts @@ -105,16 +105,25 @@ const fleetAgentsRoute = createApmServerRoute({ return { cloudStandaloneSetup, isFleetEnabled: true, - fleetAgents: fleetAgents.map((agent) => { - const packagePolicy = policiesGroupedById[agent.id]; - const packagePolicyVars = packagePolicy.inputs[0]?.vars; - return { - id: agent.id, - name: agent.name, - apmServerUrl: packagePolicyVars?.url?.value, - secretToken: packagePolicyVars?.secret_token?.value, - }; - }), + fleetAgents: fleetAgents.map( + ( + agent + ): { + id: string; + name: string; + apmServerUrl: string | undefined; + secretToken: string | undefined; + } => { + const packagePolicy = policiesGroupedById[agent.id]; + const packagePolicyVars = packagePolicy.inputs[0]?.vars; + return { + id: agent.id, + name: agent.name, + apmServerUrl: packagePolicyVars?.url?.value, + secretToken: packagePolicyVars?.secret_token?.value, + }; + } + ), }; }, }); diff --git a/x-pack/plugins/apm/server/routes/observability_overview/route.ts b/x-pack/plugins/apm/server/routes/observability_overview/route.ts index faccd5eb29602..e32c04b849664 100644 --- a/x-pack/plugins/apm/server/routes/observability_overview/route.ts +++ b/x-pack/plugins/apm/server/routes/observability_overview/route.ts @@ -58,25 +58,36 @@ const observabilityOverviewRoute = createApmServerRoute({ kuery: '', }); - return withApmSpan('observability_overview', async () => { - const [serviceCount, transactionPerMinute] = await Promise.all([ - getServiceCount({ - setup, - searchAggregatedTransactions, - start, - end, - }), - getTransactionsPerMinute({ - setup, - bucketSize, - searchAggregatedTransactions, - start, - end, - intervalString, - }), - ]); - return { serviceCount, transactionPerMinute }; - }); + return withApmSpan( + 'observability_overview', + async (): Promise<{ + serviceCount: number; + transactionPerMinute: + | { value: undefined; timeseries: never[] } + | { + value: number; + timeseries: Array<{ x: number; y: number | null }>; + }; + }> => { + const [serviceCount, transactionPerMinute] = await Promise.all([ + getServiceCount({ + setup, + searchAggregatedTransactions, + start, + end, + }), + getTransactionsPerMinute({ + setup, + bucketSize, + searchAggregatedTransactions, + start, + end, + intervalString, + }), + ]); + return { serviceCount, transactionPerMinute }; + } + ); }, }); diff --git a/x-pack/plugins/apm/server/routes/rum_client/route.ts b/x-pack/plugins/apm/server/routes/rum_client/route.ts index e3bee6da6722c..d7d1e2837c53e 100644 --- a/x-pack/plugins/apm/server/routes/rum_client/route.ts +++ b/x-pack/plugins/apm/server/routes/rum_client/route.ts @@ -407,6 +407,7 @@ function decodeUiFilters( } } +// eslint-disable-next-line @typescript-eslint/explicit-function-return-type async function setupUXRequest( resources: APMRouteHandlerResources & { params: TParams } ) { diff --git a/x-pack/plugins/apm/server/routes/services/route.ts b/x-pack/plugins/apm/server/routes/services/route.ts index db7793568676b..949105807b0f2 100644 --- a/x-pack/plugins/apm/server/routes/services/route.ts +++ b/x-pack/plugins/apm/server/routes/services/route.ts @@ -49,6 +49,9 @@ import { } from '../../../../ml/server'; import { getServiceInstancesDetailedStatisticsPeriods } from './get_service_instances/detailed_statistics'; import { ML_ERRORS } from '../../../common/anomaly_detection'; +import { ScopedAnnotationsClient } from '../../../../observability/server'; +import { Annotation } from './../../../../observability/common/annotations'; +import { ConnectionStatsItemWithImpact } from './../../../common/connections'; const servicesRoute = createApmServerRoute({ endpoint: 'GET /internal/apm/services', @@ -373,8 +376,10 @@ const serviceAnnotationsRoute = createApmServerRoute({ const [annotationsClient, searchAggregatedTransactions] = await Promise.all( [ observability - ? withApmSpan('get_scoped_annotations_client', () => - observability.setup.getScopedAnnotationsClient(context, request) + ? withApmSpan( + 'get_scoped_annotations_client', + (): Promise => + observability.setup.getScopedAnnotationsClient(context, request) ) : undefined, getSearchAggregatedTransactions({ @@ -443,8 +448,10 @@ const serviceAnnotationsCreateRoute = createApmServerRoute({ } = resources; const annotationsClient = observability - ? await withApmSpan('get_scoped_annotations_client', () => - observability.setup.getScopedAnnotationsClient(context, request) + ? await withApmSpan( + 'get_scoped_annotations_client', + (): Promise => + observability.setup.getScopedAnnotationsClient(context, request) ) : undefined; @@ -454,20 +461,22 @@ const serviceAnnotationsCreateRoute = createApmServerRoute({ const { body, path } = params; - return withApmSpan('create_annotation', () => - annotationsClient.create({ - message: body.service.version, - ...body, - '@timestamp': new Date(body['@timestamp']).toISOString(), - annotation: { - type: 'deployment', - }, - service: { - ...body.service, - name: path.serviceName, - }, - tags: uniq(['apm'].concat(body.tags ?? [])), - }) + return withApmSpan( + 'create_annotation', + (): Promise<{ _id: string; _index: string; _source: Annotation }> => + annotationsClient.create({ + message: body.service.version, + ...body, + '@timestamp': new Date(body['@timestamp']).toISOString(), + annotation: { + type: 'deployment', + }, + service: { + ...body.service, + name: path.serviceName, + }, + tags: uniq(['apm'].concat(body.tags ?? [])), + }) ); }, }); @@ -925,18 +934,25 @@ export const serviceDependenciesRoute = createApmServerRoute({ ]); return { - serviceDependencies: currentPeriod.map((item) => { - const { stats, ...rest } = item; - const previousPeriodItem = previousPeriod.find( - (prevItem) => item.location.id === prevItem.location.id - ); - - return { - ...rest, - currentStats: stats, - previousStats: previousPeriodItem?.stats || null, - }; - }), + serviceDependencies: currentPeriod.map( + ( + item + ): Omit & { + currentStats: ConnectionStatsItemWithImpact['stats']; + previousStats: ConnectionStatsItemWithImpact['stats'] | null; + } => { + const { stats, ...rest } = item; + const previousPeriodItem = previousPeriod.find( + (prevItem): boolean => item.location.id === prevItem.location.id + ); + + return { + ...rest, + currentStats: stats, + previousStats: previousPeriodItem?.stats || null, + }; + } + ), }; }, }); diff --git a/x-pack/plugins/apm/server/routes/settings/anomaly_detection/route.ts b/x-pack/plugins/apm/server/routes/settings/anomaly_detection/route.ts index 44dac0d9bc4a0..974b7f57289db 100644 --- a/x-pack/plugins/apm/server/routes/settings/anomaly_detection/route.ts +++ b/x-pack/plugins/apm/server/routes/settings/anomaly_detection/route.ts @@ -19,6 +19,7 @@ import { notifyFeatureUsage } from '../../../feature'; import { updateToV3 } from './update_to_v3'; import { environmentStringRt } from '../../../../common/environment_rt'; import { getMlJobsWithAPMGroup } from '../../../lib/anomaly_detection/get_ml_jobs_with_apm_group'; +import { ElasticsearchClient } from '../../../../../../../src/core/server'; // get ML anomaly detection jobs for each environment const anomalyDetectionJobsRoute = createApmServerRoute({ @@ -49,7 +50,7 @@ const anomalyDetectionJobsRoute = createApmServerRoute({ return { jobs, - hasLegacyJobs: jobs.some((job) => job.version === 1), + hasLegacyJobs: jobs.some((job): boolean => job.version === 1), }; }, }); @@ -128,7 +129,10 @@ const anomalyDetectionUpdateToV3Route = createApmServerRoute({ setupRequest(resources), resources.core .start() - .then((start) => start.elasticsearch.client.asInternalUser), + .then( + (start): ElasticsearchClient => + start.elasticsearch.client.asInternalUser + ), ]); const { logger } = resources;