diff --git a/components/LogViewer.tsx b/components/LogViewer.tsx index 642d403..bb8e2f2 100644 --- a/components/LogViewer.tsx +++ b/components/LogViewer.tsx @@ -3,10 +3,10 @@ import { Result, Rx, useRxSet, useRxSuspenseSuccess } from "@effect-rx/rx-react"; import { FetchHttpClient, HttpClient } from "@effect/platform"; import { Effect, Stream } from "effect"; +import { useMemo } from "react"; import { rpcClient } from "@/app/api/client"; import { SchemaName, VerboseLogRequest, VerboseLogURLRequest } from "@/services/Domain"; -import { useMemo } from "react"; const runtime = Rx.runtime(FetchHttpClient.layer); diff --git a/components/PipelineHealth.tsx b/components/PipelineHealth.tsx index 8449bc0..1dbddf7 100644 --- a/components/PipelineHealth.tsx +++ b/components/PipelineHealth.tsx @@ -1,7 +1,7 @@ "use client"; import { Result, useRx, useRxSet, useRxValue } from "@effect-rx/rx-react"; -import { DateTime, Duration, Effect } from "effect"; +import { DateTime } from "effect"; import { Suspense, useMemo } from "react"; import { AggregateBySelector } from "@/components/PipelineHealth/AggregateBySelector"; @@ -22,15 +22,6 @@ export function PipelineHealth() { const pullTimeSeriesData = useRxSet(timeSeriesGroupedRx); useMemo(pullTimeSeriesData, [pullTimeSeriesData]); - const updateFrom = useRxSet(fromRx); - useMemo( - () => updateFrom(Effect.runSync(DateTime.now).pipe(DateTime.subtractDuration(Duration.days(3)))), - [updateFrom] - ); - - const updateUntil = useRxSet(untilRx); - useMemo(() => updateUntil(Effect.runSync(DateTime.now)), [updateUntil]); - // Gets const from = useRxValue(fromRx); const until = useRxValue(untilRx); diff --git a/components/PipelineHealth/AggregateBySelector.tsx b/components/PipelineHealth/AggregateBySelector.tsx index 9431d37..3709296 100644 --- a/components/PipelineHealth/AggregateBySelector.tsx +++ b/components/PipelineHealth/AggregateBySelector.tsx @@ -1,3 +1,26 @@ +/** + * Aggregation determines at what interval the time series data is grouped by. + * The time series data can be grouped by seconds, minutes, hours, days, months, + * or years. The AggregateBySelector component is a dropdown menu that allows + * the user to select the aggregation interval. + * + * Aggregation by seconds and minutes is disabled when include empty buckets is + * enabled because otherwise the graph would be too dense / render poorly. The + * database service only returns the raw data from the database and we apply + * this transformation and empty buckets on the client so that we don't have to + * refetch data when the user changes this selection. + * + * TODO: I think aggregation by seconds and minutes does make sense so long as + * the time range is small enough. A quantifiable metric might be the number of + * resultant buckets, i.e if selecting seconds would result in more than N + * buckets (where N is a threshold determined by testing until the graph renders + * poorly) then disable it. This metric should be implemented by pulling in the + * RowsRx, and taking all the keys from that map. The grouping function from the + * RowsRx should be pulled out so that it could be reused here, and then the + * same grouping logic should be applied here for each aggregation method, + * disabling all aggregation methods that exceed the threshold. + */ + "use client"; import { useRx, useRxValue } from "@effect-rx/rx-react"; diff --git a/components/PipelineHealth/EmptyBucketsToggle.tsx b/components/PipelineHealth/EmptyBucketsToggle.tsx index 21c14cf..80c0c10 100644 --- a/components/PipelineHealth/EmptyBucketsToggle.tsx +++ b/components/PipelineHealth/EmptyBucketsToggle.tsx @@ -1,3 +1,24 @@ +/** + * Empty buckets determines whether or not to show buckets that have no data. + * This is useful for showing gaps in data, but is disabled by default for + * performance reasons. + * + * When empty buckets are enable, after the data is aggregated, we will fill in + * any missing buckets with empty data. The database service only returns the + * raw data from the database and we apply this transformation and aggregation + * on the client so that we don't have to refetch data when the user changes + * this selection. + * + * If aggregation by seconds or minutes is enabled, empty buckets will be + * disabled because the graph would be too dense and render poorly. + * + * TODO: I think showing empty buckets does make sense so long as the number of + * buckets does not exceed a threshold that would cause the graph to render + * poorly. This threshold could be determined through testing. A quantifiable + * metric would be the number of buckets after aggregation is applied. See the + * comment in AggregateBySelector.tsx for more details. + */ + "use client"; import { useRx, useRxValue } from "@effect-rx/rx-react"; diff --git a/components/PipelineHealth/LocaleSelector.tsx b/components/PipelineHealth/LocaleSelector.tsx index c3802ce..334258a 100644 --- a/components/PipelineHealth/LocaleSelector.tsx +++ b/components/PipelineHealth/LocaleSelector.tsx @@ -1,3 +1,23 @@ +/** + * When I started the image health website, I used "locale" when what I actually + * meant was "timezone." TODO: replace "locale" with "timezone" throughout the + * app. + * + * This component is a dropdown menu that allows the user to select a timezone. + * By default, the timezone should be set to the timezone of the user's device. + * Any times referenced in the app (outside of updateFromRx and updateUntilRx) + * should be in this timezone. + * + * The exceptions for updateFromRx and updateUntilRx are intentional. When + * setting a new time for the range, you can give it any time in any time range. + * The timezone will then be applied to the datetime without changing the time + * itself. This approach has serval advantages over tracking everything as a UTC + * datetime or a Zoned datetime or a Local datetime. And not needing to make + * sure that the timezone is correct when setting a new from/until rx is + * important because those times should always come from the calendar and it is + * easy to forget to offset for the timezone otherwise. + */ + "use client"; import { Result, useRx } from "@effect-rx/rx-react"; diff --git a/components/PipelineHealth/PipelineStepHistogram.tsx b/components/PipelineHealth/PipelineStepHistogram.tsx index a4b9dce..daa8d6a 100644 --- a/components/PipelineHealth/PipelineStepHistogram.tsx +++ b/components/PipelineHealth/PipelineStepHistogram.tsx @@ -1,13 +1,13 @@ "use client"; +import { useRxSuspenseSuccess } from "@effect-rx/rx-react"; import { Array, Function, Option, Record, Schema, Tuple } from "effect"; import { Bar, BarChart, CartesianGrid, XAxis, YAxis } from "recharts"; +import { rowsRx } from "@/components/PipelineHealth/rx"; import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card"; import { ChartConfig, ChartContainer, ChartTooltip, ChartTooltipContent } from "@/components/ui/chart"; import { PipelineStepName, ShortPipelineName } from "@/services/Domain"; -import { useRxSuspenseSuccess } from "@effect-rx/rx-react"; -import { rowsRx } from "./rx"; const chart1 = "percentPipelineFailure" as const; @@ -44,6 +44,7 @@ export function PipelineStepHistogram() { return group; }) ); + const sorted = bucketsWithFailures.sort( (a, b) => PipelineStepName.literals.indexOf(a.pipelineStep) - PipelineStepName.literals.indexOf(b.pipelineStep) ); @@ -72,9 +73,9 @@ export function PipelineStepHistogram() { tickLine={true} axisLine={false} tickMargin={8} - tickFormatter={(longname: typeof PipelineStepName.Type) => { - return Schema.decodeSync(ShortPipelineName)(longname); - }} + tickFormatter={(longName: typeof PipelineStepName.Type) => + Schema.decodeSync(ShortPipelineName)(longName) + } /> `${value}`} /> } /> diff --git a/components/PipelineHealth/RunTimeHist.tsx b/components/PipelineHealth/RunTimeHist.tsx index 0b2cbcb..85ff0e6 100644 --- a/components/PipelineHealth/RunTimeHist.tsx +++ b/components/PipelineHealth/RunTimeHist.tsx @@ -43,7 +43,7 @@ export const chartConfigs = { }, [chart5]: { color: "#FF0000", - title: "Number of Failed of Runs", + title: "Number of Failed Runs", label: "Number of Failed Runs", icon: Cross2Icon, }, @@ -101,7 +101,8 @@ export function AverageProcessingTimeLineChart() { }) ) ); - const activeChartKey = activeChart === "All" ? chart6 : activeChart === "success" ? chart2 : chart3; + const activeChartKey = activeChart === "all" ? chart6 : activeChart === "success" ? chart2 : chart3; + // Chart implementation return ( @@ -121,7 +122,7 @@ export function AverageProcessingTimeLineChart() { className="flex flex-1 flex-col justify-center gap-1 border-t px-6 py-4 text-left even:border-l data-[active=true]:bg-muted/50 sm:border-l sm:border-t-0 sm:px-8 sm:py-6" onClick={() => chart === "averageProcessingTimeAllRuns" - ? setActiveChart("All") + ? setActiveChart("all") : chart === "averageSuccessProcessingTime" ? setActiveChart("success") : setActiveChart("failure") @@ -201,7 +202,7 @@ export function AverageProcessingTimeLineChart() { height={36} /> - {activeChart === "All" ? ( + {activeChart === "all" ? ( )} - {activeChart === "All" ? ( + {activeChart === "all" ? ( )} - {activeChart !== "All" ? ( + {activeChart !== "all" ? ( { const start = filePath.indexOf("telescope_"); diff --git a/components/PipelineHealth/rx.ts b/components/PipelineHealth/rx.ts index 22db2fc..48bc7d2 100644 --- a/components/PipelineHealth/rx.ts +++ b/components/PipelineHealth/rx.ts @@ -11,12 +11,10 @@ import { Effect, Function, HashSet, - Layer, - Logger, - LogLevel, Match, Number, Option, + Predicate, Record, Schema, Sink, @@ -26,14 +24,17 @@ import { import { rpcClient } from "@/app/api/client"; import { PipelineStepName, ResultRow, RunsInTimeRangeRequest, SchemaName, ShortPipelineName } from "@/services/Domain"; -// Rx runtime -const runtime = Rx.runtime(Layer.provideMerge(FetchHttpClient.layer, Logger.minimumLogLevel(LogLevel.All))); +/** Rx runtime. */ +const runtime = Rx.runtime(FetchHttpClient.layer); // ------------------------------------------------------------ // Rx Atoms for pipeline health page // ------------------------------------------------------------ -// localeRx tracks the current timezone/locale that the user has selected, which is an IANA time zone identifier +/** + * "localeRx" tracks the current timezone/locale that the user has selected, + * which is an IANA time zone identifier. + */ export const localeRx = Rx.fn( (locale: string, _ctx: Rx.Context): Effect.Effect => Function.pipe( @@ -42,35 +43,124 @@ export const localeRx = Rx.fn( Effect.succeed ), { - initialValue: DateTime.zoneMakeLocal(), + /** + * Default timezone is UTC because this could be on the server if in an + * SSR context? So we might not know the users timezone. Also, we + * wouldn't be able to set an initial value for from and until because + * what timezone would this be then? + */ + initialValue: DateTime.zoneUnsafeMakeNamed("UTC"), } ); -// fromRx tracks the start of the time range that the user has selected -export const fromRx = Rx.fn( - (datetime: DateTime.DateTime, ctx: Rx.Context): Effect.Effect => - Effect.map(ctx.result(localeRx), (locale) => DateTime.setZone(datetime, locale)) +/** + * "fromRx" tracks the start of the time range that the user has selected. When + * setting this value, the attached timezone is ignored and the timezone from + * the "localeRx" is attached instead. This is because this time should always + * be set using the calendar component, where the user is supplying the exact + * time they want in the locale they have selected. So it makes sense to ignore + * any timezone and the local timezone and just attach the users selected + * timezone instead. + */ +export const fromRx = Rx.fn( + ( + input: Date | DateTime.DateTime | undefined, + ctx: Rx.Context + ): Effect.Effect => + Effect.gen(function* () { + const locale = yield* ctx.result(localeRx); + const setZone = DateTime.setZone(locale); + const now = yield* DateTime.now; + + const datetime = Function.pipe( + Match.value(input), + Match.when(Predicate.isDate, (d) => DateTime.make(d)), + Match.when(DateTime.isDateTime, (d) => Option.some(d)), + Match.when(Predicate.isUndefined, (_) => Option.some(now)), + Match.exhaustive + ); + + if (Option.isNone(datetime)) { + return yield* Effect.fail(new Cause.IllegalArgumentException("Invalid date")); + } + + return setZone(datetime.value); + }), + { + /** Default is 72 hours ago. */ + initialValue: Function.pipe( + Effect.runSync(DateTime.now), + DateTime.subtractDuration(Duration.hours(72)), + DateTime.setZone(DateTime.zoneUnsafeMakeNamed("UTC")) + ), + } ); -// untilRx tracks the end of the time range that the user has selected -export const untilRx = Rx.fn( - (datetime: DateTime.DateTime, ctx: Rx.Context): Effect.Effect => - Effect.map(ctx.result(localeRx), (locale) => DateTime.setZone(datetime, locale)) +/** + * "untilRx" tracks the end of the time range that the user has selected. When + * setting this value, the attached timezone is ignored and the timezone from + * the "localeRx" is attached instead. This is because this time should always + * be set using the calendar component, where the user is supplying the exact + * time they want in the locale they have selected. So it makes sense to ignore + * any timezone and the local timezone and just attach the users selected + * timezone instead. + */ +export const untilRx = Rx.fn( + ( + input: Date | DateTime.DateTime | undefined, + ctx: Rx.Context + ): Effect.Effect => + Effect.gen(function* () { + const locale = yield* ctx.result(localeRx); + const setZone = DateTime.setZone(locale); + const now = yield* DateTime.now; + + const datetime = Function.pipe( + Match.value(input), + Match.when(Predicate.isDate, (d) => DateTime.make(d)), + Match.when(DateTime.isDateTime, (d) => Option.some(d)), + Match.when(Predicate.isUndefined, (_) => Option.some(now)), + Match.exhaustive + ); + + if (Option.isNone(datetime)) { + return yield* Effect.fail(new Cause.IllegalArgumentException("Invalid date")); + } + + return setZone(datetime.value); + }), + { + /** Default is now. */ + initialValue: Function.pipe( + Effect.runSync(DateTime.now), + DateTime.subtractDuration(Duration.millis(0)), + DateTime.setZone(DateTime.zoneUnsafeMakeNamed("UTC")) + ), + } ); -// includeEmptyBucketsRx tracks whether or not to include empty buckets in the time series data +/** + * "includeEmptyBucketsRx" tracks whether or not to include empty buckets in the + * time series data. + */ export const includeEmptyBucketsRx = Rx.make(false); -// activeLabelRx tracks the currently selected label +/** "activeLabelRx" tracks the currently selected label. */ export const activeLabelRx = Rx.make(undefined); -// activeDataRx tracks whether the user is looking at successful or failed runs -export const activeDataRx = Rx.make<"success" | "failure" | "All">("All" as const); +/** + * "activeDataRx" tracks whether the user is looking at successful or failed + * runs. + */ +export const activeDataRx = Rx.make<"success" | "failure" | "all">("all" as const); -// aggregateByRx tracks the time unit that the user has selected to aggregate the time series data by +/** + * "aggregateByRx" tracks the time unit that the user has selected to aggregate + * the time series data by. + */ export const aggregateByRx = Rx.make>("days"); -// creating list of Pipeline to select from when querying +/** "steps2queryRx" tracks the list of pipeline steps to show in the graphs. */ export const steps2queryRx = Rx.make>( HashSet.fromIterable(PipelineStepName.literals) ); @@ -79,9 +169,12 @@ export const steps2queryRx = Rx.make, never> = runtime.fn( - (_: void, ctx: Rx.Context): Effect.Effect, never, HttpClient.HttpClient> => +/** Fetches all the rows from the database in the time range. */ +export const rowsRx: Rx.RxResultFn, Cause.IllegalArgumentException> = runtime.fn( + ( + _: void, + ctx: Rx.Context + ): Effect.Effect, Cause.IllegalArgumentException, HttpClient.HttpClient> => Effect.Do.pipe( Effect.bind("from", () => ctx.result(fromRx).pipe(Effect.map(DateTime.toUtc))), Effect.bind("until", () => ctx.result(untilRx).pipe(Effect.map(DateTime.toUtc))), @@ -94,7 +187,7 @@ export const rowsRx: Rx.RxResultFn, never> = runtime.fn( ) ); -// Computes the success rate, failure rate, and total number of runs +/** Computes the success rate, failure rate, and total number of runs. */ export const totalsRx: Rx.Rx< Result.Result< { @@ -102,10 +195,16 @@ export const totalsRx: Rx.Rx< failureRate: number; totalRuns: number; }, - never + Cause.IllegalArgumentException > > = runtime.rx( - (ctx: Rx.Context): Effect.Effect<{ successRate: number; failureRate: number; totalRuns: number }, never, never> => + ( + ctx: Rx.Context + ): Effect.Effect< + { successRate: number; failureRate: number; totalRuns: number }, + Cause.IllegalArgumentException, + never + > => Effect.gen(function* () { const rows = yield* ctx.result(rowsRx); const total = rows.length; @@ -116,7 +215,7 @@ export const totalsRx: Rx.Rx< }) ); -// Computes the average processing time for successful and failed runs +/** Computes the average processing time for successful and failed runs. */ export const timeSeriesGroupedRx: Rx.RxResultFn< void, Record< @@ -130,7 +229,7 @@ export const timeSeriesGroupedRx: Rx.RxResultFn< numberSuccessfulRuns: number; } >, - never + Cause.IllegalArgumentException > = runtime.fn( ( _: void, @@ -147,7 +246,7 @@ export const timeSeriesGroupedRx: Rx.RxResultFn< numberSuccessfulRuns: number; } >, - never, + Cause.IllegalArgumentException, never > => Effect.gen(function* () { @@ -230,7 +329,10 @@ export const timeSeriesGroupedRx: Rx.RxResultFn< }) ); -// Formats the time series data into a format that can be displayed in the table +/** + * Formats the time series data into a format that can be displayed in the + * table. + */ export const tableDataRx: Rx.Rx< Result.Result< Array<{ @@ -244,7 +346,7 @@ export const tableDataRx: Rx.Rx< pipelineStepName: typeof PipelineStepName.Type; shortPipelineStepName: typeof ShortPipelineName.to.Type; }>, - never + Cause.IllegalArgumentException > > = runtime.rx( ( @@ -261,7 +363,7 @@ export const tableDataRx: Rx.Rx< pipelineStepName: typeof PipelineStepName.Type; shortPipelineStepName: typeof ShortPipelineName.to.Type; }>, - never, + Cause.IllegalArgumentException, never > => Effect.gen(function* () {