Skip to content

Commit

Permalink
Fix alerts created with comparison
Browse files Browse the repository at this point in the history
  • Loading branch information
AdityaHegde committed Dec 11, 2024
1 parent 349c759 commit 16c6c1f
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 113 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export async function getDashboardFromAggregationRequest({
dashboard.visibleMeasureKeys = new Set(
req.measures?.map((m) => m.name ?? "") ?? [],
);
console.log(req.measures);

// if the selected sort is a measure set it to leaderboardMeasureName
if (
Expand Down
143 changes: 33 additions & 110 deletions web-admin/src/features/dashboards/query-mappers/utils.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,7 @@
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 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 @@ -18,17 +10,15 @@ import {
TimeRangePreset,
} from "@rilldata/web-common/lib/time/types";
import {
getQueryServiceMetricsViewAggregationQueryKey,
getQueryServiceMetricsViewTimeRangeQueryKey,
getRuntimeServiceGetExploreQueryKey,
queryServiceMetricsViewAggregation,
type QueryServiceMetricsViewAggregationBody,
queryServiceMetricsViewTimeRange,
runtimeServiceGetExplore,
type V1Expression,
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 +49,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,96 +118,6 @@ export function getSelectedTimeRange(
return selectedTimeRange;
}

export async function convertExprToToplist(
queryClient: QueryClient,
instanceId: string,
metricsView: string,
dimensionName: string,
measureName: string,
timeRange: V1TimeRange | undefined,
comparisonTimeRange: V1TimeRange | undefined,
executionTime: string,
where: V1Expression | undefined,
having: V1Expression,
) {
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 toplist = await queryClient.fetchQuery({
queryKey: getQueryServiceMetricsViewAggregationQueryKey(
instanceId,
metricsView,
toplistBody,
),
queryFn: () =>
queryServiceMetricsViewAggregation(instanceId, metricsView, toplistBody),
});
if (!toplist.data) {
return undefined;
}
return createInExpression(
dimensionName,
toplist.data.map((t) => t[dimensionName]),
);
}

const ExploreNameRegex = /\/explore\/((?:\w|-)+)/;
export function getExploreName(webOpenPath: string) {
const matches = ExploreNameRegex.exec(webOpenPath);
Expand All @@ -231,7 +133,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 +153,32 @@ 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,
});
}

console.log(exploreState);
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
4 changes: 3 additions & 1 deletion web-common/src/features/alerts/EditAlertForm.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@
{ query: { queryClient } },
);
$: exploreName = getExploreName(alertSpec.annotations?.web_open_path ?? "");
const exploreName = getExploreName(
alertSpec.annotations?.web_open_path ?? "",
);
const formState = createForm<AlertFormValues>({
initialValues: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,7 @@ export function mapMeasureFilterToExpr(
);
}
}

export function stripMeasureSuffix(measureName: string) {
return measureName.replace(HasSuffixRegex, "");
}
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ function toVisibleMeasuresUrlParam(

const measures = [...exploreState.visibleMeasureKeys];
const presetMeasures = preset.measures ?? exploreSpec.measures ?? [];
console.log(measures, presetMeasures);
if (arrayUnorderedEquals(measures, presetMeasures)) {
return undefined;
}
Expand Down
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

0 comments on commit 16c6c1f

Please sign in to comment.