diff --git a/package.json b/package.json index 3fcc38873..3769e8957 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@netdata/charts", - "version": "6.12.10", + "version": "6.12.11", "description": "Netdata frontend SDK and chart utilities", "main": "dist/index.js", "module": "dist/es6/index.js", diff --git a/src/chartLibraries/dygraph/index.js b/src/chartLibraries/dygraph/index.js index 47b32161a..ddde9223c 100644 --- a/src/chartLibraries/dygraph/index.js +++ b/src/chartLibraries/dygraph/index.js @@ -152,6 +152,15 @@ export default (sdk, chart) => { hoverX.toggle(attributes.enabledHover) navigation.toggle(attributes.enabledNavigation, attributes.navigation) + const onUnitsConversionChange = () => + updateOptions({ + ...makeChartTypeOptions(), + digitsAfterDecimal: + chart.getAttribute("unitsConversionFractionDigits")[0] < 0 + ? 0 + : chart.getAttribute("unitsConversionFractionDigits")[0], + }) + listeners = [ chartUI.on("resize", () => dygraph.resize()), chart.onAttributeChange("hoverX", dimensions => { @@ -183,15 +192,9 @@ export default (sdk, chart) => { updateOptions(makeChartTypeOptions()) }), - chart.onAttributeChange("unitsConversionPrefix", () => { - updateOptions({ - ...makeChartTypeOptions(), - digitsAfterDecimal: - chart.getAttribute("unitsConversionFractionDigits")[0] < 0 - ? 0 - : chart.getAttribute("unitsConversionFractionDigits")[0], - }) - }), + // scalable conversions move the prefix, conversable ones move the base + chart.onAttributeChange("unitsConversionPrefix", onUnitsConversionChange), + chart.onAttributeChange("unitsConversionBase", onUnitsConversionChange), chart.onAttributeChange("selectedLegendDimensions", () => { if (chart.getAttribute("processing")) return diff --git a/src/chartLibraries/dygraph/index.test.js b/src/chartLibraries/dygraph/index.test.js index 6ae0951f2..8747c2269 100644 --- a/src/chartLibraries/dygraph/index.test.js +++ b/src/chartLibraries/dygraph/index.test.js @@ -160,6 +160,34 @@ describe("dygraphChart", () => { expect(mockDygraph.updateOptions.mock.calls[1][1]).toBe(true) }) + it("redraws when a conversable conversion changes the base unit", () => { + const { sdk, chart } = makeTestChart({ + attributes: { + highlighting: false, + panning: false, + processing: false, + }, + }) + + chart.getPayload = () => ({ + data: [[1617946860000, 10]], + labels: ["time", "value"], + }) + chart.getDateWindow = () => [1617946860000, 1617947750000] + chart.formatXAxis = x => x.toString() + chart.getThemeAttribute = () => "#333" + chart.getUnitSign = () => "" + + const instance = dygraphChart(sdk, chart) + instance.mount(document.createElement("div")) + instance.render() + mockDygraph.updateOptions.mockClear() + + chart.updateAttribute("unitsConversionBase", ["[degF]"]) + + expect(mockDygraph.updateOptions).toHaveBeenCalled() + }) + it("skips render when highlighting, panning, or processing", () => { const { sdk, chart } = makeTestChart({ attributes: { diff --git a/src/components/filterToolbox/timeAggregation.js b/src/components/filterToolbox/timeAggregation.js index 753161f5a..17e4daca0 100644 --- a/src/components/filterToolbox/timeAggregation.js +++ b/src/components/filterToolbox/timeAggregation.js @@ -3,7 +3,7 @@ import { Flex, TextMicro } from "@netdata/netdata-ui" import { useAttributeValue, useChart } from "@/components/provider" import Dropdown from "./dropdownSingleSelect" -const useMenuItems = (chart, perTier = []) => { +export const useMenuItems = (chart, perTier = []) => { const [firstTier, ...restTiers] = perTier return useMemo( () => @@ -38,6 +38,19 @@ const useMenuItems = (chart, perTier = []) => { short: "SUM()", "data-track": chart.track("time-aggregation-sum"), }, + { + justDesc: true, + description: + "The function below does not combine values at all. Use it for metrics that represent a state rather than a quantity.", + }, + { + value: "latest", + label: "Latest value", + description: + "Show the last database value in each point instead of combining them. Use it for states, statuses and boolean metrics, where a combined value such as 0.5 is meaningless. Requires a recent Netdata agent; older agents fall back to average.", + short: "LATEST()", + "data-track": chart.track("time-aggregation-latest"), + }, Array.isArray(restTiers) && typeof firstTier?.points !== "undefined" && { justDesc: true, diff --git a/src/components/filterToolbox/timeAggregation.test.js b/src/components/filterToolbox/timeAggregation.test.js new file mode 100644 index 000000000..23b2a63fd --- /dev/null +++ b/src/components/filterToolbox/timeAggregation.test.js @@ -0,0 +1,40 @@ +import { renderHook } from "@testing-library/react" +import { useMenuItems } from "./timeAggregation" + +const chart = { track: name => name } + +describe("time aggregation menu items", () => { + it("offers latest with a LATEST() short label", () => { + const { result } = renderHook(() => useMenuItems(chart)) + const latest = result.current.find(item => item.value === "latest") + + expect(latest).toBeTruthy() + expect(latest.short).toBe("LATEST()") + expect(latest.label).toBe("Latest value") + }) + + it("precedes latest with a description-only separator", () => { + const { result } = renderHook(() => useMenuItems(chart)) + const index = result.current.findIndex(item => item.value === "latest") + + expect(index).toBeGreaterThan(0) + expect(result.current[index - 1].justDesc).toBe(true) + }) + + it("keeps the everyday functions ahead of latest", () => { + const { result } = renderHook(() => useMenuItems(chart)) + const valueAt = value => result.current.findIndex(item => item.value === value) + + expect(valueAt("min")).toBeLessThan(valueAt("latest")) + expect(valueAt("max")).toBeLessThan(valueAt("latest")) + expect(valueAt("average")).toBeLessThan(valueAt("latest")) + expect(valueAt("sum")).toBeLessThan(valueAt("latest")) + }) + + it("mentions the older-agent fallback in the description", () => { + const { result } = renderHook(() => useMenuItems(chart)) + const latest = result.current.find(item => item.value === "latest") + + expect(latest.description).toMatch(/fall back to average/i) + }) +}) diff --git a/src/components/provider/selectors.js b/src/components/provider/selectors.js index b0ff48d68..a9586be50 100644 --- a/src/components/provider/selectors.js +++ b/src/components/provider/selectors.js @@ -300,10 +300,14 @@ export const useUnits = (key = "units") => { const forceUpdate = useForceUpdate() - useImmediateListener( - () => chart.onAttributeChange(`${key}ConversionPrefix`, forceUpdate), - [chart, key] - ) + useImmediateListener(() => { + const offListeners = [ + chart.onAttributeChange(`${key}ConversionPrefix`, forceUpdate), + chart.onAttributeChange(`${key}ConversionBase`, forceUpdate), + ] + + return () => offListeners.forEach(off => off()) + }, [chart, key]) return chart.getUnits(key) } @@ -367,6 +371,7 @@ export const useValueUnitAttributes = ( const units = useAttributeValue(unitsKey) const desiredUnits = useAttributeValue("desiredUnits") const secondsAsTime = useAttributeValue("secondsAsTime") + const temperature = useAttributeValue("temperature") const viewDimensions = useAttributeValue("viewDimensions") return useMemo(() => { @@ -387,6 +392,7 @@ export const useValueUnitAttributes = ( units, desiredUnits, secondsAsTime, + temperature, viewDimensions, ]) } @@ -620,7 +626,8 @@ export const useValue = ( return unregister( chart.onAttributeChange("hoverX", () => setState(getValue())), chart.on("dimensionChanged", () => setState(getValue())), - chart.onAttributeChange(`${unitsKey}Conversion`, () => setState(getValue())), + chart.onAttributeChange(`${unitsKey}ConversionPrefix`, () => setState(getValue())), + chart.onAttributeChange(`${unitsKey}ConversionBase`, () => setState(getValue())), chart.on("successFetch", () => setState(getValue())) ) }, [chart, id, valueKey, period, unitsKey, abs, allowNull]) diff --git a/src/components/provider/selectors.test.js b/src/components/provider/selectors.test.js index 1ec3687e4..456e4eb26 100644 --- a/src/components/provider/selectors.test.js +++ b/src/components/provider/selectors.test.js @@ -17,6 +17,7 @@ import { useLatestDisplayValue, useLatestDisplayValueWithUnit, useLatestValue, + useValueWithUnit, getValueByPeriod, } from "./selectors" @@ -417,4 +418,22 @@ describe("Chart Provider Selectors", () => { expect(result.current.formatted.convertedUnit).toBe("KiB") }) }) + + describe("useValueWithUnit", () => { + it("reconverts when the temperature preference changes", () => { + const { result, chart } = renderHookWithChart( + () => useValueWithUnit(100, { scaleByValue: true }), + { attributes: { units: ["Cel"], desiredUnits: ["auto"] } } + ) + + expect(result.current.convertedUnit).toBe("°C") + + act(() => { + chart.updateAttribute("temperature", "fahrenheit") + }) + + expect(result.current.convertedUnit).toBe("°F") + expect(result.current.convertedValue).toBe("212") + }) + }) }) diff --git a/src/components/toolbox/settings/numberFormat.js b/src/components/toolbox/settings/numberFormat.js index c7566f313..3f0d055c2 100644 --- a/src/components/toolbox/settings/numberFormat.js +++ b/src/components/toolbox/settings/numberFormat.js @@ -2,13 +2,28 @@ import React, { useMemo, useState, useEffect } from "react" import { Flex, TextSmall, TextInput, Select } from "@netdata/netdata-ui" import { useAttributeValue, useChart } from "@/components/provider" import { getScales, getUnitConfig, isScalable } from "@/helpers/units" -import conversableUnits, { keys as conversableKeys } from "@/helpers/units/conversableUnits" +import conversableUnits, { + keys as conversableKeys, + isTemperatureUnit, +} from "@/helpers/units/conversableUnits" + +const symbolOf = unit => getUnitConfig(unit).print_symbol || unit + +// what "auto" resolves to right now, asked of the converter itself rather than restated here +const resolvePreferred = (chart, unit) => { + const target = (conversableKeys[unit] || []).find(key => + conversableUnits[unit][key].check(chart, 0) + ) + + return symbolOf(target || unit) +} const NumberFormat = () => { const chart = useChart() const units = useAttributeValue("units") const desiredUnitsAttr = useAttributeValue("desiredUnits") || ["auto"] const staticFractionDigitsAttr = useAttributeValue("staticFractionDigits") + const temperature = useAttributeValue("temperature") const [selectedUnitIndex, setSelectedUnitIndex] = useState(0) @@ -25,17 +40,28 @@ const NumberFormat = () => { ) const scaleOptions = useMemo(() => { + if (isTemperatureUnit(selectedUnit)) { + return [ + { value: "auto", label: `Follow preference (${resolvePreferred(chart, selectedUnit)})` }, + { value: "original", label: symbolOf(selectedUnit) }, + ...(conversableKeys[selectedUnit] || []).map(key => ({ + value: key, + label: symbolOf(key), + })), + ] + } + const options = [ { value: "auto", label: "Auto scale" }, { value: "original", label: "No conversion" }, ] - if (isScalable(selectedUnit)) { + if (isScalable(selectedUnit) || conversableUnits[selectedUnit]) { if (conversableUnits[selectedUnit]) { const scaleKeys = conversableKeys[selectedUnit] || Object.keys(conversableUnits[selectedUnit]) scaleKeys.forEach(key => { - options.push({ value: key, label: key }) + options.push({ value: key, label: symbolOf(key) }) }) } else { const [scaleKeys] = getScales(selectedUnit) @@ -50,7 +76,7 @@ const NumberFormat = () => { } return options - }, [selectedUnit]) + }, [chart, selectedUnit, temperature]) useEffect(() => { setDesiredUnits(desiredUnitsAttr[selectedUnitIndex] || "auto") @@ -77,6 +103,7 @@ const NumberFormat = () => { update({ staticFractionDigits: value }) } + return ( diff --git a/src/components/toolbox/settings/numberFormat.test.js b/src/components/toolbox/settings/numberFormat.test.js index fa1a01f24..6a5fe1a75 100644 --- a/src/components/toolbox/settings/numberFormat.test.js +++ b/src/components/toolbox/settings/numberFormat.test.js @@ -15,3 +15,30 @@ it("labels scale options with the base unit, not the incoming prefixed unit", as expect(screen.getByText("Mi By/s")).toBeInTheDocument() expect(screen.queryByText("Ki KiBy/s")).not.toBeInTheDocument() }) + +// temperature units render a Temperature select ahead of Scale +const openScale = async user => { + const selects = screen.getAllByRole("combobox") + await user.click(selects[selects.length - 1]) +} + +it("offers Fahrenheit for a Celsius chart, labelled by its symbol", async () => { + const { user } = renderWithChart(, { + attributes: { units: ["Cel"], desiredUnits: ["auto"] }, + }) + + await openScale(user) + + expect(screen.getByText("°F")).toBeInTheDocument() + expect(screen.queryByText("[degF]")).not.toBeInTheDocument() +}) + +it("offers Celsius for a Fahrenheit chart", async () => { + const { user } = renderWithChart(, { + attributes: { units: ["[degF]"], desiredUnits: ["auto"] }, + }) + + await openScale(user) + + expect(screen.getByText("°C")).toBeInTheDocument() +}) diff --git a/src/components/toolbox/settings/tabs/timeAggregation.js b/src/components/toolbox/settings/tabs/timeAggregation.js index bb834bd2e..059c1cfbc 100644 --- a/src/components/toolbox/settings/tabs/timeAggregation.js +++ b/src/components/toolbox/settings/tabs/timeAggregation.js @@ -1,129 +1,7 @@ import React, { memo, useMemo } from "react" import { Flex, TextSmall, Select } from "@netdata/netdata-ui" import { useAttributeValue, useChart } from "@/components/provider" - -const useMenuItems = (chart, perTier = []) => { - const [firstTier, ...restTiers] = perTier - return useMemo( - () => - [ - { - value: "min", - label: "Minimum", - description: "Reveal short dives that would otherwise be smoothed out.", - short: "MIN()", - "data-track": chart.track("time-aggregation-min"), - }, - { - value: "max", - label: "Maximum", - description: "Reveal short spikes that would otherwise be smoothed out.", - short: "MAX()", - "data-track": chart.track("time-aggregation-max"), - }, - { - value: "average", - label: "Mean or Average", - description: - "Calculate the longer term average, as if data were collected at screen resolution.", - short: "AVG()", - "data-track": chart.track("time-aggregation-average"), - }, - { - value: "sum", - label: "Sum", - description: - "Provide the sum of the points that are aggregated over time. Use it when a sense of volume is needed over the aggregation period. It may not be sensible to use this function on all data types.", - short: "SUM()", - "data-track": chart.track("time-aggregation-sum"), - }, - Array.isArray(restTiers) && - typeof firstTier?.points !== "undefined" && { - justDesc: true, - description: `The functions below lose accuracy when applied on tiered data, compared to high resolution data. Your current query is ${ - (firstTier.points * 100.0) / perTier.reduce((h, t) => h + t.points, 0) - }% high resolution and ${ - (restTiers.reduce((h, t) => h + t.points, 0) * 100.0) / - perTier.reduce((h, t) => h + t.points, 0).toFixed(2) - }% tiered data of lower resolution.`, - }, - { - value: "percentile", - label: "Percentile", - description: - "Provide the maximum value of a percentage of the aggregated points, having the smaller values. The default is p95, which provides the maximum value of the aggregated points after ignoring the top 5% of them.", - short: "PERCENTILE()", - "data-track": chart.track("time-aggregation-percentile95"), - }, - { - value: "trimmed-mean", - label: "Trimmed Average or Trimmed Mean", - description: - "Like average, but first remove a percentage of the extreme high and low values.", - short: "TRIMMEAN()", - "data-track": chart.track("time-aggregation-trimmed-mean5"), - }, - { - value: "median", - label: "Median", - description: - "The middle value of all points that would otherwise be smoothed out. This function works like average, but short extreme dives and spikes influence it significantly less than average.", - short: "MEDIAN()", - "data-track": chart.track("time-aggregation-median"), - }, - { - value: "trimmed-median", - label: "Trimmed Median", - description: - "Like median, but first remove a percentage of the extreme high and low values.", - short: "TRIMMEDIAN()", - "data-track": chart.track("time-aggregation-trimmed-median5"), - }, - { - value: "stddev", - label: "Standard deviation", - description: - "Reveal how far each point lies from the average. A high standard deviation means that values are generally far from the average, while a low standard deviation indicates that values are clustered close to the mean. The result is again in the original units of the data source metric.", - short: "STDDEV()", - "data-track": chart.track("time-aggregation-stddev"), - }, - { - value: "cv", - label: "Coefficient of variation or Relative standard deviation", - description: - "The ratio of the standard deviation to the average. Its use is the same as standard deviation, but expressed as a percentage related to the average. The units change to %.", - short: "CV()", - "data-track": chart.track("time-aggregation-cv"), - }, - { - value: "incremental-sum", - label: "Incremental Sum or Delta", - description: - "Provide the difference between the newest and the oldest values of the aggregated points. Each point will be positive if the trend grows and negative if the trend shrinks.", - short: "DELTA()", - "data-track": chart.track("time-aggregation-incremental-sum"), - }, - - { - value: "ses", - label: "Single exponential smoothing", - description: - "Use the aggregated points to produce a forecast of the next value, and reveal the forecasted value. Use it when there are indications that the trend is more predictable using the more recent points than the older ones.", - short: "SES()", - "data-track": chart.track("time-aggregation-ses"), - }, - { - value: "des", - label: "Double exponential smoothing", - description: - "Like single exponential smoothing, but better suited when the aggregated points may have a strong trend.", - short: "DES()", - "data-track": chart.track("time-aggregation-des"), - }, - ].filter(Boolean), - [chart, firstTier?.points] - ) -} +import { useMenuItems } from "@/components/filterToolbox/timeAggregation" const useMenuAliasItems = ({ chart, method }) => useMemo(() => { diff --git a/src/components/toolbox/settings/tabs/timeAggregation.test.js b/src/components/toolbox/settings/tabs/timeAggregation.test.js new file mode 100644 index 000000000..0bcc331f9 --- /dev/null +++ b/src/components/toolbox/settings/tabs/timeAggregation.test.js @@ -0,0 +1,15 @@ +import React from "react" +import "@testing-library/jest-dom" +import { screen } from "@testing-library/react" +import { renderWithChart } from "@jest/testUtilities" +import TimeAggregation from "./timeAggregation" + +it("offers the latest value option, matching the filter toolbox", async () => { + const { user } = renderWithChart(, { + attributes: { groupingMethod: "average" }, + }) + + await user.click(screen.getByRole("combobox")) + + expect(screen.getByText("Latest value")).toBeInTheDocument() +}) diff --git a/src/components/toolbox/settings/unitPreference.test.js b/src/components/toolbox/settings/unitPreference.test.js new file mode 100644 index 000000000..5c6924ca1 --- /dev/null +++ b/src/components/toolbox/settings/unitPreference.test.js @@ -0,0 +1,77 @@ +import React from "react" +import "@testing-library/jest-dom" +import { screen } from "@testing-library/react" +import { renderWithChart } from "@jest/testUtilities" +import NumberFormat from "./numberFormat" + +describe("temperature units in chart settings", () => { + it("uses a single control, not a separate preference select", () => { + renderWithChart(, { + attributes: { units: ["Cel"], desiredUnits: ["auto"], temperature: "fahrenheit" }, + }) + + expect(screen.getAllByRole("combobox")).toHaveLength(1) + }) + + it("shows what the inherited preference resolves to", () => { + renderWithChart(, { + attributes: { units: ["Cel"], desiredUnits: ["auto"], temperature: "fahrenheit" }, + }) + + expect(screen.getByText("Follow preference (°F)")).toBeInTheDocument() + }) + + it("resolves to the source unit when the preference does not convert", () => { + renderWithChart(, { + attributes: { units: ["Cel"], desiredUnits: ["auto"] }, + }) + + expect(screen.getByText("Follow preference (°C)")).toBeInTheDocument() + }) + + it("offers both temperature units by symbol", async () => { + const { user } = renderWithChart(, { + attributes: { units: ["Cel"], desiredUnits: ["auto"] }, + }) + + await user.click(screen.getByRole("combobox")) + + expect(screen.getByText("°C")).toBeInTheDocument() + expect(screen.getByText("°F")).toBeInTheDocument() + expect(screen.queryByText("No conversion")).not.toBeInTheDocument() + }) + + it("pins this chart to Fahrenheit", async () => { + const { user, chart } = renderWithChart(, { + attributes: { units: ["Cel"], desiredUnits: ["auto"] }, + }) + + await user.click(screen.getByRole("combobox")) + await user.click(screen.getByText("°F")) + + expect(chart.getAttribute("desiredUnits")).toEqual(["[degF]"]) + }) + + it("pins this chart to its source unit", async () => { + const { user, chart } = renderWithChart(, { + attributes: { units: ["Cel"], desiredUnits: ["auto"], temperature: "fahrenheit" }, + }) + + await user.click(screen.getByRole("combobox")) + await user.click(screen.getByText("°C")) + + expect(chart.getAttribute("desiredUnits")).toEqual(["original"]) + }) + + it("keeps the generic labels for non-temperature units", async () => { + const { user } = renderWithChart(, { + attributes: { units: ["By"], desiredUnits: ["auto"] }, + }) + + await user.click(screen.getByRole("combobox")) + + expect(screen.getAllByText("Auto scale").length).toBeGreaterThan(0) + expect(screen.getByText("No conversion")).toBeInTheDocument() + expect(screen.queryByText(/Follow preference/)).not.toBeInTheDocument() + }) +}) diff --git a/src/helpers/stepped.js b/src/helpers/stepped.js deleted file mode 100644 index d4d70b61c..000000000 --- a/src/helpers/stepped.js +++ /dev/null @@ -1,6 +0,0 @@ -export const stateUnits = new Set(["state", "{state}", "status", "{status}"]) - -export const isStateUnits = units => { - const list = (Array.isArray(units) ? units : [units]).filter(Boolean) - return list.length > 0 && list.every(unit => stateUnits.has(unit)) -} diff --git a/src/helpers/stepped.test.js b/src/helpers/stepped.test.js deleted file mode 100644 index 8ca5f2ef3..000000000 --- a/src/helpers/stepped.test.js +++ /dev/null @@ -1,32 +0,0 @@ -import { isStateUnits } from "./stepped" - -describe("isStateUnits", () => { - it("returns true for state units", () => { - expect(isStateUnits(["{state}"])).toBe(true) - expect(isStateUnits(["state"])).toBe(true) - }) - - it("returns true for status units", () => { - expect(isStateUnits(["{status}"])).toBe(true) - expect(isStateUnits(["status"])).toBe(true) - }) - - it("accepts a plain string", () => { - expect(isStateUnits("state")).toBe(true) - }) - - it("returns false for non-state units", () => { - expect(isStateUnits(["bytes/s"])).toBe(false) - expect(isStateUnits(["%"])).toBe(false) - }) - - it("returns false when mixed with non-state units", () => { - expect(isStateUnits(["state", "bytes/s"])).toBe(false) - }) - - it("returns false for empty or missing units", () => { - expect(isStateUnits([])).toBe(false) - expect(isStateUnits([""])).toBe(false) - expect(isStateUnits(undefined)).toBe(false) - }) -}) diff --git a/src/helpers/unitConversion/getConversionUnits.js b/src/helpers/unitConversion/getConversionUnits.js index f0f0a4818..50c0b8372 100644 --- a/src/helpers/unitConversion/getConversionUnits.js +++ b/src/helpers/unitConversion/getConversionUnits.js @@ -3,7 +3,13 @@ import conversableUnits, { keys as conversableKeys, } from "@/helpers/units/conversableUnits" import { shouldUseExponential } from "@/helpers/formatNumber" -import convert, { getScales, getUnitConfig, isScalable, getExponent } from "@/helpers/units" +import convert, { + getScales, + getUnitConfig, + isScalable, + getExponent, + unitLabelModes, +} from "@/helpers/units" const selfOrExponent = (u, scaleByKey) => { const exponent = getExponent(u) @@ -100,7 +106,7 @@ const conversable = (chart, units, min, max, desiredUnits, maxDecimals) => { if (desiredUnits && desiredUnits !== "auto" && desiredUnits !== "original") { return desiredUnits in scales - ? [makeConversableKey(units, desiredUnits), undefined, desiredUnits] + ? [makeConversableKey(units, desiredUnits), undefined, "", desiredUnits, unitLabelModes.full] : ["original"] } @@ -117,13 +123,14 @@ const conversable = (chart, units, min, max, desiredUnits, maxDecimals) => { const candidates = scaleKeys .slice(0, scaleIndex + 1) .reverse() - .map(key => [makeConversableKey(units, key), null, "", key]) + .map(key => [makeConversableKey(units, key), null, "", key, unitLabelModes.full]) return choosePrecisionCandidate(chart, candidates, min, max, maxDecimals) } const getMethod = (chart, units, min, max, maxDecimals) => { - if (!isScalable(units)) return ["original"] + // conversable units may be non-scalable (Cel, [degF]) yet still convertible + if (!isScalable(units) && !conversableUnits[units]) return ["original"] const allUnits = chart.getAttribute("units") const desiredUnitsArray = chart.getAttribute("desiredUnits") || ["auto"] @@ -140,7 +147,7 @@ const getMethod = (chart, units, min, max, maxDecimals) => { } const makeConversionAttributes = (chart, unit, candidate, min, max, maxDecimals) => { - const [method, divider, prefix = "", base = ""] = candidate + const [method, divider, prefix = "", base = "", labelMode] = candidate const cMin = convert(chart, method, min, divider) const cMax = convert(chart, method, max, divider) const delta = Math.abs(cMin === cMax ? cMin : cMax - cMin) @@ -152,6 +159,7 @@ const makeConversionAttributes = (chart, unit, candidate, min, max, maxDecimals) fractionDigits: fractionDigits > maxDecimals ? maxDecimals : fractionDigits, prefix, base, + labelMode, unit, } } @@ -198,7 +206,7 @@ const getConversionUnits = (chart, unitsKey, options = {}) => { return units.reduce( (h, unit) => { - const { method, divider, fractionDigits, prefix, base } = getConversionAttributes( + const { method, divider, fractionDigits, prefix, base, labelMode } = getConversionAttributes( chart, unit, options @@ -209,10 +217,11 @@ const getConversionUnits = (chart, unitsKey, options = {}) => { h.fractionDigits.push(fractionDigits) h.prefix.push(prefix) h.base.push(base) + h.labelMode.push(labelMode) return h }, - { method: [], fractionDigits: [], prefix: [], base: [], divider: [] } + { method: [], fractionDigits: [], prefix: [], base: [], divider: [], labelMode: [] } ) } diff --git a/src/helpers/unitConversion/index.js b/src/helpers/unitConversion/index.js index 672d8c834..5eacb3320 100644 --- a/src/helpers/unitConversion/index.js +++ b/src/helpers/unitConversion/index.js @@ -35,6 +35,7 @@ const baseConvert = (chart, unitsKey = "units", min, max) => { prefix = "", base = "", divider, + labelMode, } = getConversionUnits(chart, unitsKey, { min, max }) const unitsStsByContext = chart.getAttribute(`${unitsKey}StsByContext`) @@ -55,6 +56,7 @@ const baseConvert = (chart, unitsKey = "units", min, max) => { [`${unitsKey}ConversionMethod`]: method, [`${unitsKey}ConversionPrefix`]: prefix, [`${unitsKey}ConversionBase`]: base, + [`${unitsKey}ConversionLabelMode`]: labelMode, [`${unitsKey}ConversionFractionDigits`]: fractionDigits, [`${unitsKey}ConversionDivider`]: divider, }) @@ -106,9 +108,14 @@ export default chart => { const offVisibleDimensionsChanged = chart.on("visibleDimensionsChanged", () => onConvert()) const offYAxisChange = chart.on("yAxisChange", onConvert) + // unit preferences feed the conversable checks, so they must re-run conversion themselves + const offTemperature = chart.onAttributeChange("temperature", () => onConvert()) + const offSecondsAsTime = chart.onAttributeChange("secondsAsTime", () => onConvert()) return () => { offYAxisChange() offVisibleDimensionsChanged() + offTemperature() + offSecondsAsTime() } } diff --git a/src/helpers/unitConversion/temperature.test.js b/src/helpers/unitConversion/temperature.test.js new file mode 100644 index 000000000..6afc77ee9 --- /dev/null +++ b/src/helpers/unitConversion/temperature.test.js @@ -0,0 +1,205 @@ +import unitConversion from "./index" +import { getConversionAttributes } from "./getConversionUnits" +import convert from "@/helpers/units" +import { makeTestChart } from "@jest/testUtilities" + +const makeChart = attributes => { + const { chart } = makeTestChart({ + attributes: { + unitsStsByContext: {}, + dbUnitsStsByContext: {}, + ...attributes, + }, + }) + + return chart +} + +const read = (chart, units, { min, max }) => { + const unitAttributes = getConversionAttributes(chart, units, { min, max }) + + return { + method: unitAttributes.method, + label: chart.getUnitSign({ unitAttributes }), + convert: value => convert(chart, unitAttributes.method, value, unitAttributes.divider), + } +} + +describe("temperature preference", () => { + describe("unset (default) renders whatever the agent reported", () => { + it("leaves Celsius sources in Celsius", () => { + const chart = makeChart({ units: ["Cel"], desiredUnits: ["auto"] }) + + const { method, label, convert: c } = read(chart, "Cel", { min: 0, max: 100 }) + + expect(method).toBe("original") + expect(label).toBe("°C") + expect(c(100)).toBe(100) + }) + + it("leaves Fahrenheit sources in Fahrenheit", () => { + const chart = makeChart({ units: ["[degF]"], desiredUnits: ["auto"] }) + + const { method, label, convert: c } = read(chart, "[degF]", { min: 32, max: 212 }) + + expect(method).toBe("original") + expect(label).toBe("°F") + expect(c(212)).toBe(212) + }) + }) + + describe("fahrenheit", () => { + it("converts Celsius sources", () => { + const chart = makeChart({ + units: ["Cel"], + desiredUnits: ["auto"], + temperature: "fahrenheit", + }) + + const { method, label, convert: c } = read(chart, "Cel", { min: 0, max: 100 }) + + expect(method).toBe("Cel-[degF]") + expect(label).toBe("°F") + expect(c(0)).toBe(32) + expect(c(100)).toBe(212) + }) + + it("leaves Fahrenheit sources alone", () => { + const chart = makeChart({ + units: ["[degF]"], + desiredUnits: ["auto"], + temperature: "fahrenheit", + }) + + const { method, label, convert: c } = read(chart, "[degF]", { min: 32, max: 212 }) + + expect(method).toBe("original") + expect(label).toBe("°F") + expect(c(212)).toBe(212) + }) + }) + + describe("celsius", () => { + it("converts Fahrenheit sources", () => { + const chart = makeChart({ + units: ["[degF]"], + desiredUnits: ["auto"], + temperature: "celsius", + }) + + const { method, label, convert: c } = read(chart, "[degF]", { min: 32, max: 212 }) + + expect(method).toBe("[degF]-Cel") + expect(label).toBe("°C") + expect(c(32)).toBe(0) + expect(c(212)).toBe(100) + }) + + it("leaves Celsius sources alone", () => { + const chart = makeChart({ + units: ["Cel"], + desiredUnits: ["auto"], + temperature: "celsius", + }) + + const { method, label, convert: c } = read(chart, "Cel", { min: 0, max: 100 }) + + expect(method).toBe("original") + expect(label).toBe("°C") + expect(c(100)).toBe(100) + }) + }) + + describe("per-chart selection overrides the preference", () => { + it("converts to Fahrenheit while the preference says celsius", () => { + const chart = makeChart({ + units: ["Cel"], + desiredUnits: ["[degF]"], + temperature: "celsius", + }) + + const { method, label, convert: c } = read(chart, "Cel", { min: 0, max: 100 }) + + expect(method).toBe("Cel-[degF]") + expect(label).toBe("°F") + expect(c(100)).toBe(212) + }) + + it("keeps source units when the chart asks for no conversion", () => { + const chart = makeChart({ + units: ["Cel"], + desiredUnits: ["original"], + temperature: "fahrenheit", + }) + + const { method, label, convert: c } = read(chart, "Cel", { min: 0, max: 100 }) + + expect(method).toBe("original") + expect(label).toBe("°C") + expect(c(100)).toBe(100) + }) + + it("honours an explicit choice regardless of the data magnitude", () => { + const chart = makeChart({ units: ["h"], desiredUnits: ["ns"] }) + + expect(read(chart, "h", { min: 0, max: 1000 }).method).toBe("h-ns") + expect(read(chart, "h", { min: 0, max: 1e9 }).method).toBe("h-ns") + }) + }) + + describe("changing the preference recomputes conversion", () => { + const makeStaticRangeChart = () => + makeChart({ + units: ["Cel"], + dbUnits: ["Cel"], + desiredUnits: ["auto"], + staticValueRange: [0, 100], + dimensionIds: ["temp"], + visibleDimensionIds: ["temp"], + }) + + it("switches to Fahrenheit when the preference is set", () => { + const chart = makeStaticRangeChart() + unitConversion(chart) + chart.trigger("yAxisChange") + + expect(chart.getAttribute("unitsConversionMethod")).toEqual(["original"]) + + chart.updateAttribute("temperature", "fahrenheit") + + expect(chart.getAttribute("unitsConversionMethod")).toEqual(["Cel-[degF]"]) + }) + + it("switches back when the preference is cleared", () => { + const chart = makeStaticRangeChart() + unitConversion(chart) + chart.updateAttribute("temperature", "fahrenheit") + + expect(chart.getAttribute("unitsConversionMethod")).toEqual(["Cel-[degF]"]) + + chart.updateAttribute("temperature", undefined) + + expect(chart.getAttribute("unitsConversionMethod")).toEqual(["original"]) + }) + + it("recomputes when secondsAsTime changes", () => { + const chart = makeChart({ + units: ["s"], + dbUnits: ["s"], + desiredUnits: ["auto"], + staticValueRange: [0, 3700], + secondsAsTime: true, + dimensionIds: ["uptime"], + visibleDimensionIds: ["uptime"], + }) + unitConversion(chart) + chart.trigger("yAxisChange") + + expect(chart.getAttribute("unitsConversionMethod")).toEqual(["s-h:mm:ss"]) + + chart.updateAttribute("secondsAsTime", false) + + expect(chart.getAttribute("unitsConversionMethod")).toEqual(["original"]) + }) + }) +}) diff --git a/src/helpers/unitConversion/unitLabels.test.js b/src/helpers/unitConversion/unitLabels.test.js new file mode 100644 index 000000000..9b45e6757 --- /dev/null +++ b/src/helpers/unitConversion/unitLabels.test.js @@ -0,0 +1,131 @@ +import { getConversionAttributes } from "./getConversionUnits" +import { makeTestChart } from "@jest/testUtilities" + +// Every unit/scale pair a user can pick from chart Settings -> Display -> Value formatting -> Scale. +// Expected labels are hardcoded on purpose: they must not be derived from the same tables the +// implementation reads, or the assertions would be tautological. +// Compact duration targets expect "" because the formatted value already carries its units ("1h1m40s"). +const C = "°C" +const F = "°F" +const NS = "ns" +const US = "µs" +const MS = "ms" +const S = "s" +const NONE = "" + +const cases = [ + ["Cel", "[degF]", F], + ["[degF]", "Cel", C], + + ["ns", "ns", NS], + ["ns", "us", US], + ["ns", "ms", MS], + ["ns", "s", S], + + ["ms", "ns", NS], + ["ms", "us", US], + ["ms", "ms", MS], + ["ms", "s", S], + ["ms", "a:mo:d", NONE], + ["ms", "mo:d:h", NONE], + ["ms", "d:h:mm", NONE], + ["ms", "h:mm:ss", NONE], + ["ms", "mm:ss", NONE], + + ["s", "ns", NS], + ["s", "us", US], + ["s", "ms", MS], + ["s", "s", S], + ["s", "a:mo:d", NONE], + ["s", "mo:d:h", NONE], + ["s", "d:h:mm", NONE], + ["s", "h:mm:ss", NONE], + ["s", "mm:ss", NONE], + ["s", "dHH:MM:ss", NONE], + + ["min", "ns", NS], + ["min", "us", US], + ["min", "ms", MS], + ["min", "s", S], + ["min", "a:mo:d", NONE], + ["min", "mo:d:h", NONE], + ["min", "d:h:mm", NONE], + ["min", "h:mm:ss", NONE], + ["min", "mm:ss", NONE], + ["min", "dHH:MM:ss", NONE], + + ["h", "ns", NS], + ["h", "us", US], + ["h", "ms", MS], + ["h", "s", S], + ["h", "a:mo:d", NONE], + ["h", "mo:d:h", NONE], + ["h", "d:h:mm", NONE], + ["h", "h:mm:ss", NONE], + ["h", "mm:ss", NONE], + ["h", "dHH:MM:ss", NONE], + + ["d", "ns", NS], + ["d", "us", US], + ["d", "ms", MS], + ["d", "s", S], + ["d", "a:mo:d", NONE], + ["d", "mo:d:h", NONE], + ["d", "d:h:mm", NONE], + ["d", "h:mm:ss", NONE], + ["d", "mm:ss", NONE], + ["d", "dHH:MM:ss", NONE], + + ["wk", "ns", NS], + ["wk", "us", US], + ["wk", "ms", MS], + ["wk", "s", S], + ["wk", "a:mo:d", NONE], + ["wk", "mo:d:h", NONE], + ["wk", "d:h:mm", NONE], + ["wk", "h:mm:ss", NONE], + ["wk", "mm:ss", NONE], + ["wk", "dHH:MM:ss", NONE], + + ["mo", "ns", NS], + ["mo", "us", US], + ["mo", "ms", MS], + ["mo", "s", S], + ["mo", "a:mo:d", NONE], + ["mo", "mo:d:h", NONE], + ["mo", "d:h:mm", NONE], + ["mo", "h:mm:ss", NONE], + ["mo", "mm:ss", NONE], + ["mo", "dHH:MM:ss", NONE], + + ["a", "ns", NS], + ["a", "us", US], + ["a", "ms", MS], + ["a", "s", S], + ["a", "a:mo:d", NONE], + ["a", "mo:d:h", NONE], + ["a", "d:h:mm", NONE], + ["a", "h:mm:ss", NONE], + ["a", "mm:ss", NONE], + ["a", "dHH:MM:ss", NONE], +] + +const getLabel = (sourceUnits, desiredUnits) => { + const { chart } = makeTestChart({ + attributes: { units: [sourceUnits], desiredUnits: [desiredUnits] }, + }) + + const unitAttributes = getConversionAttributes(chart, sourceUnits, { min: 0, max: 1000 }) + + return chart.getUnitSign({ unitAttributes }) +} + +describe("explicit unit selection renders the target unit's label", () => { + it("covers every selectable pair", () => { + expect(cases).toHaveLength(85) + }) + + it.each(cases)("%s -> %s shows %p", (sourceUnits, desiredUnits, expected) => { + expect(getLabel(sourceUnits, desiredUnits)).toBe(expected) + }) +}) diff --git a/src/helpers/units/conversableUnits.js b/src/helpers/units/conversableUnits.js index af3a55483..ea74698b6 100644 --- a/src/helpers/units/conversableUnits.js +++ b/src/helpers/units/conversableUnits.js @@ -110,8 +110,13 @@ const makeSecondSourceConverters = multiplier => const millisecondKeys = ["ns", "us", "ms", "s", "a:mo:d", "mo:d:h", "d:h:mm", "h:mm:ss", "mm:ss"] const secondKeys = [...millisecondKeys, "dHH:MM:ss"] +export const temperatureUnits = new Set(["Cel", "[degF]"]) + +export const isTemperatureUnit = unit => temperatureUnits.has(unit) + export const keys = { Cel: ["[degF]"], + "[degF]": ["Cel"], ns: ["ns", "us", "ms", "s"], ms: millisecondKeys, s: secondKeys, @@ -130,6 +135,12 @@ export default { convert: value => (value * 9) / 5 + 32, }, }, + "[degF]": { + Cel: { + check: chart => chart.getAttribute("temperature") === "celsius", + convert: value => ((value - 32) * 5) / 9, + }, + }, ns: { ns: { check: (chart, max) => chart.getAttribute("secondsAsTime") && max < 1_000, diff --git a/src/helpers/units/index.js b/src/helpers/units/index.js index c7f527a11..c79f3c33d 100644 --- a/src/helpers/units/index.js +++ b/src/helpers/units/index.js @@ -47,6 +47,25 @@ const findCurly = u => { export const getAlias = u => allUnits.aliases[u] || (allUnits.units[u] ? u : findCurly(u)) +export const stateUnits = new Set(["{state}", "{status}", "{boolean}"]) + +const isStateUnit = unit => { + if (typeof unit !== "string") return false + + const trimmed = unit.trim() + if (!trimmed) return false + + return ( + stateUnits.has(getAlias(trimmed)) || stateUnits.has(getAlias(trimmed.toLowerCase())) + ) +} + +export const isStateUnits = units => { + const list = (Array.isArray(units) ? units : [units]).filter(Boolean) + + return list.length > 0 && list.every(isStateUnit) +} + export const getNormalizedUnit = u => { const alias = getAlias(u) const config = getUnitConfig(alias) diff --git a/src/helpers/units/index.test.js b/src/helpers/units/index.test.js index eb8d30485..451f81ed9 100644 --- a/src/helpers/units/index.test.js +++ b/src/helpers/units/index.test.js @@ -13,6 +13,8 @@ import unitConverter, { getUnitsString, isRateUnit, stripRateUnit, + isStateUnits, + stateUnits, } from "." import scalableUnits from "./scalableUnits" @@ -660,4 +662,57 @@ describe("units helpers", () => { expect(stripRateUnit(undefined)).toBe(undefined) }) }) + describe("state units", () => { + it("detects the canonical state unit names", () => { + expect(isStateUnits("state")).toBe(true) + expect(isStateUnits("status")).toBe(true) + expect(isStateUnits("boolean")).toBe(true) + }) + + it("detects the curly forms", () => { + expect(isStateUnits("{state}")).toBe(true) + expect(isStateUnits("{status}")).toBe(true) + expect(isStateUnits("{boolean}")).toBe(true) + }) + + it("resolves collector-specific spellings through the alias table", () => { + expect(isStateUnits("app pool status")).toBe(true) + expect(isStateUnits("exit status")).toBe(true) + }) + + it("is case-insensitive and tolerates padding", () => { + expect(isStateUnits("Status")).toBe(true) + expect(isStateUnits(" STATE ")).toBe(true) + }) + + it("rejects counter, rate, duration and percentage units", () => { + expect(isStateUnits("containers")).toBe(false) + expect(isStateUnits("queries/s")).toBe(false) + expect(isStateUnits("sessions")).toBe(false) + expect(isStateUnits("connections")).toBe(false) + expect(isStateUnits("seconds")).toBe(false) + expect(isStateUnits("%")).toBe(false) + expect(isStateUnits("bytes/s")).toBe(false) + }) + + it("requires every unit in an array to be a state unit", () => { + expect(isStateUnits(["state"])).toBe(true) + expect(isStateUnits(["status", "boolean"])).toBe(true) + expect(isStateUnits(["state", "bytes/s"])).toBe(false) + }) + + it("rejects empty, missing and non-string units", () => { + expect(isStateUnits([])).toBe(false) + expect(isStateUnits([""])).toBe(false) + expect(isStateUnits(undefined)).toBe(false) + expect(isStateUnits(null)).toBe(false) + expect(isStateUnits(42)).toBe(false) + }) + + it("exposes the canonical alias set", () => { + expect(stateUnits.has("{state}")).toBe(true) + expect(stateUnits.has("{status}")).toBe(true) + expect(stateUnits.has("{boolean}")).toBe(true) + }) + }) }) diff --git a/src/sdk/initialAttributes.js b/src/sdk/initialAttributes.js index f15c46450..e65e1038d 100644 --- a/src/sdk/initialAttributes.js +++ b/src/sdk/initialAttributes.js @@ -100,14 +100,17 @@ export default { unitsConversionFractionDigits: [0], unitsConversionPrefix: [""], unitsConversionBase: [""], + unitsConversionLabelMode: [""], dbUnitsConversionMethod: [""], dbUnitsConversionDivider: [-1], dbUnitsConversionFractionDigits: [0], dbUnitsConversionPrefix: [""], dbUnitsConversionBase: [""], + dbUnitsConversionLabelMode: [""], - temperature: "celsius", + // unset means "render whatever the agent reported"; only an explicit choice converts + temperature: undefined, secondsAsTime: true, timezone: undefined, locale: undefined, diff --git a/src/sdk/makeChart/camelizePayload.js b/src/sdk/makeChart/camelizePayload.js index 8fae06341..03b1a9580 100644 --- a/src/sdk/makeChart/camelizePayload.js +++ b/src/sdk/makeChart/camelizePayload.js @@ -1,6 +1,5 @@ import { heatmapOrChartType } from "@/helpers/heatmap" -import { isStateUnits } from "@/helpers/stepped" -import { getAlias } from "@/helpers/units" +import { getAlias, isStateUnits } from "@/helpers/units" import normalizeSelectedInstances from "@/helpers/normalizeSelectedInstances" import { getPointValue } from "./getPointValue" diff --git a/src/sdk/makeChart/filters/getAggregateMethod.js b/src/sdk/makeChart/filters/getAggregateMethod.js index 349c27bac..d7f96e279 100644 --- a/src/sdk/makeChart/filters/getAggregateMethod.js +++ b/src/sdk/makeChart/filters/getAggregateMethod.js @@ -1,4 +1,4 @@ -import { stateUnits } from "@/helpers/stepped" +import { isStateUnits } from "@/helpers/units" const averageUnits = new Set([ "%", @@ -66,7 +66,7 @@ export default chart => { let lowerUnit = unit.toLowerCase() if (averageUnits.has(unit) || averageRegex.test(lowerUnit)) return "avg" - if (stateUnits.has(unit) || stateUnits.has(lowerUnit)) return "sum" + if (isStateUnits(unit)) return "sum" return "avg" } diff --git a/src/sdk/makeChart/makeDimensions.js b/src/sdk/makeChart/makeDimensions.js index a2a1c188b..2a3af1531 100644 --- a/src/sdk/makeChart/makeDimensions.js +++ b/src/sdk/makeChart/makeDimensions.js @@ -409,9 +409,10 @@ export default (chart, sdk) => { const method = chart.getAttribute(`${key}ConversionMethod`)[unitIndex] const fractionDigits = chart.getAttribute(`${key}ConversionFractionDigits`)[unitIndex] const divider = chart.getAttribute(`${key}ConversionDivider`)[unitIndex] + const labelMode = chart.getAttribute(`${key}ConversionLabelMode`)?.[unitIndex] const unit = chart.getAttribute(key)[unitIndex] - return { method, fractionDigits, base, prefix, divider, unit } + return { method, fractionDigits, base, prefix, divider, labelMode, unit } } chart.getUnitAttributesForValue = ( diff --git a/src/sdk/makeChart/makeGetUnitSign.js b/src/sdk/makeChart/makeGetUnitSign.js index a3d2f8067..558783ba4 100644 --- a/src/sdk/makeChart/makeGetUnitSign.js +++ b/src/sdk/makeChart/makeGetUnitSign.js @@ -9,7 +9,7 @@ export default chart => unitAttributes, value, } = {}) => { - const { base, prefix, unit } = + const { base, prefix, unit, labelMode } = unitAttributes || (typeof value === "undefined" ? chart.getUnitAttributes(dimensionId, key) @@ -17,5 +17,5 @@ export default chart => if (withoutConversion) return getNormalizedUnitConfig(unit).name - return getUnitsString(unit, prefix, base, long) + return getUnitsString(unit, prefix, base, long, { mode: labelMode }) })