diff --git a/web/src/components/dashboards/viewPanel/ViewPanel.spec.ts b/web/src/components/dashboards/viewPanel/ViewPanel.spec.ts index 144cac89c3..ab6f01f23b 100644 --- a/web/src/components/dashboards/viewPanel/ViewPanel.spec.ts +++ b/web/src/components/dashboards/viewPanel/ViewPanel.spec.ts @@ -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) => { + mockGetDashboard.mockResolvedValue({ + title: "Test Dashboard", + tabs: [{ tabId: "default", panels: [] }], + variables: { list: [levelVariable], showDynamicFilters: false }, + }); + let manager: ReturnType | undefined; + wrapper = createWrapper( + { initialVariableValues: { values: [{ ...levelVariable, ...passed }] } }, + { + VariablesValueSelector: { + template: "
", + setup: () => { + manager = inject>("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(); diff --git a/web/src/components/dashboards/viewPanel/ViewPanel.vue b/web/src/components/dashboards/viewPanel/ViewPanel.vue index fdd7038c6b..415410eb01 100644 --- a/web/src/components/dashboards/viewPanel/ViewPanel.vue +++ b/web/src/components/dashboards/viewPanel/ViewPanel.vue @@ -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); diff --git a/web/src/composables/dashboard/useVariablesManager.spec.ts b/web/src/composables/dashboard/useVariablesManager.spec.ts index f387fa90db..5c06a5a7b9 100644 --- a/web/src/composables/dashboard/useVariablesManager.spec.ts +++ b/web/src/composables/dashboard/useVariablesManager.spec.ts @@ -1260,6 +1260,40 @@ describe("useVariablesManager live apply and commit-once", () => { expect(pod.isVariableLoadingPending).toBe(true); }); + describe("adoptLoadedValues", () => { + const adopt = async (passed: Partial & Record) => { + 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); diff --git a/web/src/composables/dashboard/useVariablesManager.ts b/web/src/composables/dashboard/useVariablesManager.ts index 8f8a266dcb..c2550217b3 100644 --- a/web/src/composables/dashboard/useVariablesManager.ts +++ b/web/src/composables/dashboard/useVariablesManager.ts @@ -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[]) => { + 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, }; }; diff --git a/web/src/utils/dashboard/colorPalette.spec.ts b/web/src/utils/dashboard/colorPalette.spec.ts index 0d2182ebd2..50bf92e3a9 100644 --- a/web/src/utils/dashboard/colorPalette.spec.ts +++ b/web/src/utils/dashboard/colorPalette.spec.ts @@ -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")); diff --git a/web/src/utils/dashboard/convertPromQLData.ts b/web/src/utils/dashboard/convertPromQLData.ts index 147b4c8dd6..cb32826f2b 100644 --- a/web/src/utils/dashboard/convertPromQLData.ts +++ b/web/src/utils/dashboard/convertPromQLData.ts @@ -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( diff --git a/web/src/utils/dashboard/convertSQLData.spec.ts b/web/src/utils/dashboard/convertSQLData.spec.ts index a644005c61..b71ddbe60f 100644 --- a/web/src/utils/dashboard/convertSQLData.spec.ts +++ b/web/src/utils/dashboard/convertSQLData.spec.ts @@ -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 = [ diff --git a/web/src/utils/dashboard/convertSQLData.ts b/web/src/utils/dashboard/convertSQLData.ts index a756bb25f5..f009197738 100644 --- a/web/src/utils/dashboard/convertSQLData.ts +++ b/web/src/utils/dashboard/convertSQLData.ts @@ -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; itemStyle?: Record }, + 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, }; diff --git a/web/src/utils/dashboard/panelDiagnostics.ts b/web/src/utils/dashboard/panelDiagnostics.ts index 353fe802e3..a5c19080dc 100644 --- a/web/src/utils/dashboard/panelDiagnostics.ts +++ b/web/src/utils/dashboard/panelDiagnostics.ts @@ -57,7 +57,8 @@ export const toPartitions = (entry: unknown): Record[] => { return entry && typeof entry === "object" ? [entry as Record] : []; }; -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() !== ""); } diff --git a/web/src/utils/dashboard/promqlChartSeriesProps.ts b/web/src/utils/dashboard/promqlChartSeriesProps.ts index 9ebaae11f0..3b3e8946df 100644 --- a/web/src/utils/dashboard/promqlChartSeriesProps.ts +++ b/web/src/utils/dashboard/promqlChartSeriesProps.ts @@ -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 | undefined, + type: "dashed" | "dotted" = "dashed", +): Record => ({ ...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, diff --git a/web/src/utils/queryUtils.spec.ts b/web/src/utils/queryUtils.spec.ts index 15c7a35115..c1d94dfd63 100644 --- a/web/src/utils/queryUtils.spec.ts +++ b/web/src/utils/queryUtils.spec.ts @@ -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 = [ [ diff --git a/web/src/utils/queryUtils.ts b/web/src/utils/queryUtils.ts index 94cb0a4666..0541def6b1 100644 --- a/web/src/utils/queryUtils.ts +++ b/web/src/utils/queryUtils.ts @@ -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)); } } diff --git a/web/src/utils/timezone.spec.ts b/web/src/utils/timezone.spec.ts index 12772c211d..c6a7ea5895 100644 --- a/web/src/utils/timezone.spec.ts +++ b/web/src/utils/timezone.spec.ts @@ -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( diff --git a/web/src/utils/timezone.ts b/web/src/utils/timezone.ts index c7a3ab2a83..820266ca12 100644 --- a/web/src/utils/timezone.ts +++ b/web/src/utils/timezone.ts @@ -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,