mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-04 00:08:55 +00:00
fix(dashboards): range warning text, dashed comparison series, View Panel variables (#16013) (#16232)
This commit is contained in:
@@ -15,6 +15,8 @@
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { flushPromises, mount, VueWrapper } from "@vue/test-utils";
|
||||
import { inject } from "vue";
|
||||
import type { useVariablesManager } from "@/composables/dashboard/useVariablesManager";
|
||||
import ViewPanel from "./ViewPanel.vue";
|
||||
import config from "@/aws-exports";
|
||||
import PanelErrorButtons from "@/components/dashboards/PanelErrorButtons.vue";
|
||||
@@ -469,6 +471,59 @@ describe("ViewPanel", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("variable values passed from the dashboard", () => {
|
||||
const levelVariable = {
|
||||
type: "query_values",
|
||||
name: "level",
|
||||
label: "Level",
|
||||
query_data: { stream_type: "logs", stream: "app_logs", field: "level" },
|
||||
value: "",
|
||||
options: [],
|
||||
};
|
||||
const mountWithPassed = async (passed: Record<string, unknown>) => {
|
||||
mockGetDashboard.mockResolvedValue({
|
||||
title: "Test Dashboard",
|
||||
tabs: [{ tabId: "default", panels: [] }],
|
||||
variables: { list: [levelVariable], showDynamicFilters: false },
|
||||
});
|
||||
let manager: ReturnType<typeof useVariablesManager> | undefined;
|
||||
wrapper = createWrapper(
|
||||
{ initialVariableValues: { values: [{ ...levelVariable, ...passed }] } },
|
||||
{
|
||||
VariablesValueSelector: {
|
||||
template: "<div />",
|
||||
setup: () => {
|
||||
manager = inject<ReturnType<typeof useVariablesManager>>("variablesManager");
|
||||
return {};
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
await flushPromises();
|
||||
return manager?.variablesData.global.find((v) => v.name === "level");
|
||||
};
|
||||
|
||||
it("loads a variable itself when the dashboard had not loaded it yet", async () => {
|
||||
const level = await mountWithPassed({
|
||||
value: null,
|
||||
isLoading: true,
|
||||
isVariablePartialLoaded: false,
|
||||
});
|
||||
expect(level?.isVariablePartialLoaded).toBe(false);
|
||||
});
|
||||
|
||||
it("reuses a value the dashboard already loaded", async () => {
|
||||
const level = await mountWithPassed({
|
||||
value: "debug",
|
||||
isLoading: false,
|
||||
isVariableLoadingPending: false,
|
||||
isVariablePartialLoaded: true,
|
||||
});
|
||||
expect(level?.value).toBe("debug");
|
||||
expect(level?.isVariablePartialLoaded).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Component Initialization", () => {
|
||||
it("should display panel title", () => {
|
||||
wrapper = createWrapper();
|
||||
|
||||
@@ -764,20 +764,7 @@ export default defineComponent({
|
||||
props.initialVariableValues.values &&
|
||||
props.initialVariableValues.values.length > 0
|
||||
) {
|
||||
// Update variablesManager with passed values
|
||||
props.initialVariableValues.values.forEach((passedVar: any) => {
|
||||
// Find and update the variable in the manager (global scope)
|
||||
const globalVar = variablesManager.variablesData.global.find(
|
||||
(v: any) => v.name === passedVar.name,
|
||||
);
|
||||
if (globalVar) {
|
||||
globalVar.value = passedVar.value;
|
||||
globalVar.isVariablePartialLoaded = true;
|
||||
globalVar.isLoading = false;
|
||||
// Set pending to false to prevent API call
|
||||
globalVar.isVariableLoadingPending = false;
|
||||
}
|
||||
});
|
||||
variablesManager.adoptLoadedValues(props.initialVariableValues.values);
|
||||
|
||||
// Also populate currentVariablesDataRef with passed values
|
||||
Object.assign(currentVariablesDataRef, props.initialVariableValues);
|
||||
|
||||
@@ -1260,6 +1260,40 @@ describe("useVariablesManager live apply and commit-once", () => {
|
||||
expect(pod.isVariableLoadingPending).toBe(true);
|
||||
});
|
||||
|
||||
describe("adoptLoadedValues", () => {
|
||||
const adopt = async (passed: Partial<VariableConfig> & Record<string, unknown>) => {
|
||||
const manager = useVariablesManager(gt);
|
||||
await manager.initialize(queryChainConfig(), {});
|
||||
manager.adoptLoadedValues([{ name: "ns", type: "query_values", ...passed }]);
|
||||
return manager.variablesData.global;
|
||||
};
|
||||
|
||||
it("keeps a loaded value and schedules its children", async () => {
|
||||
const [ns, pod] = await adopt({ value: "a", isVariablePartialLoaded: true });
|
||||
expect(ns.value).toBe("a");
|
||||
expect(ns.isVariableLoadingPending).toBe(false);
|
||||
expect(pod.isVariableLoadingPending).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps a loaded value that is only being refreshed", async () => {
|
||||
const [ns] = await adopt({ value: "a", isVariablePartialLoaded: true, isLoading: true });
|
||||
expect(ns.value).toBe("a");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["still loading", { value: null, isVariablePartialLoaded: false, isLoading: true }],
|
||||
[
|
||||
"waiting to reload",
|
||||
{ value: "a", isVariablePartialLoaded: true, isVariableLoadingPending: true },
|
||||
],
|
||||
])("leaves a value that is %s to its own fetch", async (_case, passed) => {
|
||||
const [ns, pod] = await adopt(passed);
|
||||
expect(ns.value).toBe(null);
|
||||
expect(ns.isVariableLoadingPending).toBe(true);
|
||||
expect(pod.isVariableLoadingPending).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
it("an aborted dependent load releases the commit", async () => {
|
||||
const { manager, service, commits } = await setupSettled();
|
||||
manager.setLiveMode(true);
|
||||
|
||||
@@ -1112,6 +1112,24 @@ export const useVariablesManager = (t: TranslateFn) => {
|
||||
scheduleChildrenOfRestored(restoredKeys);
|
||||
};
|
||||
|
||||
/** Takes global values another view already loaded, so they are not fetched again, and starts their children's loads. */
|
||||
const adoptLoadedValues = (passed: Partial<VariableRuntimeState>[]) => {
|
||||
const adopted = passed.flatMap((p) => {
|
||||
// A query value mid-fetch or due to reload after its parent changed is not a value to keep.
|
||||
if (p.type === "query_values" && (!p.isVariablePartialLoaded || p.isVariableLoadingPending))
|
||||
return [];
|
||||
const v = variablesData.global.find((g) => g.name === p.name);
|
||||
if (!v) return [];
|
||||
v.value = p.value;
|
||||
v.isVariablePartialLoaded = true;
|
||||
v.isLoading = false;
|
||||
v.isVariableLoadingPending = false;
|
||||
return [v];
|
||||
});
|
||||
recordLoadedAgainst(adopted);
|
||||
scheduleChildrenOfRestored(adopted.map((v) => getVariableKey(v.name, v.scope)));
|
||||
};
|
||||
|
||||
interface ParsedUrlKey {
|
||||
name: string;
|
||||
scope: "global" | "tabs" | "panels";
|
||||
@@ -1262,6 +1280,7 @@ export const useVariablesManager = (t: TranslateFn) => {
|
||||
isVariableReady,
|
||||
getDependentVariables,
|
||||
loadFromUrl,
|
||||
adoptLoadedValues,
|
||||
getUrlParams,
|
||||
};
|
||||
};
|
||||
|
||||
@@ -238,6 +238,12 @@ describe("getDistinctSeriesColors", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("gives pie slices info and warn different colours", () => {
|
||||
const colors = palette("base.css");
|
||||
const assigned = getDistinctSeriesColors(["debug", "error", "info", "warn"], colors);
|
||||
expect(assigned.get("info")).not.toBe(assigned.get("warn"));
|
||||
});
|
||||
|
||||
it("reuses colours only once every slot is taken", () => {
|
||||
const names = Array.from({ length: 14 }, (_, i) => `series-${i}`);
|
||||
const assigned = getDistinctSeriesColors(names, palette("base.css"));
|
||||
|
||||
@@ -89,7 +89,11 @@ import {
|
||||
labelFreeName,
|
||||
} from "./promql/shared/legendBuilder";
|
||||
import { getCachedSemanticGroups } from "@/utils/semanticGroupsCache";
|
||||
import { getPropsByChartTypeForSeries, getSeriesItemOpacity } from "./promqlChartSeriesProps";
|
||||
import {
|
||||
getPropsByChartTypeForSeries,
|
||||
getSeriesItemOpacity,
|
||||
getShiftedLineStyle,
|
||||
} from "./promqlChartSeriesProps";
|
||||
import { applyMeasuredYAxisLeftInset } from "./chartDimensionUtils";
|
||||
import { buildSeriesMarkLine } from "./promql/shared/markLines";
|
||||
import { legendExprOf, legendFallbackOf } from "@/utils/dashboard/promql/formula";
|
||||
@@ -1191,13 +1195,10 @@ export const convertPromQLData = async (
|
||||
// Twins share their primary's colour, so the line style is what tells them apart.
|
||||
...(nameSuffixes[index]
|
||||
? {
|
||||
lineStyle: {
|
||||
...seriesPropsBasedOnChartType?.lineStyle,
|
||||
type:
|
||||
explorerOverlays && seriesRoles[index] !== "forecast"
|
||||
? "dotted"
|
||||
: "dashed",
|
||||
},
|
||||
lineStyle: getShiftedLineStyle(
|
||||
seriesPropsBasedOnChartType?.lineStyle,
|
||||
explorerOverlays && seriesRoles[index] !== "forecast" ? "dotted" : "dashed",
|
||||
),
|
||||
}
|
||||
: {}),
|
||||
...getAreaStyleOverride(
|
||||
|
||||
@@ -4176,6 +4176,62 @@ describe("convertSQLData", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["line", { lineStyle: { type: "dashed" } }],
|
||||
["bar", { itemStyle: { opacity: 0.5 } }],
|
||||
])(
|
||||
"draws a compared %s series in its series' colour, set apart by style",
|
||||
async (type, style) => {
|
||||
const searchData = [
|
||||
[{ timestamp: "2023-01-01", value: 10 }],
|
||||
[{ timestamp: "2023-01-02", value: 20 }],
|
||||
];
|
||||
const resultMetaData = { value: [[mockResultMetaData[0]], [mockResultMetaData[0]]] };
|
||||
const metadata = {
|
||||
queries: [
|
||||
{ panelQueryIndex: 0, timeRangeGap: { seconds: 0, periodAsStr: "" } },
|
||||
{
|
||||
panelQueryIndex: 0,
|
||||
timeRangeGap: { seconds: 3_600_000, periodAsStr: "1 Hour ago" },
|
||||
},
|
||||
],
|
||||
};
|
||||
const schema = {
|
||||
...mockPanelSchema,
|
||||
type,
|
||||
queries: [
|
||||
{
|
||||
fields: {
|
||||
x: [{ alias: "timestamp", label: "Timestamp" }],
|
||||
y: [{ alias: "value", label: "Count" }],
|
||||
z: [],
|
||||
breakdown: [],
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
const result = await convertMultiSQLData(
|
||||
schema,
|
||||
searchData,
|
||||
mockStore,
|
||||
mockChartPanelRef,
|
||||
mockHoveredSeriesState,
|
||||
resultMetaData,
|
||||
metadata,
|
||||
mockChartPanelStyle,
|
||||
mockAnnotations,
|
||||
);
|
||||
|
||||
const [current, compared] = result.options.series.filter(
|
||||
(s: { name?: string }) => s.name,
|
||||
);
|
||||
expect(compared.name).toBe("Count (1 Hour ago)");
|
||||
expect(compared.color).toBe(current.color);
|
||||
expect(compared).toMatchObject(style);
|
||||
expect(current.lineStyle?.type).not.toBe("dashed");
|
||||
expect(current.itemStyle?.opacity).toBeUndefined();
|
||||
},
|
||||
);
|
||||
it("should handle metadata queries without periodAsStr", async () => {
|
||||
const schema = mockPanelSchema;
|
||||
const searchData = [
|
||||
|
||||
@@ -23,6 +23,7 @@
|
||||
*/
|
||||
import { convertSQLChartData } from "./sql";
|
||||
import { applySeriesColorMappings } from "./chartColorUtils";
|
||||
import { getSeriesItemOpacity, getShiftedLineStyle } from "./promqlChartSeriesProps";
|
||||
import { layoutMetricGrid } from "./metricGrid";
|
||||
import { calculateGridPositions, getTrellisGrid } from "./calculateGridForSubPlot";
|
||||
import { formatUnitValue, getUnitValue } from "./convertDataIntoUnitValue";
|
||||
@@ -651,6 +652,20 @@ export const convertMultiSQLData = async (
|
||||
// A right-clicked series alerts on its own panel query, a shifted one on its parent's.
|
||||
const seriesRoleAt = (i: number) =>
|
||||
Number(metadata?.queries?.[i]?.timeRangeGap?.seconds) ? "shifted" : "primary";
|
||||
// As in PromQL, a shifted series keeps its primary's colour and is told apart by its style.
|
||||
const roleStyle = (
|
||||
it: { lineStyle?: Record<string, unknown>; itemStyle?: Record<string, unknown> },
|
||||
i: number,
|
||||
) =>
|
||||
seriesRoleAt(i) === "shifted"
|
||||
? {
|
||||
lineStyle: getShiftedLineStyle(it.lineStyle),
|
||||
itemStyle: {
|
||||
...it.itemStyle,
|
||||
...getSeriesItemOpacity(chartType, panelSchema?.config, true),
|
||||
},
|
||||
}
|
||||
: {};
|
||||
|
||||
// Label Q1 series
|
||||
if (options[0].options.series) {
|
||||
@@ -666,6 +681,7 @@ export const convertMultiSQLData = async (
|
||||
_queryIndex: 0,
|
||||
_panelQueryIndex: pIdx0,
|
||||
_seriesRole: seriesRoleAt(0),
|
||||
...roleStyle(it, 0),
|
||||
};
|
||||
});
|
||||
}
|
||||
@@ -687,6 +703,7 @@ export const convertMultiSQLData = async (
|
||||
_queryIndex: i,
|
||||
_panelQueryIndex: panelQueryIndex,
|
||||
_seriesRole: seriesRoleAt(i),
|
||||
...roleStyle(it, i),
|
||||
// C4: prefix stack names to prevent cross-query stacking
|
||||
stack: it.stack ? `q${i}-${it.stack}` : it.stack,
|
||||
};
|
||||
|
||||
@@ -57,7 +57,8 @@ export const toPartitions = (entry: unknown): Record<string, any>[] => {
|
||||
return entry && typeof entry === "object" ? [entry as Record<string, any>] : [];
|
||||
};
|
||||
|
||||
const toMessages = (value: unknown): string[] => {
|
||||
/** The backend sends `function_error` as a list; blank and non-text entries are dropped. */
|
||||
export const toMessages = (value: unknown): string[] => {
|
||||
if (Array.isArray(value)) {
|
||||
return value.filter((m): m is string => typeof m === "string" && m.trim() !== "");
|
||||
}
|
||||
|
||||
@@ -144,6 +144,12 @@ export const getPropsByChartTypeForSeries = (type: string, config?: SeriesStyleC
|
||||
}
|
||||
};
|
||||
|
||||
/** A time-shifted twin keeps its primary's colour, so its line is dashed (dotted in the Explorer) to tell the two apart. */
|
||||
export const getShiftedLineStyle = (
|
||||
lineStyle: Record<string, unknown> | undefined,
|
||||
type: "dashed" | "dotted" = "dashed",
|
||||
): Record<string, unknown> => ({ ...lineStyle, type });
|
||||
|
||||
/** Item opacity for a PromQL series; a time-shifted bar or scatter twin is drawn at half its base opacity. */
|
||||
export const getSeriesItemOpacity = (
|
||||
type: string,
|
||||
|
||||
@@ -949,6 +949,24 @@ describe("processQueryMetadataErrors", () => {
|
||||
expect(result).toContain("Simple error");
|
||||
});
|
||||
|
||||
it("keeps a chunk's single function_error string whole", () => {
|
||||
const metadata = [[{ function_error: "Whole message" }]];
|
||||
|
||||
expect(processQueryMetadataErrors(metadata)).toBe("Whole message");
|
||||
});
|
||||
|
||||
it("drops blank and non-text entries of a function_error list", () => {
|
||||
const metadata = [[{ function_error: ["", "Error F", { code: 1 }] }]];
|
||||
|
||||
expect(processQueryMetadataErrors(metadata)).toBe("Error F");
|
||||
});
|
||||
|
||||
it("puts each message of a flat function_error list on its own line", () => {
|
||||
const metadata = [{ function_error: ["Error D", "Error E"] }];
|
||||
|
||||
expect(processQueryMetadataErrors(metadata)).toBe("Error D\nError E");
|
||||
});
|
||||
|
||||
it("deduplicates repeated messages", () => {
|
||||
const metadata = [
|
||||
[
|
||||
|
||||
@@ -4,6 +4,7 @@ import config from "../aws-exports";
|
||||
import { useStore } from "vuex";
|
||||
import CronExpressionParser from "cron-parser";
|
||||
import { getFunctionErrorMessage } from "@/utils/timezone";
|
||||
import { toMessages } from "@/utils/dashboard/panelDiagnostics";
|
||||
import { gt, type TranslateFn } from "@/types/i18n";
|
||||
|
||||
export const getPath = () => {
|
||||
@@ -479,7 +480,7 @@ export const processQueryMetadataErrors = (metadata: any, timezone: string = "UT
|
||||
);
|
||||
combinedWarnings.push(combinedMessage);
|
||||
} else if (chunk?.function_error) {
|
||||
combinedWarnings.push(...chunk.function_error);
|
||||
combinedWarnings.push(...toMessages(chunk.function_error));
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -494,7 +495,7 @@ export const processQueryMetadataErrors = (metadata: any, timezone: string = "UT
|
||||
);
|
||||
combinedWarnings.push(combinedMessage);
|
||||
} else if (query?.function_error) {
|
||||
combinedWarnings.push(query.function_error);
|
||||
combinedWarnings.push(...toMessages(query.function_error));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -403,6 +403,19 @@ describe("getFunctionErrorMessage", () => {
|
||||
expect(result).toContain("Some error");
|
||||
});
|
||||
|
||||
it("prints the backend's message list as lines, not as a JSON array", () => {
|
||||
const result = getFunctionErrorMessage(
|
||||
["Query duration is modified due to query range restriction of 1 hour", "Second note"],
|
||||
1_700_000_000_000_000,
|
||||
1_700_003_600_000_000,
|
||||
"UTC",
|
||||
);
|
||||
expect(result).toContain(
|
||||
"Query duration is modified due to query range restriction of 1 hour\nSecond note (Data returned for:",
|
||||
);
|
||||
expect(result).not.toContain("[");
|
||||
});
|
||||
|
||||
it("returns original message when timestampToTimezoneDate throws (invalid timezone)", () => {
|
||||
// Pass an invalid timezone string to make luxon throw inside timestampToTimezoneDate
|
||||
const result = getFunctionErrorMessage(
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import { DateTime } from "luxon";
|
||||
import { durationFormatter } from "@/utils/formatters";
|
||||
import { gt, raw } from "@/types/i18n";
|
||||
import { toMessages } from "@/utils/dashboard/panelDiagnostics";
|
||||
|
||||
let moment: any;
|
||||
let momentInitialized = false;
|
||||
@@ -190,11 +191,12 @@ export const getDuration = (createdAt: number) => {
|
||||
};
|
||||
|
||||
export const getFunctionErrorMessage = (
|
||||
message: string,
|
||||
functionError: string | string[],
|
||||
newStartTime: number,
|
||||
newEndTime: number,
|
||||
timezone = "UTC",
|
||||
) => {
|
||||
const message = toMessages(functionError).join("\n");
|
||||
try {
|
||||
const startTimeFormatted = timestampToTimezoneDate(
|
||||
newStartTime / 1000,
|
||||
|
||||
Reference in New Issue
Block a user