Skip to content

Commit

Permalink
fix: alerts opened with new url params. (#6257)
Browse files Browse the repository at this point in the history
* Fix alerts created with comparison

* Add back converting of unsupported filter to IN filter
  • Loading branch information
AdityaHegde authored Dec 12, 2024
1 parent 3e30dad commit b853554
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 105 deletions.
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import type { QueryMapperArgs } from "@rilldata/web-admin/features/dashboards/query-mappers/types";
import { fillTimeRange } from "@rilldata/web-admin/features/dashboards/query-mappers/utils";
import {
convertQueryFilterToToplistQuery,
fillTimeRange,
} from "@rilldata/web-admin/features/dashboards/query-mappers/utils";
import {
ComparisonDeltaAbsoluteSuffix,
ComparisonDeltaRelativeSuffix,
ComparisonPercentOfTotal,
mapExprToMeasureFilter,
measureHasSuffix,
} from "@rilldata/web-common/features/dashboards/filters/measure-filters/measure-filter-entry";
import { splitWhereFilter } from "@rilldata/web-common/features/dashboards/filters/measure-filters/measure-filter-utils";
import { mergeFilters } from "@rilldata/web-common/features/dashboards/pivot/pivot-merge-filters";
import {
SortDirection,
SortType,
Expand Down Expand Up @@ -72,9 +77,20 @@ export async function getDashboardFromAggregationRequest({
}
if (req.having?.cond?.exprs?.length && req.dimensions?.[0]?.name) {
const dimension = req.dimensions[0].name;
if (
if (exprHasComparison(req.having)) {
const expr = await convertQueryFilterToToplistQuery(
instanceId,
explore.metricsView ?? "",
req,
dimension,
);
dashboard.whereFilter =
mergeFilters(
dashboard.whereFilter ?? createAndExpression([]),
createAndExpression([expr]),
) ?? createAndExpression([]);
} else if (
req.having.cond.exprs.length > 1 ||
exprHasComparison(req.having) ||
dashboard.dimensionThresholdFilters.length > 0
) {
const extraFilter = createSubQueryExpression(
Expand Down Expand Up @@ -111,8 +127,12 @@ export async function getDashboardFromAggregationRequest({
}

dashboard.visibleMeasureKeys = new Set(
req.measures?.map((m) => m.name ?? "") ?? [],
req.measures
?.map((m) => m.name ?? "")
.filter((m) => !measureHasSuffix(m)) ?? [],
);
dashboard.allMeasuresVisible =
dashboard.visibleMeasureKeys.size === explore.measures?.length;

// if the selected sort is a measure set it to leaderboardMeasureName
if (
Expand Down
149 changes: 50 additions & 99 deletions web-admin/src/features/dashboards/query-mappers/utils.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
import {
ComparisonDeltaAbsoluteSuffix,
ComparisonDeltaRelativeSuffix,
ComparisonPercentOfTotal,
} from "@rilldata/web-common/features/dashboards/filters/measure-filters/measure-filter-entry";
import {
createInExpression,
forEachIdentifier,
} from "@rilldata/web-common/features/dashboards/stores/filter-utils";
import { createInExpression } from "@rilldata/web-common/features/dashboards/stores/filter-utils";
import type { MetricsExplorerEntity } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity";
import { PreviousCompleteRangeMap } from "@rilldata/web-common/features/dashboards/time-controls/time-range-mappers";
import { convertExploreStateToURLSearchParams } from "@rilldata/web-common/features/dashboards/url-state/convertExploreStateToURLSearchParams";
import { getDefaultExplorePreset } from "@rilldata/web-common/features/dashboards/url-state/getDefaultExplorePreset";
import { queryClient } from "@rilldata/web-common/lib/svelte-query/globalQueryClient";
import { isoDurationToFullTimeRange } from "@rilldata/web-common/lib/time/ranges/iso-ranges";
import {
Expand All @@ -19,16 +12,18 @@ import {
} from "@rilldata/web-common/lib/time/types";
import {
getQueryServiceMetricsViewAggregationQueryKey,
getQueryServiceMetricsViewTimeRangeQueryKey,
getRuntimeServiceGetExploreQueryKey,
queryServiceMetricsViewAggregation,
type QueryServiceMetricsViewAggregationBody,
queryServiceMetricsViewTimeRange,
runtimeServiceGetExplore,
type V1Expression,
type V1MetricsViewAggregationRequest,
type V1MetricsViewTimeRangeResponse,
type V1TimeRange,
type V1TimeRangeSummary,
} from "@rilldata/web-common/runtime-client";
import { runtime } from "@rilldata/web-common/runtime-client/runtime-store";
import type { QueryClient } from "@tanstack/svelte-query";
import { get } from "svelte/store";

// We are manually sending in duration, offset and round to grain for previous complete ranges.
Expand Down Expand Up @@ -59,8 +54,10 @@ export function fillTimeRange(

if (reqComparisonTimeRange) {
if (
!reqComparisonTimeRange.isoOffset &&
reqComparisonTimeRange.isoDuration
(!reqComparisonTimeRange.isoOffset &&
reqComparisonTimeRange.isoDuration) ||
(reqComparisonTimeRange.isoOffset &&
reqComparisonTimeRange.isoOffset === reqComparisonTimeRange.isoDuration)
) {
dashboard.selectedComparisonTimeRange = {
name: TimeComparisonOption.CONTIGUOUS,
Expand Down Expand Up @@ -126,103 +123,37 @@ export function getSelectedTimeRange(
return selectedTimeRange;
}

export async function convertExprToToplist(
queryClient: QueryClient,
const ExploreNameRegex = /\/explore\/((?:\w|-)+)/;
export function getExploreName(webOpenPath: string) {
const matches = ExploreNameRegex.exec(webOpenPath);
if (!matches || matches.length < 1) return "";
return matches[1];
}

export async function convertQueryFilterToToplistQuery(
instanceId: string,
metricsView: string,
dimensionName: string,
measureName: string,
timeRange: V1TimeRange | undefined,
comparisonTimeRange: V1TimeRange | undefined,
executionTime: string,
where: V1Expression | undefined,
having: V1Expression,
req: V1MetricsViewAggregationRequest,
dimension: string,
) {
let hasPercentOfTotals = false;
forEachIdentifier(having, (_, ident) => {
if (ident?.endsWith(ComparisonPercentOfTotal)) {
hasPercentOfTotals = true;
}
});

const toplistBody: QueryServiceMetricsViewAggregationBody = {
measures: [
{
name: measureName,
},
...(comparisonTimeRange
? [
{
name: measureName + ComparisonDeltaAbsoluteSuffix,
comparisonDelta: { measure: measureName },
},
{
name: measureName + ComparisonDeltaRelativeSuffix,
comparisonRatio: { measure: measureName },
},
]
: []),
...(hasPercentOfTotals
? [
{
name: measureName + ComparisonPercentOfTotal,
percentOfTotal: { measure: measureName },
},
]
: []),
],
dimensions: [{ name: dimensionName }],
...(timeRange
? {
timeRange: {
...timeRange,
end: executionTime,
},
}
: {}),
...(comparisonTimeRange
? {
comparisonTimeRange: {
...comparisonTimeRange,
end: executionTime,
},
}
: {}),
where,
having,
sort: [
{
name: measureName,
desc: false,
},
],
limit: "250",
const params = <QueryServiceMetricsViewAggregationBody>{
...req,
};
const toplist = await queryClient.fetchQuery({
queryKey: getQueryServiceMetricsViewAggregationQueryKey(
instanceId,
metricsView,
toplistBody,
params,
),
queryFn: () =>
queryServiceMetricsViewAggregation(instanceId, metricsView, toplistBody),
queryServiceMetricsViewAggregation(instanceId, metricsView, params),
});
if (!toplist.data) {
return undefined;
}
return createInExpression(
dimensionName,
toplist.data.map((t) => t[dimensionName]),
dimension,
toplist.data?.map((d) => d[dimension]) ?? [],
);
}

const ExploreNameRegex = /\/explore\/((?:\w|-)+)/;
export function getExploreName(webOpenPath: string) {
const matches = ExploreNameRegex.exec(webOpenPath);
if (!matches || matches.length < 1) return "";
return matches[1];
}

export async function getExplorePageUrl(
curPageUrl: URL,
organization: string,
Expand All @@ -231,7 +162,7 @@ export async function getExplorePageUrl(
exploreState: MetricsExplorerEntity,
) {
const instanceId = get(runtime).instanceId;
const { explore } = await queryClient.fetchQuery({
const { explore, metricsView } = await queryClient.fetchQuery({
queryFn: ({ signal }) =>
runtimeServiceGetExplore(
instanceId,
Expand All @@ -251,11 +182,31 @@ export async function getExplorePageUrl(
const url = new URL(`${curPageUrl.protocol}//${curPageUrl.host}`);
url.pathname = `/${organization}/${project}/explore/${exploreName}`;

const exploreSpec = explore?.explore?.state?.validSpec;
const exploreSpec = explore?.explore?.state?.validSpec ?? {};
const metricsViewName = exploreSpec.metricsView;

let fullTimeRange: V1MetricsViewTimeRangeResponse | undefined;
if (
metricsView?.metricsView?.state?.validSpec?.timeDimension &&
metricsViewName
) {
fullTimeRange = await queryClient.fetchQuery({
queryFn: () =>
queryServiceMetricsViewTimeRange(instanceId, metricsViewName, {}),
queryKey: getQueryServiceMetricsViewTimeRangeQueryKey(
instanceId,
metricsViewName,
{},
),
staleTime: Infinity,
cacheTime: Infinity,
});
}

url.search = convertExploreStateToURLSearchParams(
exploreState,
exploreSpec ?? {},
exploreSpec?.defaultPreset ?? {},
exploreSpec,
getDefaultExplorePreset(exploreSpec, fullTimeRange),
);
return url.toString();
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
goto(`/${organization}/${project}/-/alerts/${alertId}`);
}
$: if ($dashboardStateForAlert.data) {
$: if ($dashboardStateForAlert?.data) {
void gotoExplorePage();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,10 @@ export function mapMeasureFilterToExpr(
);
}
}

export function measureHasSuffix(measureName: string) {
return HasSuffixRegex.test(measureName);
}
export function stripMeasureSuffix(measureName: string) {
return measureName.replace(HasSuffixRegex, "");
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { stripMeasureSuffix } from "@rilldata/web-common/features/dashboards/filters/measure-filters/measure-filter-entry";
import { base64ToProto } from "@rilldata/web-common/features/dashboards/proto-state/fromProto";
import {
createAndExpression,
Expand Down Expand Up @@ -214,7 +215,11 @@ function fromFilterUrlParam(
return false;
}

if (measures.has(ident) || dimensions.has(ident)) {
if (
measures.has(ident) ||
measures.has(stripMeasureSuffix(ident)) ||
dimensions.has(ident)
) {
return true;
}
missingFields.push(ident);
Expand Down

1 comment on commit b853554

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.