Compare commits

...

2 Commits

Author SHA1 Message Date
Gaurav Tewari
b7962bac4d feat: update datepicker interaction 2026-08-10 18:05:58 +05:30
Pandey
f44d6c7c84 docs(contributing): document the kind/spec envelope for sum types (#12494)
#### Description

- Documents the kind/spec envelope pattern for sum types in
`docs/contributing/go/types.md`: the envelope shape, why it goes at the
point of variance rather than the resource root, the tagging-style
rationale (adjacently tagged vs internally tagged vs sibling optional
fields), the validating `UnmarshalJSON`, the OpenAPI variant structs,
and the data-migration-vs-storable-twin trade-off for legacy persisted
shapes.
- Examples are generic (`FooConfig` with `bar`/`baz` kinds), with
`RuleThresholdData`, `EvaluationEnvelope` and the dashboard plugins as
the in-tree references.
- Cross-links from `handler.md`'s "`oneOf` with a discriminator"
section, which keeps owning the schema mechanics.
2026-08-10 11:22:33 +00:00
8 changed files with 365 additions and 8 deletions

View File

@@ -349,7 +349,7 @@ func (Step) JSONSchema() (jsonschema.Schema, error) {
### `oneOf` with a discriminator
For a sum type whose variants are keyed by a property (e.g. `kind`), expose the variants via `JSONSchemaOneOf()` and add a discriminator. Without it, code generators intersect the variants (`A & B & C`) instead of producing a clean discriminated union (`A | B | C`).
For a sum type whose variants are keyed by a property (e.g. `kind`), expose the variants via `JSONSchemaOneOf()` and add a discriminator. Without it, code generators intersect the variants (`A & B & C`) instead of producing a clean discriminated union (`A | B | C`). How to model the sum type itself is covered in [types.md](types.md#sum-types-the-kindspec-envelope) — this section is only about its schema.
The parent keeps its `JSONSchemaOneOf()` (the `oneOf` itself) and *additionally* tags it via `PrepareJSONSchema` with the `x-signoz-discriminator` extension; `signoz.attachDiscriminators` then promotes that marker to a real OpenAPI 3 `discriminator` (and strips the duplicate parent properties) after reflection.

View File

@@ -99,6 +99,69 @@ Each flavor exists for a concrete reason:
The core `AuthDomain` holds the two live halves — `storableAuthDomain` and `authDomainConfig` — and owns business methods such as `Update(config)`. Conversions use the `New<Output>From<Input>` form: `NewAuthDomainFromConfig`, `NewAuthDomainFromStorableAuthDomain`, `NewGettableAuthDomainFromAuthDomain`.
## Sum types: the kind/spec envelope
When a domain type is a *sum type* — exactly one of several variants, selected by a discriminator — model it as an envelope with a `kind` and a `spec`:
```go
type FooConfig struct {
Kind FooKind `json:"kind" required:"true"`
Spec any `json:"spec" required:"true"`
}
```
```json
{ "kind": "bar", "spec": { "url": "...", "timeout": "30s" } }
```
`Kind` is a `valuer.String` enum implementing `Enum()`; `Spec` holds exactly one concrete variant type (`BarSpec`, `BazSpec`, …). `RuleThresholdData` and `EvaluationEnvelope` in `pkg/types/ruletypes/` are the canonical in-tree examples; the dashboard panel/query/variable plugins in `pkg/types/dashboardtypes/` are the same pattern behind generics. (`QueryEnvelope` in querybuildertypes uses `type` as the discriminator key for historical reasons; new envelopes use `kind`.)
### The envelope goes at the point of variance, not the resource root
Put the envelope on the field that actually varies. The resource root is almost never a sum type — a `Foo` has a `name` and an `enabled` flag regardless of which kind it is configured with; only its configuration varies, so the envelope is the `config` field:
```json
{ "name": "my-foo", "enabled": true, "config": { "kind": "bar", "spec": { "...": "..." } } }
```
Hoisting `kind`/`spec` to the root would turn the whole resource into a `oneOf`: every flavor (`PostableFoo`, `UpdatableFoo`, `GettableFoo`) then needs one variant schema per kind, each repeating the common fields; every new common field has to be added to all of them; and generated clients get unions of large objects instead of one small union that narrows on `config.kind`. A root-level `kind` also collides with the resource-model meaning of the word — root `kind` conventionally answers "what resource is this" (`Dashboard`), never "which flavor of config does it hold".
The existing domains already follow this placement:
- **Rules** — plain root; envelopes on the varying fields: `thresholds: {kind, spec}` and `evaluation: {kind, spec}`.
- **Dashboards** — metadata at the root plus one typed `spec`; the unions sit deep inside, at each panel/query/variable plugin (`{kind, spec}` in `perses_plugin_wrappers.go`).
- **Saved views** — root `{schemaVersion, spec}`, where `spec` is a *versioning* envelope holding one fixed type, not a union; the unions are inside it (`spec.queries: [{type, spec}]`). Same word, different job — a versioned body is not a discriminated union.
### Why this tagging style
Of the union encodings in common use, the envelope is the *adjacently tagged* one — tag and payload side by side. Variant payloads stay collision-free, and each kind maps to a named wrapper schema that carries the discriminator, which is exactly what OpenAPI generators need. The alternatives lose on those points: *internally tagged* (`{"kind": "bar", ...fields flattened}`) mixes common and variant fields, admits cross-variant key collisions, and forces every variant schema to redeclare the discriminator; *sibling optional fields* (`{"kind": "bar", "barConfig": {}, "bazConfig": {}}`) is the anti-pattern the first rule below exists to prevent.
The rules that make the envelope work:
- **Never model variants as sibling fields.** A struct with `Bar *BarSpec`, `Baz *BazSpec` next to a discriminator cannot be expressed as an OpenAPI discriminated union, forces nilability checks on every consumer, and silently admits contradictory payloads (kind=bar with a baz spec). The chosen variant *is* the payload.
- **The envelope owns `UnmarshalJSON`.** Decode `kind` first, then switch on it to decode and validate the matching concrete type into `Spec`. Unknown kinds and missing specs are rejected at the boundary:
```go
func (typ *FooConfig) UnmarshalJSON(data []byte) error {
var raw map[string]json.RawMessage
// ... unmarshal raw, decode raw["kind"] ...
switch kind {
case FooKindBar:
spec := BarSpec{}
if err := json.Unmarshal(raw["spec"], &spec); err != nil {
return err
}
typ.Spec = spec
// ... one case per kind, default rejects ...
}
typ.Kind = kind
return nil
}
```
- **Consumers type-assert on `Spec`** (`config.Spec.(BarSpec)`) after switching on `Kind`. If assertion sites multiply, add typed accessors on the envelope (see `EvaluationEnvelope.GetEvaluation()`).
- **OpenAPI needs one unexported variant struct per kind** (`fooConfigBar{Kind; Spec BarSpec}`), exposed via `JSONSchemaOneOf()` and mapped via `PrepareJSONSchema` with the `x-signoz-discriminator` extension. The schema mechanics are covered in [handler.md](handler.md#oneof-with-a-discriminator).
- **A legacy persisted shape gets a data migration or a `StorableX`.** When rows were written before the envelope existed, prefer an idempotent `sqlmigration` that rewrites them into the new shape, so the storable type simply nests the envelope. Only when the old shape must keep being written (external writers, rollback windows) keep it in a storable twin and convert at the type boundary.
## Conventions that tie the flavors together
- **Conversions** use either a `New<Output>From<Input>` constructor — e.g. `NewChannelFromReceiver`, `NewGettableAuthDomainFromAuthDomain` — or a receiver-style `ToY()` method. Both forms coexist in the codebase; use whichever fits the call site.
@@ -139,6 +202,8 @@ Both are optional. Do not introduce them if `PostableX` already covers the case.
- Every domain package defines the core type `X`. Only `X` is mandatory.
- Add `PostableX` / `GettableX` / `UpdatableX` / `StorableX` one at a time, only when the shape actually diverges from `X`.
- Model sum types as a `{kind, spec}` envelope with a validating `UnmarshalJSON` — never as sibling variant fields next to a discriminator.
- The envelope goes on the field that varies, never at the resource root — common fields stay on the resource, outside the union.
- Domain logic lives on `X`, not on the flavor types.
- Conversions can be a `New<Output>From<Input>` constructor or a receiver-style `ToY()` method — pick whichever reads best at the call site.
- Use a type alias when two shapes are truly identical.

View File

@@ -84,6 +84,45 @@
color: rgba($color: var(--l1-foreground), $alpha: 0.4);
}
}
&.invalid-flash {
animation:
timeSelection-input-shake 300ms ease-out,
timeSelection-input-invalid-flash 1200ms ease-out;
input {
background-color: transparent;
}
}
}
@keyframes timeSelection-input-shake {
0%,
100% {
transform: translateX(0);
}
25%,
75% {
transform: translateX(5px);
}
50% {
transform: translateX(-5px);
}
}
@keyframes timeSelection-input-invalid-flash {
0%,
50% {
background-color: color-mix(in srgb, var(--bg-cherry-500) 18%, transparent);
}
}
@media (prefers-reduced-motion: reduce) {
.timeSelection-input.invalid-flash {
animation: none;
}
}
.valid-format-error {

View File

@@ -1,9 +1,10 @@
import { useState } from 'react';
import { fireEvent, render, screen } from '@testing-library/react';
import { act, fireEvent, render, screen } from '@testing-library/react';
import dayjs from 'dayjs';
import * as timeUtils from 'utils/timeUtils';
import CustomTimePicker from './CustomTimePicker';
import { INVALID_FLASH_DURATION_MS } from './useInvalidFlash';
jest.mock('react-router-dom', () => {
const actual = jest.requireActual('react-router-dom');
@@ -285,4 +286,102 @@ describe('CustomTimePicker', () => {
expect((input as HTMLInputElement).value).toBe('Live');
});
describe('invalid entry flash', () => {
const FLASH_START_DELAY_MS = 50;
const enterInvalidRange = (input: HTMLElement): void => {
fireEvent.focus(input);
fireEvent.change(input, {
target: { value: '10/08/2026 14:30 - 10/08/2026 15:30' },
});
fireEvent.keyDown(input, { key: 'Enter', code: 'Enter' });
};
const getFieldWrapper = (input: HTMLElement): HTMLElement =>
input.closest('.timeSelection-input') as HTMLElement;
beforeEach(() => {
jest.useFakeTimers();
});
afterEach(() => {
jest.useRealTimers();
});
it('flashes the field, then clears the flash on its own', () => {
render(<Wrapper />);
const input = screen.getByRole('textbox');
enterInvalidRange(input);
act(() => {
jest.advanceTimersByTime(FLASH_START_DELAY_MS);
});
expect(getFieldWrapper(input)).toHaveClass('invalid-flash');
act(() => {
jest.advanceTimersByTime(INVALID_FLASH_DURATION_MS);
});
expect(getFieldWrapper(input)).not.toHaveClass('invalid-flash');
});
it('flashes again on a second consecutive invalid entry', () => {
render(<Wrapper />);
const input = screen.getByRole('textbox');
enterInvalidRange(input);
act(() => {
jest.advanceTimersByTime(FLASH_START_DELAY_MS + INVALID_FLASH_DURATION_MS);
});
expect(getFieldWrapper(input)).not.toHaveClass('invalid-flash');
enterInvalidRange(input);
act(() => {
jest.advanceTimersByTime(FLASH_START_DELAY_MS);
});
expect(getFieldWrapper(input)).toHaveClass('invalid-flash');
});
it('drops the error state when closing restores the previous value', () => {
const onError = jest.fn();
render(<Wrapper onError={onError} />);
const input = screen.getByRole('textbox');
enterInvalidRange(input);
act(() => {
jest.advanceTimersByTime(FLASH_START_DELAY_MS);
});
expect(getFieldWrapper(input)).toHaveClass('error');
// Chevron close without an intervening blur takes the branch that reverts
// the input to the previously applied range
fireEvent.click(
document.querySelector('.time-input-suffix-icon-badge') as HTMLElement,
);
expect(getFieldWrapper(input)).not.toHaveClass('error');
expect((input as HTMLInputElement).value).toBe(
'2024-01-01 00:00:00 - 2024-01-01 01:00:00',
);
expect(onError).toHaveBeenLastCalledWith(false);
});
it('keeps the persistent error styling after the flash has gone', () => {
render(<Wrapper />);
const input = screen.getByRole('textbox');
enterInvalidRange(input);
act(() => {
jest.advanceTimersByTime(FLASH_START_DELAY_MS + INVALID_FLASH_DURATION_MS);
});
expect(getFieldWrapper(input)).toHaveClass('error');
expect(getFieldWrapper(input)).not.toHaveClass('invalid-flash');
});
});
});

View File

@@ -30,6 +30,7 @@ import { popupContainer } from 'utils/selectPopupContainer';
import { TimeRangeValidationResult, validateTimeRange } from 'utils/timeUtils';
import CustomTimePickerPopoverContent from './CustomTimePickerPopoverContent';
import { useInvalidFlash } from './useInvalidFlash';
import './CustomTimePicker.styles.scss';
@@ -106,6 +107,7 @@ function CustomTimePicker({
const [inputErrorDetails, setInputErrorDetails] = useState<
TimeRangeValidationResult['errorDetails'] | null
>(null);
const { isFlashing, triggerFlash } = useInvalidFlash();
const location = useLocation();
const inputRef = useRef<InputRef>(null);
@@ -275,6 +277,9 @@ function CustomTimePicker({
if (!newOpen) {
setCustomDTPickerVisible?.(false);
setActiveView('datetime');
// The rejected value is being discarded in favour of the previous one, so
// the error it raised must not outlive it
resetErrorStatus();
if (showLiveLogs) {
setSelectedTimePlaceholderValue('Live');
@@ -340,6 +345,7 @@ function CustomTimePicker({
if (minTime && (!minTime.isValid() || minTime < maxAllowedMinTime)) {
setInputStatus(CustomTimePickerInputStatus.ERROR);
triggerFlash();
onError(true);
setInputErrorDetails({
message: `Please enter time less than ${maxAllowedMinTimeInMonths} months`,
@@ -392,6 +398,7 @@ function CustomTimePicker({
if (!isValidTimeRange) {
setInputStatus(CustomTimePickerInputStatus.ERROR);
triggerFlash();
onError(true);
setInputErrorDetails(errorDetails || null);
return;
@@ -485,6 +492,9 @@ function CustomTimePicker({
setOpen(false);
setCustomDTPickerVisible?.(false);
// The rejected value is being discarded in favour of the previous one, so
// the error it raised must not outlive it
resetErrorStatus();
if (showLiveLogs) {
setInputValue('Live');
@@ -600,6 +610,7 @@ function CustomTimePicker({
className={cx(
'timeSelection-input',
inputStatus === CustomTimePickerInputStatus.ERROR ? 'error' : '',
isFlashing ? 'invalid-flash' : '',
)}
type="text"
status={

View File

@@ -0,0 +1,47 @@
import { useCallback, useEffect, useRef, useState } from 'react';
export const INVALID_FLASH_DURATION_MS = 1200;
interface UseInvalidFlashResult {
isFlashing: boolean;
triggerFlash: () => void;
}
/**
* Drives a one-shot "invalid input" flash that clears itself after
* `durationMs`, leaving any persistent error styling to the caller.
*
* A re-trigger drops the flag for one frame before setting it again: a CSS
* animation only restarts when the class is genuinely removed and re-added, so
* without that gap a second failed attempt in a row would not animate.
*/
export function useInvalidFlash(
durationMs: number = INVALID_FLASH_DURATION_MS,
): UseInvalidFlashResult {
const [isFlashing, setIsFlashing] = useState(false);
const timeoutRef = useRef<ReturnType<typeof setTimeout>>();
const frameRef = useRef<number>();
const clearPending = useCallback((): void => {
if (timeoutRef.current !== undefined) {
clearTimeout(timeoutRef.current);
}
if (frameRef.current !== undefined) {
cancelAnimationFrame(frameRef.current);
}
}, []);
useEffect(() => clearPending, [clearPending]);
const triggerFlash = useCallback((): void => {
clearPending();
setIsFlashing(false);
frameRef.current = requestAnimationFrame(() => {
setIsFlashing(true);
timeoutRef.current = setTimeout(() => setIsFlashing(false), durationMs);
});
}, [clearPending, durationMs]);
return { isFlashing, triggerFlash };
}

View File

@@ -0,0 +1,78 @@
import { DATE_TIME_FORMATS } from 'constants/dateTimeFormats';
import dayjs from 'dayjs';
import { validateTimeRange } from 'utils/timeUtils';
const FORMAT = DATE_TIME_FORMATS.UK_DATETIME_SECONDS;
const TIMEZONE = 'Africa/Lagos';
const inTimezone = (offsetMinutes: number): string =>
dayjs().tz(TIMEZONE).subtract(offsetMinutes, 'minute').format(FORMAT);
describe('validateTimeRange', () => {
it('accepts a well formed past range', () => {
const result = validateTimeRange(
inTimezone(120),
inTimezone(60),
FORMAT,
TIMEZONE,
);
expect(result.isValid).toBe(true);
expect(result.startTimeMs).toBeLessThan(result.endTimeMs as number);
});
it.each([
['missing seconds', '10/08/2026 14:30'],
['date only', '10/08/2026'],
['empty string', ''],
['unparseable text', 'garbage'],
['truncated minutes', '10/08/2026 14:3'],
])('rejects %s without throwing', (_label, startTime) => {
let result;
expect(() => {
result = validateTimeRange(startTime, inTimezone(60), FORMAT, TIMEZONE);
}).not.toThrow();
expect(result).toMatchObject({
isValid: false,
errorDetails: { code: 'INVALID_DATE_TIME_FORMAT' },
});
});
it('rejects a missing end time instead of defaulting it to now', () => {
const result = validateTimeRange(
inTimezone(60),
undefined as unknown as string,
FORMAT,
TIMEZONE,
);
expect(result.isValid).toBe(false);
expect(result.errorDetails?.code).toBe('INVALID_DATE_TIME_FORMAT');
});
it('rejects future dates', () => {
const result = validateTimeRange(
inTimezone(-120),
inTimezone(-60),
FORMAT,
TIMEZONE,
);
expect(result.isValid).toBe(false);
expect(result.errorDetails?.code).toBe('DATES_IN_THE_FUTURE');
});
it('rejects a range where start is not before end', () => {
const result = validateTimeRange(
inTimezone(60),
inTimezone(120),
FORMAT,
TIMEZONE,
);
expect(result.isValid).toBe(false);
expect(result.errorDetails?.code).toBe('START_TIME_AFTER_END_TIME');
});
});

View File

@@ -1,5 +1,5 @@
import { DATE_TIME_FORMATS } from 'constants/dateTimeFormats';
import dayjs from 'dayjs';
import dayjs, { Dayjs } from 'dayjs';
import customParseFormat from 'dayjs/plugin/customParseFormat';
import duration from 'dayjs/plugin/duration';
import relativeTime from 'dayjs/plugin/relativeTime';
@@ -220,6 +220,23 @@ export interface TimeRangeValidationResult {
endTimeMs?: number;
}
const safeParseInTimezone = (
value: string,
format: string,
timezone: string,
): Dayjs | null => {
if (!value || !dayjs(value, format).isValid()) {
return null;
}
try {
const parsed = dayjs.tz(value, format, timezone);
return parsed.isValid() ? parsed : null;
} catch {
return null;
}
};
/**
* Validates a start and end datetime string.
*
@@ -243,14 +260,12 @@ export const validateTimeRange = (
format: string,
timezone: string,
): TimeRangeValidationResult => {
const start = dayjs.tz(startTime, format, timezone);
const end = dayjs.tz(endTime, format, timezone);
const start = safeParseInTimezone(startTime, format, timezone);
const end = safeParseInTimezone(endTime, format, timezone);
const now = dayjs().tz(timezone);
const startTimeMs = start.valueOf();
const endTimeMs = end.valueOf();
// Invalid format or parsing failure
if (!start.isValid() || !end.isValid()) {
if (!start || !end) {
return {
isValid: false,
errorDetails: {
@@ -270,6 +285,9 @@ Shortcuts:
};
}
const startTimeMs = start.valueOf();
const endTimeMs = end.valueOf();
// dates must not be in the future
if (start.isAfter(now) || end.isAfter(now)) {
return {