[APM] transactionType should be required on service-specific endpoi…

…nts (elastic#86893) (elastic#87316) * making transaction type required on some apis * addressing PR comments Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
smith · Jan 5, 2021 · 4857bbf · 4857bbf
1 parent e83e464
commit 4857bbf
Show file tree

Hide file tree

Showing 9 changed files with 40 additions and 61 deletions.
diff --git a/x-pack/plugins/apm/public/components/shared/charts/transaction_error_rate_chart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/transaction_error_rate_chart/index.tsx
@@ -36,7 +36,7 @@ export function TransactionErrorRateChart({
   const { start, end, transactionName } = urlParams;
 
   const { data, status } = useFetcher(() => {
-    if (serviceName && start && end) {
+    if (transactionType && serviceName && start && end) {
       return callApmApi({
         endpoint:
           'GET /api/apm/services/{serviceName}/transactions/charts/error_rate',

diff --git a/x-pack/plugins/apm/public/hooks/use_transaction_throughput_chart_fetcher.ts b/x-pack/plugins/apm/public/hooks/use_transaction_throughput_chart_fetcher.ts
@@ -10,18 +10,20 @@ import { useFetcher } from './use_fetcher';
 import { useUrlParams } from '../context/url_params_context/use_url_params';
 import { getThrouputChartSelector } from '../selectors/throuput_chart_selectors';
 import { useTheme } from './use_theme';
+import { useApmServiceContext } from '../context/apm_service/use_apm_service_context';
 
 export function useTransactionThroughputChartsFetcher() {
   const { serviceName } = useParams<{ serviceName?: string }>();
+  const { transactionType } = useApmServiceContext();
   const theme = useTheme();
   const {
-    urlParams: { transactionType, start, end, transactionName },
+    urlParams: { start, end, transactionName },
     uiFilters,
   } = useUrlParams();
 
   const { data, error, status } = useFetcher(
     (callApmApi) => {
-      if (serviceName && start && end) {
+      if (transactionType && serviceName && start && end) {
         return callApmApi({
           endpoint:
             'GET /api/apm/services/{serviceName}/transactions/charts/throughput',

diff --git a/...lib/services/get_service_transaction_groups/get_timeseries_data_for_transaction_groups.ts b/...lib/services/get_service_transaction_groups/get_timeseries_data_for_transaction_groups.ts
@@ -87,11 +87,6 @@ export async function getTimeseriesDataForTransactionGroups({
             size,
           },
           aggs: {
-            transaction_types: {
-              terms: {
-                field: TRANSACTION_TYPE,
-              },
-            },
             timeseries: {
               date_histogram: {
                 field: '@timestamp',

diff --git a/x-pack/plugins/apm/server/lib/services/get_service_transaction_groups/index.ts b/x-pack/plugins/apm/server/lib/services/get_service_transaction_groups/index.ts
@@ -80,6 +80,7 @@ export async function getServiceTransactionGroups({
       start,
       end,
       latencyAggregationType,
+      transactionType,
     }),
     totalTransactionGroups,
     isAggregationAccurate,

diff --git a/...ns/apm/server/lib/services/get_service_transaction_groups/merge_transaction_group_data.ts b/...ns/apm/server/lib/services/get_service_transaction_groups/merge_transaction_group_data.ts
@@ -4,17 +4,10 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { LatencyAggregationType } from '../../../../common/latency_aggregation_types';
 import { EVENT_OUTCOME } from '../../../../common/elasticsearch_fieldnames';
-
-import {
-  TRANSACTION_PAGE_LOAD,
-  TRANSACTION_REQUEST,
-} from '../../../../common/transaction_types';
+import { LatencyAggregationType } from '../../../../common/latency_aggregation_types';
 import { getLatencyValue } from '../../helpers/latency_aggregation_type';
-
 import { TransactionGroupTimeseriesData } from './get_timeseries_data_for_transaction_groups';
-
 import { TransactionGroupWithoutTimeseriesData } from './get_transaction_groups_for_page';
 
 export function mergeTransactionGroupData({
@@ -23,12 +16,14 @@ export function mergeTransactionGroupData({
   transactionGroups,
   timeseriesData,
   latencyAggregationType,
+  transactionType,
 }: {
   start: number;
   end: number;
   transactionGroups: TransactionGroupWithoutTimeseriesData[];
   timeseriesData: TransactionGroupTimeseriesData;
   latencyAggregationType: LatencyAggregationType;
+  transactionType: string;
 }) {
   const deltaAsMinutes = (end - start) / 1000 / 60;
 
@@ -37,16 +32,6 @@ export function mergeTransactionGroupData({
       ({ key }) => key === transactionGroup.name
     );
 
-    const transactionTypes =
-      groupBucket?.transaction_types.buckets.map(
-        (bucket) => bucket.key as string
-      ) ?? [];
-
-    const transactionType =
-      transactionTypes.find(
-        (type) => type === TRANSACTION_PAGE_LOAD || type === TRANSACTION_REQUEST
-      ) ?? transactionTypes[0];
-
     const timeseriesBuckets = groupBucket?.timeseries.buckets ?? [];
 
     return timeseriesBuckets.reduce(

diff --git a/x-pack/plugins/apm/server/lib/transactions/get_throughput_charts/index.ts b/x-pack/plugins/apm/server/lib/transactions/get_throughput_charts/index.ts
@@ -34,7 +34,7 @@ async function searchThroughput({
   intervalString,
 }: {
   serviceName: string;
-  transactionType: string | undefined;
+  transactionType: string;
   transactionName: string | undefined;
   setup: Setup & SetupTimeRange;
   searchAggregatedTransactions: boolean;
@@ -48,17 +48,14 @@ async function searchThroughput({
     ...getDocumentTypeFilterForAggregatedTransactions(
       searchAggregatedTransactions
     ),
+    { term: { [TRANSACTION_TYPE]: transactionType } },
     ...setup.esFilter,
   ];
 
   if (transactionName) {
     filter.push({ term: { [TRANSACTION_NAME]: transactionName } });
   }
 
-  if (transactionType) {
-    filter.push({ term: { [TRANSACTION_TYPE]: transactionType } });
-  }
-
   const field = getTransactionDurationFieldForAggregatedTransactions(
     searchAggregatedTransactions
   );
@@ -104,7 +101,7 @@ export async function getThroughputCharts({
   searchAggregatedTransactions,
 }: {
   serviceName: string;
-  transactionType: string | undefined;
+  transactionType: string;
   transactionName: string | undefined;
   setup: Setup & SetupTimeRange;
   searchAggregatedTransactions: boolean;

diff --git a/x-pack/plugins/apm/server/routes/transactions.ts b/x-pack/plugins/apm/server/routes/transactions.ts
@@ -35,9 +35,7 @@ export const transactionGroupsRoute = createRoute({
       serviceName: t.string,
     }),
     query: t.intersection([
-      t.type({
-        transactionType: t.string,
-      }),
+      t.type({ transactionType: t.string }),
       uiFiltersRt,
       rangeRt,
     ]),
@@ -199,10 +197,8 @@ export const transactionThroughputChatsRoute = createRoute({
       serviceName: t.string,
     }),
     query: t.intersection([
-      t.partial({
-        transactionType: t.string,
-        transactionName: t.string,
-      }),
+      t.type({ transactionType: t.string }),
+      t.partial({ transactionName: t.string }),
       uiFiltersRt,
       rangeRt,
     ]),
@@ -287,12 +283,8 @@ export const transactionChartsBreakdownRoute = createRoute({
       serviceName: t.string,
     }),
     query: t.intersection([
-      t.type({
-        transactionType: t.string,
-      }),
-      t.partial({
-        transactionName: t.string,
-      }),
+      t.type({ transactionType: t.string }),
+      t.partial({ transactionName: t.string }),
       uiFiltersRt,
       rangeRt,
     ]),
@@ -322,10 +314,8 @@ export const transactionChartsErrorRateRoute = createRoute({
     query: t.intersection([
       uiFiltersRt,
       rangeRt,
-      t.partial({
-        transactionType: t.string,
-        transactionName: t.string,
-      }),
+      t.type({ transactionType: t.string }),
+      t.partial({ transactionName: t.string }),
     ]),
   }),
   options: { tags: ['access:apm'] },

diff --git a/x-pack/test/apm_api_integration/basic/tests/feature_controls.ts b/x-pack/test/apm_api_integration/basic/tests/feature_controls.ts
@@ -126,13 +126,6 @@ export default function featureControlsTests({ getService }: FtrProviderContext)
       expectForbidden: expect403,
       expectResponse: expect200,
     },
-    {
-      req: {
-        url: `/api/apm/services/foo/transactions/charts/throughput?start=${start}&end=${end}&uiFilters=%7B%22environment%22%3A%22testing%22%7D`,
-      },
-      expectForbidden: expect403,
-      expectResponse: expect200,
-    },
     {
       req: {
         url: `/api/apm/services/foo/transactions/charts/throughput?start=${start}&end=${end}&transactionType=bar&transactionName=baz&uiFilters=%7B%22environment%22%3A%22testing%22%7D`,

diff --git a/x-pack/test/apm_api_integration/basic/tests/transactions/throughput.ts b/x-pack/test/apm_api_integration/basic/tests/transactions/throughput.ts
@@ -4,6 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import expect from '@kbn/expect';
+import url from 'url';
 import archives_metadata from '../../../common/archives_metadata';
 import { PromiseReturnType } from '../../../../../plugins/observability/typings/common';
 import { FtrProviderContext } from '../../../../common/ftr_provider_context';
@@ -16,15 +17,22 @@ export default function ApiTest({ getService }: FtrProviderContext) {
   const metadata = archives_metadata[archiveName];
 
   // url parameters
-  const start = encodeURIComponent(metadata.start);
-  const end = encodeURIComponent(metadata.end);
-  const uiFilters = encodeURIComponent(JSON.stringify({ environment: 'testing' }));
+  const { start, end } = metadata;
+  const uiFilters = JSON.stringify({ environment: 'testing' });
 
   describe('Throughput', () => {
     describe('when data is not loaded ', () => {
       it('handles the empty state', async () => {
         const response = await supertest.get(
-          `/api/apm/services/opbeans-node/transactions/charts/throughput?start=${start}&end=${end}&uiFilters=${uiFilters}`
+          url.format({
+            pathname: `/api/apm/services/opbeans-node/transactions/charts/throughput`,
+            query: {
+              start,
+              end,
+              uiFilters,
+              transactionType: 'request',
+            },
+          })
         );
 
         expect(response.status).to.be(200);
@@ -41,7 +49,15 @@ export default function ApiTest({ getService }: FtrProviderContext) {
 
       before(async () => {
         response = await supertest.get(
-          `/api/apm/services/opbeans-node/transactions/charts/throughput?start=${start}&end=${end}&uiFilters=${uiFilters}`
+          url.format({
+            pathname: `/api/apm/services/opbeans-node/transactions/charts/throughput`,
+            query: {
+              start,
+              end,
+              uiFilters,
+              transactionType: 'request',
+            },
+          })
         );
       });