From 1ebb8dd2dfcaaeedcd01ba636c4a25d33f3b0b68 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Wed, 29 Jul 2020 16:07:37 -0700 Subject: [PATCH] Closes #73755 by removing the extra uri encoding from time ranges and using correct refreshValue rison types rather than just strings --- .../DiscoverLinks.integration.test.tsx | 16 ++++---- .../MachineLearningLinks/MLJobLink.test.tsx | 4 +- .../MachineLearningLinks/MLLink.test.tsx | 2 +- .../useTimeSeriesExplorerHref.test.ts | 40 +++++++++++++++++++ .../useTimeSeriesExplorerHref.ts | 5 ++- .../shared/Links/rison_helpers.test.ts | 27 +++++++++++++ .../components/shared/Links/rison_helpers.ts | 14 +++---- .../__test__/sections.test.ts | 6 +-- 8 files changed, 91 insertions(+), 23 deletions(-) create mode 100644 x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/useTimeSeriesExplorerHref.test.ts create mode 100644 x-pack/plugins/apm/public/components/shared/Links/rison_helpers.test.ts diff --git a/x-pack/plugins/apm/public/components/shared/Links/DiscoverLinks/__test__/DiscoverLinks.integration.test.tsx b/x-pack/plugins/apm/public/components/shared/Links/DiscoverLinks/__test__/DiscoverLinks.integration.test.tsx index 359468073f7f4..ca02abc395992 100644 --- a/x-pack/plugins/apm/public/components/shared/Links/DiscoverLinks/__test__/DiscoverLinks.integration.test.tsx +++ b/x-pack/plugins/apm/public/components/shared/Links/DiscoverLinks/__test__/DiscoverLinks.integration.test.tsx @@ -33,8 +33,8 @@ describe('DiscoverLinks', () => { } as Location ); - expect(href).toEqual( - `/basepath/app/discover#/?_g=(refreshInterval:(pause:true,value:'0'),time:(from:now%2Fw,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:'processor.event:"transaction" AND transaction.id:"8b60bd32ecc6e150" AND trace.id:"8b60bd32ecc6e1506735a8b6cfcf175c"'))` + expect(href).toMatchInlineSnapshot( + `"/basepath/app/discover#/?_g=(refreshInterval:(pause:!t,value:0),time:(from:now/w,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:'processor.event:\\"transaction\\" AND transaction.id:\\"8b60bd32ecc6e150\\" AND trace.id:\\"8b60bd32ecc6e1506735a8b6cfcf175c\\"'))"` ); }); @@ -50,8 +50,8 @@ describe('DiscoverLinks', () => { '?rangeFrom=now/w&rangeTo=now&refreshPaused=true&refreshInterval=0', } as Location); - expect(href).toEqual( - `/basepath/app/discover#/?_g=(refreshInterval:(pause:true,value:'0'),time:(from:now%2Fw,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:'span.id:"test-span-id"'))` + expect(href).toMatchInlineSnapshot( + `"/basepath/app/discover#/?_g=(refreshInterval:(pause:!t,value:0),time:(from:now/w,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:'span.id:\\"test-span-id\\"'))"` ); }); @@ -72,8 +72,8 @@ describe('DiscoverLinks', () => { } as Location ); - expect(href).toEqual( - `/basepath/app/discover#/?_g=(refreshInterval:(pause:true,value:'0'),time:(from:now%2Fw,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:'service.name:"service-name" AND error.grouping_key:"grouping-key"'),sort:('@timestamp':desc))` + expect(href).toMatchInlineSnapshot( + `"/basepath/app/discover#/?_g=(refreshInterval:(pause:!t,value:0),time:(from:now/w,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:'service.name:\\"service-name\\" AND error.grouping_key:\\"grouping-key\\"'),sort:('@timestamp':desc))"` ); }); @@ -95,8 +95,8 @@ describe('DiscoverLinks', () => { } as Location ); - expect(href).toEqual( - `/basepath/app/discover#/?_g=(refreshInterval:(pause:true,value:'0'),time:(from:now%2Fw,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:'service.name:"service-name" AND error.grouping_key:"grouping-key" AND some:kuery-string'),sort:('@timestamp':desc))` + expect(href).toMatchInlineSnapshot( + `"/basepath/app/discover#/?_g=(refreshInterval:(pause:!t,value:0),time:(from:now/w,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:'service.name:\\"service-name\\" AND error.grouping_key:\\"grouping-key\\" AND some:kuery-string'),sort:('@timestamp':desc))"` ); }); }); diff --git a/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLJobLink.test.tsx b/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLJobLink.test.tsx index 39082c2639a2c..9ba4aab0e23d9 100644 --- a/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLJobLink.test.tsx +++ b/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLJobLink.test.tsx @@ -22,7 +22,7 @@ describe('MLJobLink', () => { ); expect(href).toMatchInlineSnapshot( - `"/basepath/app/ml#/timeseriesexplorer?_g=(ml:(jobIds:!(myservicename-mytransactiontype-high_mean_response_time)),refreshInterval:(pause:true,value:'0'),time:(from:now%2Fw,to:now-4h))"` + `"/basepath/app/ml#/timeseriesexplorer?_g=(ml:(jobIds:!(myservicename-mytransactiontype-high_mean_response_time)),refreshInterval:(pause:!t,value:0),time:(from:now/w,to:now-4h))"` ); }); it('should produce the correct URL with jobId, serviceName, and transactionType', async () => { @@ -41,7 +41,7 @@ describe('MLJobLink', () => { ); expect(href).toMatchInlineSnapshot( - `"/basepath/app/ml#/timeseriesexplorer?_g=(ml:(jobIds:!(myservicename-mytransactiontype-high_mean_response_time)),refreshInterval:(pause:true,value:'0'),time:(from:now%2Fw,to:now-4h))&_a=(mlTimeSeriesExplorer:(entities:(service.name:opbeans-test,transaction.type:request)))"` + `"/basepath/app/ml#/timeseriesexplorer?_g=(ml:(jobIds:!(myservicename-mytransactiontype-high_mean_response_time)),refreshInterval:(pause:!t,value:0),time:(from:now/w,to:now-4h))&_a=(mlTimeSeriesExplorer:(entities:(service.name:opbeans-test,transaction.type:request),zoom:(from:now/w,to:now-4h)))"` ); }); }); diff --git a/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLLink.test.tsx b/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLLink.test.tsx index b4187b2f797ab..da345e35c10b1 100644 --- a/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLLink.test.tsx +++ b/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLLink.test.tsx @@ -21,6 +21,6 @@ test('MLLink produces the correct URL', async () => { ); expect(href).toMatchInlineSnapshot( - `"/basepath/app/ml#/some/path?_g=(ml:(jobIds:!(something)),refreshInterval:(pause:true,value:'0'),time:(from:now-5h,to:now-2h))"` + `"/basepath/app/ml#/some/path?_g=(ml:(jobIds:!(something)),refreshInterval:(pause:!t,value:0),time:(from:now-5h,to:now-2h))"` ); }); diff --git a/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/useTimeSeriesExplorerHref.test.ts b/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/useTimeSeriesExplorerHref.test.ts new file mode 100644 index 0000000000000..9ba70ad88f49f --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/useTimeSeriesExplorerHref.test.ts @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { useTimeSeriesExplorerHref } from './useTimeSeriesExplorerHref'; + +jest.mock('../../../../hooks/useApmPluginContext', () => ({ + useApmPluginContext: () => ({ + core: { http: { basePath: { prepend: (url: string) => url } } }, + }), +})); + +jest.mock('../../../../hooks/useLocation', () => ({ + useLocation: () => ({ + search: + '?rangeFrom=2020-07-29T17:27:29.000Z&rangeTo=2020-07-29T18:45:00.000Z&refreshInterval=10000&refreshPaused=true', + }), +})); + +describe('useTimeSeriesExplorerHref', () => { + it('correctly encodes time range values', async () => { + const href = useTimeSeriesExplorerHref({ + jobId: 'apm-production-485b-high_mean_transaction_duration', + serviceName: 'opbeans-java', + transactionType: 'request', + }); + + // expect(href).toMatchInlineSnapshot( + // `"/app/ml#/timeseriesexplorer?_g=(ml:(jobIds:!(apm-production-485b-high_mean_transaction_duration)),refreshInterval:(pause:true,value:'10000'),time:(from:'2020-07-29T17%3A27%3A29.000Z',to:'2020-07-29T18%3A45%3A00.000Z'))&_a=(mlTimeSeriesExplorer:(entities:(service.name:opbeans-java,transaction.type:request)))"` + // ); + // expect(href).toMatchInlineSnapshot( + // `"/app/ml#/timeseriesexplorer?_g=(ml:(jobIds:!(apm-production-485b-high_mean_transaction_duration)),refreshInterval:(pause:!t,value:10000),time:(from:'2020-07-29T17:27:29.000Z',to:'2020-07-29T18:45:00.000Z'))&_a=(mlTimeSeriesExplorer:(entities:(service.name:opbeans-java,transaction.type:request)))"` + // ); + expect(href).toMatchInlineSnapshot( + `"/app/ml#/timeseriesexplorer?_g=(ml:(jobIds:!(apm-production-485b-high_mean_transaction_duration)),refreshInterval:(pause:!t,value:10000),time:(from:'2020-07-29T17:27:29.000Z',to:'2020-07-29T18:45:00.000Z'))&_a=(mlTimeSeriesExplorer:(entities:(service.name:opbeans-java,transaction.type:request),zoom:(from:'2020-07-29T17:27:29.000Z',to:'2020-07-29T18:45:00.000Z')))"` + ); + }); +}); diff --git a/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/useTimeSeriesExplorerHref.ts b/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/useTimeSeriesExplorerHref.ts index 625b9205b6ce0..459ee8f0282ff 100644 --- a/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/useTimeSeriesExplorerHref.ts +++ b/x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/useTimeSeriesExplorerHref.ts @@ -22,12 +22,14 @@ export function useTimeSeriesExplorerHref({ }) { const { core } = useApmPluginContext(); const location = useLocation(); + const { time, refreshInterval } = getTimepickerRisonData(location.search); const search = querystring.stringify( { _g: rison.encode({ ml: { jobIds: [jobId] }, - ...getTimepickerRisonData(location.search), + time, + refreshInterval, }), ...(serviceName && transactionType ? { @@ -37,6 +39,7 @@ export function useTimeSeriesExplorerHref({ 'service.name': serviceName, 'transaction.type': transactionType, }, + zoom: time, }, }), } diff --git a/x-pack/plugins/apm/public/components/shared/Links/rison_helpers.test.ts b/x-pack/plugins/apm/public/components/shared/Links/rison_helpers.test.ts new file mode 100644 index 0000000000000..8dd662339b61a --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/Links/rison_helpers.test.ts @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { getTimepickerRisonData } from './rison_helpers'; + +describe('getTimepickerRisonData', () => { + it('returns object of timepicker range and refresh interval values', async () => { + const locationSearch = `?rangeFrom=2020-07-29T17:27:29.000Z&rangeTo=2020-07-29T18:45:00.000Z&refreshInterval=10000&refreshPaused=true`; + const timepickerValues = getTimepickerRisonData(locationSearch); + + expect(timepickerValues).toMatchInlineSnapshot(` + Object { + "refreshInterval": Object { + "pause": true, + "value": 10000, + }, + "time": Object { + "from": "2020-07-29T17:27:29.000Z", + "to": "2020-07-29T18:45:00.000Z", + }, + } + `); + }); +}); diff --git a/x-pack/plugins/apm/public/components/shared/Links/rison_helpers.ts b/x-pack/plugins/apm/public/components/shared/Links/rison_helpers.ts index 8b4d891dba83b..cab822b42be56 100644 --- a/x-pack/plugins/apm/public/components/shared/Links/rison_helpers.ts +++ b/x-pack/plugins/apm/public/components/shared/Links/rison_helpers.ts @@ -22,18 +22,16 @@ export function getTimepickerRisonData(currentSearch: Location['search']) { const currentQuery = toQuery(currentSearch); return { time: { - from: currentQuery.rangeFrom - ? encodeURIComponent(currentQuery.rangeFrom) - : '', - to: currentQuery.rangeTo ? encodeURIComponent(currentQuery.rangeTo) : '', + from: currentQuery.rangeFrom || '', + to: currentQuery.rangeTo || '', }, refreshInterval: { pause: currentQuery.refreshPaused - ? String(currentQuery.refreshPaused) - : '', + ? Boolean(currentQuery.refreshPaused) + : true, value: currentQuery.refreshInterval - ? String(currentQuery.refreshInterval) - : '', + ? parseInt(currentQuery.refreshInterval, 10) + : 0, }, }; } diff --git a/x-pack/plugins/apm/public/components/shared/TransactionActionMenu/__test__/sections.test.ts b/x-pack/plugins/apm/public/components/shared/TransactionActionMenu/__test__/sections.test.ts index 186fc082ce5fe..e057e9c034615 100644 --- a/x-pack/plugins/apm/public/components/shared/TransactionActionMenu/__test__/sections.test.ts +++ b/x-pack/plugins/apm/public/components/shared/TransactionActionMenu/__test__/sections.test.ts @@ -68,7 +68,7 @@ describe('Transaction action menu', () => { key: 'sampleDocument', label: 'View sample document', href: - 'some-basepath/app/discover#/?_g=(refreshInterval:(pause:true,value:\'0\'),time:(from:now-24h,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:\'processor.event:"transaction" AND transaction.id:"123" AND trace.id:"123"\'))', + 'some-basepath/app/discover#/?_g=(refreshInterval:(pause:!t,value:0),time:(from:now-24h,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:\'processor.event:"transaction" AND transaction.id:"123" AND trace.id:"123"\'))', condition: true, }, ], @@ -139,7 +139,7 @@ describe('Transaction action menu', () => { key: 'sampleDocument', label: 'View sample document', href: - 'some-basepath/app/discover#/?_g=(refreshInterval:(pause:true,value:\'0\'),time:(from:now-24h,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:\'processor.event:"transaction" AND transaction.id:"123" AND trace.id:"123"\'))', + 'some-basepath/app/discover#/?_g=(refreshInterval:(pause:!t,value:0),time:(from:now-24h,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:\'processor.event:"transaction" AND transaction.id:"123" AND trace.id:"123"\'))', condition: true, }, ], @@ -209,7 +209,7 @@ describe('Transaction action menu', () => { key: 'sampleDocument', label: 'View sample document', href: - 'some-basepath/app/discover#/?_g=(refreshInterval:(pause:true,value:\'0\'),time:(from:now-24h,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:\'processor.event:"transaction" AND transaction.id:"123" AND trace.id:"123"\'))', + 'some-basepath/app/discover#/?_g=(refreshInterval:(pause:!t,value:0),time:(from:now-24h,to:now))&_a=(index:apm_static_index_pattern_id,interval:auto,query:(language:kuery,query:\'processor.event:"transaction" AND transaction.id:"123" AND trace.id:"123"\'))', condition: true, }, ],