Skip to content

Commit

Permalink
fix: use urlUtil instead of UrlSearchParams (#994)
Browse files Browse the repository at this point in the history
* fix: use urlUtil instead of UrlSearchParams

* fix: fix migration with escaped chars
  • Loading branch information
gtk-grafana authored Jan 10, 2025
1 parent 032cbd9 commit c435654
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 23 deletions.
11 changes: 5 additions & 6 deletions src/Components/IndexScene/ShareButtonScene.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
import { ButtonGroup, Dropdown, IconName, Menu, MenuGroup, ToolbarButton } from '@grafana/ui';
import React from 'react';
import { config, getAppEvents, getBackendSrv, locationService, reportInteraction } from '@grafana/runtime';
import { AppEvents, toUtc } from '@grafana/data';
import { AppEvents, toUtc, urlUtil } from '@grafana/data';
import { copyText } from '../../services/text';

interface ShortLinkMenuItemData {
Expand Down Expand Up @@ -244,9 +244,8 @@ const constructAbsoluteUrl = (timeRange: SceneTimeRangeLike): string => {
const from = toUtc(timeRange.state.value.from);
const to = toUtc(timeRange.state.value.to);
const location = locationService.getLocation();
const searchParams = new URLSearchParams(location.search);
searchParams.set('from', from.toISOString());
searchParams.set('to', to.toISOString());

return `${location.pathname}?${searchParams.toString()}`;
const searchParams = urlUtil.getUrlSearchParams();
searchParams['from'] = from.toISOString();
searchParams['to'] = to.toISOString();
return urlUtil.renderUrl(location.pathname, searchParams);
};
39 changes: 29 additions & 10 deletions src/services/migrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,37 @@ import { getLineFiltersVariable } from './variableGetters';
import { LineFilterCaseSensitive } from '../Components/ServiceScene/LineFilter/LineFilterScene';
import { LineFilterOp } from './filterTypes';
import { ServiceScene } from '../Components/ServiceScene/ServiceScene';
import { urlUtil } from '@grafana/data';

function removeEscapeChar(value: string, caseSensitive: boolean) {
const charsEscapedByEscapeRegExp = ['^', '$', '.', '*', '+', '?', '(', ')', '[', ']', '{', '}', '|'];
if (!caseSensitive) {
charsEscapedByEscapeRegExp.push('\\');
}
return value
.split('')
.filter((char, index, stringArray) => {
// We need to differentiate between user entered escape chars, and escape chars added by lodash escapeRegExp to return the same query results in urls from before the line filter regex feature
// Since there is no reverse of the escapeRegExp method provided by lodash we're essentially building our own "unescapeRegExp"
const nextChar = stringArray[index + 1];
const isNextCharRegex = charsEscapedByEscapeRegExp.includes(nextChar);
return !(char === '\\' && isNextCharRegex);
})
.join('');
}

/**
* Migrates old line filter to new variables
*/
export function migrateLineFilterV1(serviceScene: ServiceScene) {
const search = locationService.getSearch();
const search = urlUtil.getUrlSearchParams();

const deprecatedLineFilter = search.get('var-lineFilter');

if (!deprecatedLineFilter) {
const deprecatedLineFilterArray = search['var-lineFilter'];
if (!Array.isArray(deprecatedLineFilterArray) || !deprecatedLineFilterArray.length) {
return;
}
const deprecatedLineFilter = deprecatedLineFilterArray[0];
if (typeof deprecatedLineFilter !== 'string' || !deprecatedLineFilter) {
return;
}

Expand All @@ -26,7 +47,7 @@ export function migrateLineFilterV1(serviceScene: ServiceScene) {
{
key: LineFilterCaseSensitive.caseSensitive,
operator: LineFilterOp.match,
value: caseSensitiveMatches[1],
value: removeEscapeChar(caseSensitiveMatches[1], true),
keyLabel: '0',
},
],
Expand All @@ -41,16 +62,14 @@ export function migrateLineFilterV1(serviceScene: ServiceScene) {
{
key: LineFilterCaseSensitive.caseInsensitive,
operator: LineFilterOp.match,
value: caseInsensitiveMatches[1],
value: removeEscapeChar(caseInsensitiveMatches[1], false),
keyLabel: '0',
},
]);
});
}

// Remove from url without refreshing
const newLocation = locationService.getLocation();
search.delete('var-lineFilter');
newLocation.search = search.toString();
locationService.replace(newLocation.pathname + '?' + newLocation.search);
delete search['var-lineFilter'];
locationService.replace(urlUtil.renderUrl(location.pathname, search));
}
18 changes: 14 additions & 4 deletions tests/exploreServicesBreakDown.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1455,23 +1455,33 @@ test.describe('explore services breakdown page', () => {
});

test('line filter migration case sensitive', async ({ page }) => {
await explorePage.gotoServicesOldUrlLineFilters('tempo-distributor', true);
// Checks chars that are escaped on-behalf of the user and chars that are user-escaped, e.g. `\n` (`%5C%5Cn`) => \n, `%5C.` (\.) => .
const urlEncodedAndEscaped =
'%5C%5Cnpage_url%3D%22https:%2F%2Fgrafana%5C.net%2Fexplore%5C%3Fleft%3D%5C%7B%22datasource%22:%22grafanacloud-prom%22,%22queries%22:%5C%5B%5C%7B%22datasource%22:%5C%7B%22type%22:%22prometheus%22,%22uid%22:%22grafanacloud-prom%22%5C%7D,%22expr%22:%22max%20by%20%5C%28kube_cluster_name,%20kube_namespace%5C%29%20%5C%28quantile_over_time%5C%280%5C.85,%20kubernetes_state_pod_age%5C%7Bplatform%3D%22data%22,kube_namespace%21~%22data-dev%5C%7Cdata-stg-%5C.%5C%2B%22,pod_phase%3D%22pending%22%5C%7D%5C%5B5m%5C%5D%5C%29%5C%29%20%3E%20600%22,%22refId%22:%22A%22%5C%7D%5C%5D,%22range%22:%5C%7B%22from%22:%22now-1h%22,%22to%22:%22now%22%5C%7D%5C%7D%22%60';
const decodedAndUnescaped =
'`\\npage_url="https://grafana.net/explore?left={"datasource":"grafanacloud-prom","queries":[{"datasource":{"type":"prometheus","uid":"grafanacloud-prom"},"expr":"max by (kube_cluster_name, kube_namespace) (quantile_over_time(0.85, kubernetes_state_pod_age{platform="data",kube_namespace!~"data-dev|data-stg-.+",pod_phase="pending"}[5m])) > 600","refId":"A"}],"range":{"from":"now-1h","to":"now"}}"`';
await explorePage.gotoServicesOldUrlLineFilters('tempo-distributor', true, urlEncodedAndEscaped);
const firstLineFilterLoc = page.getByTestId(testIds.exploreServiceDetails.searchLogs).first();

await expect(firstLineFilterLoc).toHaveCount(1);
await expect(page.getByLabel('Enable case match').nth(0)).toHaveCount(1);
await expect(page.getByLabel('Disable case match')).toHaveCount(0);
await expect(firstLineFilterLoc).toHaveValue('debug');
await expect(firstLineFilterLoc).toHaveValue(decodedAndUnescaped);
});

test('line filter migration case insensitive', async ({ page }) => {
await explorePage.gotoServicesOldUrlLineFilters('tempo-distributor', false);
// The behavior for user entered escape chars differed between case sensitive/insensitive before the line filter regex feature, we want to preserve this bug in the migration so links from before this feature will return the same results
const urlEncodedAndEscaped =
'%5C%5Cnpage_url%3D%22https:%2F%2Fgrafana%5C.net%2Fexplore%5C%3Fleft%3D%5C%7B%22datasource%22:%22grafanacloud-prom%22,%22queries%22:%5C%5B%5C%7B%22datasource%22:%5C%7B%22type%22:%22prometheus%22,%22uid%22:%22grafanacloud-prom%22%5C%7D,%22expr%22:%22max%20by%20%5C%28kube_cluster_name,%20kube_namespace%5C%29%20%5C%28quantile_over_time%5C%280%5C.85,%20kubernetes_state_pod_age%5C%7Bplatform%3D%22data%22,kube_namespace%21~%22data-dev%5C%7Cdata-stg%22,pod_phase%3D%22pending%22%5C%7D%5C%5B5m%5C%5D%5C%29%5C%29%20%3E%20600%22,%22refId%22:%22A%22%5C%7D%5C%5D,%22range%22:%5C%7B%22from%22:%22now-1h%22,%22to%22:%22now%22%5C%7D%5C%7D%22%60';
const decodedAndUnescaped =
'\\\\npage_url="https://grafana.net/explore?left={"datasource":"grafanacloud-prom","queries":[{"datasource":{"type":"prometheus","uid":"grafanacloud-prom"},"expr":"max by (kube_cluster_name, kube_namespace) (quantile_over_time(0.85, kubernetes_state_pod_age{platform="data",kube_namespace!~"data-dev|data-stg",pod_phase="pending"}[5m])) > 600","refId":"A"}],"range":{"from":"now-1h","to":"now"}}"';
await explorePage.gotoServicesOldUrlLineFilters('tempo-distributor', false, urlEncodedAndEscaped);
const firstLineFilterLoc = page.getByTestId(testIds.exploreServiceDetails.searchLogs).first();

await expect(firstLineFilterLoc).toHaveCount(1);
await expect(page.getByLabel('Disable case match')).toHaveCount(1);
await expect(page.getByLabel('Enable case match')).toHaveCount(1);
await expect(firstLineFilterLoc).toHaveValue('debug');
await expect(firstLineFilterLoc).toHaveValue(decodedAndUnescaped);
});
});
});
10 changes: 7 additions & 3 deletions tests/fixtures/explore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,16 +236,20 @@ export class ExplorePage {
);
}

async gotoServicesOldUrlLineFilters(serviceName = 'tempo-distributor', caseSensitive?: boolean) {
async gotoServicesOldUrlLineFilters(
serviceName = 'tempo-distributor',
caseSensitive?: boolean,
lineFilterValue = 'debug'
) {
if (caseSensitive) {
await this.page.goto(
// case insensitive
`/a/${pluginJson.id}/explore/service/tempo-distributor/logs?mode=service_details&patterns=[]&var-lineFilter=%7C~%20%60%28%3Fi%29debug%60&var-filters=service_name|=|${serviceName}&var-logsFormat= | logfmt`
`/a/${pluginJson.id}/explore/service/tempo-distributor/logs?mode=service_details&patterns=[]&var-lineFilter=%7C~%20%60%28%3Fi%29%60${lineFilterValue}%60&var-filters=service_name|=|${serviceName}&var-logsFormat= | logfmt`
);
} else {
await this.page.goto(
// case insensitive
`/a/${pluginJson.id}/explore/service/tempo-distributor/logs?mode=service_details&patterns=[]&var-lineFilter=%7C%3D%20%60debug%60&var-filters=service_name|=|${serviceName}&var-logsFormat= | logfmt`
`/a/${pluginJson.id}/explore/service/tempo-distributor/logs?mode=service_details&patterns=[]&var-lineFilter=%7C%3D%20%60${lineFilterValue}%60&var-filters=service_name|=|${serviceName}&var-logsFormat= | logfmt`
);
}
}
Expand Down

0 comments on commit c435654

Please sign in to comment.