Elgato_dark/hyperdx
1
0
Fork 0
mirror of https://github.com/hyperdxio/hyperdx synced 2026-04-21 13:37:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

feat: add robust source form validation and error reporting (#923)

Browse source 
Co-authored-by: Tom Alexander <teeohhem@gmail.com>
This commit is contained in:
Aaron Knudtson 2025-06-25 12:41:42 -04:00 committed by GitHub
parent d4db9fbc8f
commit b75d7c0595
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 331 additions and 199 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
.changeset/dry-ducks-shake.md Normal file
View file

@ -0,0 +1,6 @@
---
"@hyperdx/common-utils": patch
"@hyperdx/app": patch
---
feat: add robust source form validation and error reporting

5
packages/api/src/routers/api/__tests__/sources.test.ts
View file

@ -1,10 +1,10 @@
import { SourceKind } from '@hyperdx/common-utils/dist/types';
import { SourceKind, TSourceUnion } from '@hyperdx/common-utils/dist/types';
import { Types } from 'mongoose';
import { getLoggedInAgent, getServer } from '@/fixtures';
import { Source } from '@/models/source';
const MOCK_SOURCE = {
const MOCK_SOURCE: Omit<Extract<TSourceUnion, { kind: 'log' }>, 'id'> = {
kind: SourceKind.Log,
name: 'Test Source',
connection: new Types.ObjectId().toString(),
@ -13,6 +13,7 @@ const MOCK_SOURCE = {
tableName: 'test_table',
},
timestampValueExpression: 'timestamp',
defaultTableSelectExpression: 'body',
};
describe('sources router', () => {

15
packages/api/src/routers/api/sources.ts
View file

@ -1,4 +1,7 @@
import { SourceSchema } from '@hyperdx/common-utils/dist/types';
import {
SourceSchema,
sourceSchemaWithout,
} from '@hyperdx/common-utils/dist/types';
import express from 'express';
import { z } from 'zod';
import { validateRequest } from 'zod-express-middleware';
@ -26,19 +29,22 @@ router.get('/', async (req, res, next) => {
}
});
const SourceSchemaNoId = sourceSchemaWithout({ id: true });
router.post(
'/',
validateRequest({
body: SourceSchema.omit({ id: true }),
body: SourceSchemaNoId,
}),
async (req, res, next) => {
try {
const { teamId } = getNonNullUserWithTeam(req);
// TODO: HDX-1768 Eliminate type assertion
const source = await createSource(teamId.toString(), {
...req.body,
team: teamId,
});
} as any);
res.json(source);
} catch (e) {
@ -59,10 +65,11 @@ router.put(
try {
const { teamId } = getNonNullUserWithTeam(req);
// TODO: HDX-1768 Eliminate type assertion
const source = await updateSource(teamId.toString(), req.params.id, {
...req.body,
team: teamId,
});
} as any);
if (!source) {
res.status(404).send('Source not found');

7
packages/app/src/components/SQLInlineEditor.tsx
View file

@ -106,6 +106,7 @@ type SQLInlineEditorProps = {
onLanguageChange?: (language: 'sql' | 'lucene') => void;
language?: 'sql' | 'lucene';
onSubmit?: () => void;
error?: React.ReactNode;
size?: string;
label?: React.ReactNode;
disableKeywordAutocomplete?: boolean;
@ -134,6 +135,7 @@ export default function SQLInlineEditor({
onLanguageChange,
language,
onSubmit,
error,
value,
size,
label,
@ -260,7 +262,7 @@ export default function SQLInlineEditor({
shadow="none"
bg="dark.6"
style={{
border: '1px solid var(--mantine-color-gray-7)',
border: `1px solid ${error ? 'var(--mantine-color-red-7)' : 'var(--mantine-color-gray-7)'}`,
display: 'flex',
alignItems: 'center',
minHeight: size === 'xs' ? 30 : 36,
@ -357,7 +359,7 @@ function SQLInlineEditorControlledComponent({
queryHistoryType,
...props
}: Omit<SQLInlineEditorProps, 'value' | 'onChange'> & UseControllerProps<any>) {
const { field } = useController(props);
const { field, fieldState } = useController(props);
// Guard against wrongly typed values
const value = field.value || props.defaultValue;
@ -375,6 +377,7 @@ function SQLInlineEditorControlledComponent({
onChange={field.onChange}
placeholder={placeholder}
value={stringValue}
error={fieldState.error?.message}
additionalSuggestions={additionalSuggestions}
queryHistoryType={queryHistoryType}
{...props}

256
packages/app/src/components/SourceForm.tsx
View file

@ -6,10 +6,13 @@ import {
UseFormSetValue,
UseFormWatch,
} from 'react-hook-form';
import { z } from 'zod';
import {
MetricsDataType,
SourceKind,
sourceSchemaWithout,
TSource,
TSourceUnion,
} from '@hyperdx/common-utils/dist/types';
import {
Anchor,
@ -18,17 +21,12 @@ import {
Divider,
Flex,
Group,
Menu,
Radio,
SegmentedControl,
Select,
Slider,
Stack,
Switch,
Text,
Tooltip,
} from '@mantine/core';
import { useDebouncedCallback } from '@mantine/hooks';
import { notifications } from '@mantine/notifications';
import { SourceSelectControlled } from '@/components/SourceSelect';
@ -117,15 +115,7 @@ function FormRow({
// OR traceModel.logModel = 'log_id_blah'
// custom always points towards the url param
export function LogTableModelForm({
control,
watch,
setValue,
}: {
control: Control<TSource>;
watch: UseFormWatch<TSource>;
setValue: UseFormSetValue<TSource>;
}) {
export function LogTableModelForm({ control, watch }: TableModelProps) {
const databaseName = watch(`from.databaseName`, DEFAULT_DATABASE);
const tableName = watch(`from.tableName`);
const connectionId = watch(`connection`);
@ -135,25 +125,6 @@ export function LogTableModelForm({
return (
<>
<Stack gap="sm">
<FormRow label={'Server Connection'}>
<ConnectionSelectControlled control={control} name={`connection`} />
</FormRow>
<FormRow label={'Database'}>
<DatabaseSelectControlled
control={control}
name={`from.databaseName`}
connectionId={connectionId}
/>
</FormRow>
<FormRow label={'Table'}>
<DBTableSelectControlled
database={databaseName}
control={control}
name={`from.tableName`}
connectionId={connectionId}
rules={{ required: 'Table is required' }}
/>
</FormRow>
<FormRow
label={'Timestamp Column'}
helpText="DateTime column or expression that is part of your table's primary key."
@ -379,40 +350,13 @@ export function LogTableModelForm({
);
}
export function TraceTableModelForm({
control,
watch,
setValue,
}: {
control: Control<TSource>;
watch: UseFormWatch<TSource>;
setValue: UseFormSetValue<TSource>;
}) {
export function TraceTableModelForm({ control, watch }: TableModelProps) {
const databaseName = watch(`from.databaseName`, DEFAULT_DATABASE);
const tableName = watch(`from.tableName`);
const connectionId = watch(`connection`);
return (
<Stack gap="sm">
<FormRow label={'Server Connection'}>
<ConnectionSelectControlled control={control} name={`connection`} />
</FormRow>
<FormRow label={'Database'}>
<DatabaseSelectControlled
connectionId={connectionId}
control={control}
name={`from.databaseName`}
/>
</FormRow>
<FormRow label={'Table'}>
<DBTableSelectControlled
connectionId={connectionId}
database={databaseName}
control={control}
name={`from.tableName`}
rules={{ required: 'Table is required' }}
/>
</FormRow>
<FormRow
label={'Timestamp Column'}
helpText="DateTime column or expression defines the start of the span"
@ -462,7 +406,7 @@ export function TraceTableModelForm({
<Controller
control={control}
name="durationPrecision"
render={({ field: { onChange, onBlur, value, ref } }) => (
render={({ field: { onChange, value } }) => (
<div style={{ width: '90%', marginBottom: 8 }}>
<Slider
color="green"
@ -656,43 +600,14 @@ export function TraceTableModelForm({
);
}
export function SessionTableModelForm({
control,
watch,
setValue,
}: {
control: Control<TSource>;
watch: UseFormWatch<TSource>;
setValue: UseFormSetValue<TSource>;
}) {
export function SessionTableModelForm({ control, watch }: TableModelProps) {
const databaseName = watch(`from.databaseName`, DEFAULT_DATABASE);
const tableName = watch(`from.tableName`);
const connectionId = watch(`connection`);
const [showOptionalFields, setShowOptionalFields] = useState(false);
return (
<>
<Stack gap="sm">
<FormRow label={'Server Connection'}>
<ConnectionSelectControlled control={control} name={`connection`} />
</FormRow>
<FormRow label={'Database'}>
<DatabaseSelectControlled
control={control}
name={`from.databaseName`}
connectionId={connectionId}
/>
</FormRow>
<FormRow label={'Table'}>
<DBTableSelectControlled
database={databaseName}
control={control}
name={`from.tableName`}
connectionId={connectionId}
rules={{ required: 'Table is required' }}
/>
</FormRow>
<FormRow
label={'Timestamp Column'}
helpText="DateTime column or expression that is part of your table's primary key."
@ -758,15 +673,17 @@ export function SessionTableModelForm({
);
}
interface TableModelProps {
control: Control<TSourceUnion>;
watch: UseFormWatch<TSourceUnion>;
setValue: UseFormSetValue<TSourceUnion>;
}
export function MetricTableModelForm({
control,
watch,
setValue,
}: {
control: Control<TSource>;
watch: UseFormWatch<TSource>;
setValue: UseFormSetValue<TSource>;
}) {
}: TableModelProps) {
const databaseName = watch(`from.databaseName`, DEFAULT_DATABASE);
const connectionId = watch(`connection`);
@ -780,7 +697,11 @@ export function MetricTableModelForm({
const [prefix, suffix] = name.split('.');
if (prefix === 'metricTables') {
const tableName =
value?.metricTables?.[suffix as keyof typeof value.metricTables];
value.kind === SourceKind.Metric
? value?.metricTables?.[
suffix as keyof typeof value.metricTables
]
: '';
const metricType = suffix as MetricsDataType;
const isValid = await isValidMetricTable({
databaseName,
@ -811,16 +732,6 @@ export function MetricTableModelForm({
return (
<>
<Stack gap="sm">
<FormRow label={'Server Connection'}>
<ConnectionSelectControlled control={control} name={`connection`} />
</FormRow>
<FormRow label={'Database'}>
<DatabaseSelectControlled
connectionId={connectionId}
control={control}
name={`from.databaseName`}
/>
</FormRow>
{Object.values(MetricsDataType).map(metricType => (
<FormRow
key={metricType.toLowerCase()}
@ -857,9 +768,9 @@ function TableModelForm({
setValue,
kind,
}: {
control: Control<TSource>;
watch: UseFormWatch<TSource>;
setValue: UseFormSetValue<TSource>;
control: Control<TSourceUnion>;
watch: UseFormWatch<TSourceUnion>;
setValue: UseFormSetValue<TSourceUnion>;
kind: SourceKind;
}) {
switch (kind) {
@ -916,37 +827,49 @@ export function TableSourceForm({
const { data: source } = useSource({ id: sourceId });
const { data: connections } = useConnections();
const { watch, control, setValue, handleSubmit, resetField, formState } =
useForm<TSource>({
defaultValues: {
kind: SourceKind.Log,
name: defaultName,
connection: connections?.[0]?.id,
from: {
databaseName: 'default',
tableName: '',
},
const {
watch,
control,
setValue,
formState,
handleSubmit,
resetField,
setError,
clearErrors,
} = useForm<TSourceUnion>({
defaultValues: {
kind: SourceKind.Log,
name: defaultName,
connection: connections?.[0]?.id,
from: {
databaseName: 'default',
tableName: '',
},
values: source,
resetOptions: {
keepDirtyValues: true,
keepErrors: true,
},
});
},
// TODO: HDX-1768 remove type assertion
values: source as TSourceUnion,
resetOptions: {
keepDirtyValues: true,
keepErrors: true,
},
});
useEffect(() => {
const { unsubscribe } = watch(async (value, { name, type }) => {
const { unsubscribe } = watch(async (_value, { name, type }) => {
try {
// TODO: HDX-1768 get rid of this type assertion
const value = _value as TSourceUnion;
if (
value.connection != null &&
value.from?.databaseName != null &&
value.from.tableName != null &&
(value.kind === SourceKind.Metric || value.from.tableName != null) &&
name === 'from.tableName' &&
type === 'change'
) {
const config = await inferTableSourceConfig({
databaseName: value.from.databaseName,
tableName: value.from.tableName,
tableName:
value.kind !== SourceKind.Metric ? value.from.tableName : '',
connectionId: value.connection,
});
if (Object.keys(config).length > 0) {
@ -983,10 +906,42 @@ export function TableSourceForm({
const updateSource = useUpdateSource();
const deleteSource = useDeleteSource();
const sourceFormSchema = sourceSchemaWithout({ id: true });
const handleError = (error: z.ZodError<TSourceUnion>) => {
const errors = error.errors;
for (const err of errors) {
const errorPath: string = err.path.join('.');
// TODO: HDX-1768 get rid of this type assertion if possible
setError(errorPath as any, { ...err });
}
notifications.show({
color: 'red',
message: (
<Stack>
<Text size="sm">
<b>Failed to create source</b>
</Text>
{errors.map((err, i) => (
<Text key={i} size="sm">
✖ {err.message}
</Text>
))}
</Stack>
),
});
};
const _onCreate = useCallback(() => {
clearErrors();
handleSubmit(data => {
const parseResult = sourceFormSchema.safeParse(data);
if (parseResult.error) {
handleError(parseResult.error);
return;
}
createSource.mutate(
{ source: data },
// TODO: HDX-1768 get rid of this type assertion
{ source: data as TSource },
{
onSuccess: data => {
onCreate?.(data);
@ -995,21 +950,28 @@ export function TableSourceForm({
message: 'Source created',
});
},
onError: () => {
onError: error => {
notifications.show({
color: 'red',
message: 'Failed to create source',
message: `Failed to create source - ${error.message}`,
});
},
},
);
})();
}, [handleSubmit, createSource, onCreate]);
}, [handleSubmit, createSource, onCreate, kind, formState]);
const _onSave = useCallback(() => {
clearErrors();
handleSubmit(data => {
const parseResult = sourceFormSchema.safeParse(data);
if (parseResult.error) {
handleError(parseResult.error);
return;
}
updateSource.mutate(
{ source: data },
// TODO: HDX-1768 get rid of this type assertion
{ source: data as TSource },
{
onSuccess: () => {
onSave?.();
@ -1029,6 +991,9 @@ export function TableSourceForm({
})();
}, [handleSubmit, updateSource, onSave]);
const databaseName = watch(`from.databaseName`, DEFAULT_DATABASE);
const connectionId = watch(`connection`);
return (
<div
style={
@ -1110,6 +1075,27 @@ export function TableSourceForm({
)}
/>
</FormRow>
<FormRow label={'Server Connection'}>
<ConnectionSelectControlled control={control} name={`connection`} />
</FormRow>
<FormRow label={'Database'}>
<DatabaseSelectControlled
control={control}
name={`from.databaseName`}
connectionId={connectionId}
/>
</FormRow>
{kind !== SourceKind.Metric && (
<FormRow label={'Table'}>
<DBTableSelectControlled
database={databaseName}
control={control}
name={`from.tableName`}
connectionId={connectionId}
rules={{ required: 'Table is required' }}
/>
</FormRow>
)}
</Stack>
<TableModelForm
control={control}

2
packages/app/src/source.ts
View file

@ -1,3 +1,5 @@
// TODO: HDX-1768 Change TSource here to TSourceUnion and adjust as needed. Then, go to
// SourceForm.tsx and remove type assertions for TSource and TSourceUnion
import pick from 'lodash/pick';
import objectHash from 'object-hash';
import store from 'store2';

239
packages/common-utils/src/types.ts
View file

@ -24,15 +24,20 @@ export enum DisplayType {
export type KeyValue<Key = string, Value = string> = { key: Key; value: Value };
export const MetricTableSchema = z.object(
Object.values(MetricsDataType).reduce(
(acc, key) => ({
...acc,
[key]: z.string(),
}),
{} as Record<MetricsDataType, z.ZodString>,
),
);
export const MetricTableSchema = z
.object(
Object.values(MetricsDataType).reduce(
(acc, key) => ({
...acc,
[key]: z.string().optional(),
}),
{} as Record<MetricsDataType, z.ZodString>,
),
)
.refine(
tables => Object.values(tables).some(table => table && table.length > 0),
{ message: 'At least one metric table must be specified' },
);
export type MetricTable = z.infer<typeof MetricTableSchema>;
@ -481,54 +486,176 @@ export enum SourceKind {
Metric = 'metric',
}
export const SourceSchema = z.object({
from: z.object({
databaseName: z.string(),
tableName: z.string(),
}),
timestampValueExpression: z.string(),
connection: z.string(),
// --------------------------
// TABLE SOURCE FORM VALIDATION
// --------------------------
// Common
kind: z.nativeEnum(SourceKind),
// Base schema with fields common to all source types
const SourceBaseSchema = z.object({
id: z.string(),
name: z.string(),
displayedTimestampValueExpression: z.string().optional(),
implicitColumnExpression: z.string().optional(),
serviceNameExpression: z.string().optional(),
bodyExpression: z.string().optional(),
tableFilterExpression: z.string().optional(),
eventAttributesExpression: z.string().optional(),
resourceAttributesExpression: z.string().optional(),
defaultTableSelectExpression: z.string().optional(),
// Logs
uniqueRowIdExpression: z.string().optional(),
severityTextExpression: z.string().optional(),
traceSourceId: z.string().optional(),
// Traces & Logs
traceIdExpression: z.string().optional(),
spanIdExpression: z.string().optional(),
// Sessions
sessionSourceId: z.string().optional(),
// Traces
durationExpression: z.string().optional(),
durationPrecision: z.number().min(0).max(9).optional(),
parentSpanIdExpression: z.string().optional(),
spanNameExpression: z.string().optional(),
spanEventsValueExpression: z.string().optional(),
spanKindExpression: z.string().optional(),
statusCodeExpression: z.string().optional(),
statusMessageExpression: z.string().optional(),
logSourceId: z.string().optional().nullable(),
// OTEL Metrics
metricTables: MetricTableSchema.optional(),
metricSourceId: z.string().optional(),
name: z.string().min(1, 'Name is required'),
kind: z.nativeEnum(SourceKind),
connection: z.string().min(1, 'Server Connection is required'),
from: z.object({
databaseName: z.string().min(1, 'Database is required'),
tableName: z.string().min(1, 'Table is required'),
}),
timestampValueExpression: z.string().min(1, 'Timestamp Column is required'),
});
export type TSource = z.infer<typeof SourceSchema>;
// Log source form schema
const LogSourceAugmentation = {
kind: z.literal(SourceKind.Log),
defaultTableSelectExpression: z.string({
message: 'Default Table Select Expression is required',
}),
// Optional fields for logs
serviceNameExpression: z.string().optional(),
severityTextExpression: z.string().optional(),
bodyExpression: z.string().optional(),
eventAttributesExpression: z.string().optional(),
resourceAttributesExpression: z.string().optional(),
displayedTimestampValueExpression: z.string().optional(),
metricSourceId: z.string().optional(),
traceSourceId: z.string().optional(),
traceIdExpression: z.string().optional(),
spanIdExpression: z.string().optional(),
implicitColumnExpression: z.string().optional(),
uniqueRowIdExpression: z.string().optional(),
tableFilterExpression: z.string().optional(),
};
// Trace source form schema
const TraceSourceAugmentation = {
kind: z.literal(SourceKind.Trace),
defaultTableSelectExpression: z.string().optional(),
// Required fields for traces
durationExpression: z.string().min(1, 'Duration Expression is required'),
durationPrecision: z.number().min(0).max(9).default(3),
traceIdExpression: z.string().min(1, 'Trace ID Expression is required'),
spanIdExpression: z.string().min(1, 'Span ID Expression is required'),
parentSpanIdExpression: z
.string()
.min(1, 'Parent span ID expression is required'),
spanNameExpression: z.string().min(1, 'Span Name Expression is required'),
spanKindExpression: z.string().min(1, 'Span Kind Expression is required'),
// Optional fields for traces
logSourceId: z.string().optional().nullable(),
sessionSourceId: z.string().optional(),
metricSourceId: z.string().optional(),
statusCodeExpression: z.string().optional(),
statusMessageExpression: z.string().optional(),
serviceNameExpression: z.string().optional(),
resourceAttributesExpression: z.string().optional(),
eventAttributesExpression: z.string().optional(),
spanEventsValueExpression: z.string().optional(),
implicitColumnExpression: z.string().optional(),
};
// Session source form schema
const SessionSourceAugmentation = {
kind: z.literal(SourceKind.Session),
// Required fields for sessions
eventAttributesExpression: z
.string()
.min(1, 'Log Attributes Expression is required'),
resourceAttributesExpression: z
.string()
.min(1, 'Resource Attributes Expression is required'),
traceSourceId: z
.string({ message: 'Correlated Trace Source is required' })
.min(1, 'Correlated Trace Source is required'),
// Optional fields for sessions
implicitColumnExpression: z.string().optional(),
};
// Metric source form schema
const MetricSourceAugmentation = {
kind: z.literal(SourceKind.Metric),
// override from SourceBaseSchema
from: z.object({
databaseName: z.string().min(1, 'Database is required'),
tableName: z.string(),
}),
// Metric tables - at least one should be provided
metricTables: MetricTableSchema,
resourceAttributesExpression: z
.string()
.min(1, 'Resource Attributes is required'),
// Optional fields for metrics
logSourceId: z.string().optional(),
};
// Union of all source form schemas for validation
export const SourceSchema = z.discriminatedUnion('kind', [
SourceBaseSchema.extend(LogSourceAugmentation),
SourceBaseSchema.extend(TraceSourceAugmentation),
SourceBaseSchema.extend(SessionSourceAugmentation),
SourceBaseSchema.extend(MetricSourceAugmentation),
]);
export type TSourceUnion = z.infer<typeof SourceSchema>;
// This function exists to perform schema validation with omission of a certain
// value. It is not possible to do on the discriminatedUnion directly
export function sourceSchemaWithout(
omissions: { [k in keyof z.infer<typeof SourceBaseSchema>]?: true } = {},
) {
// TODO: Make these types work better if possible
return z.discriminatedUnion('kind', [
SourceBaseSchema.omit(omissions).extend(LogSourceAugmentation),
SourceBaseSchema.omit(omissions).extend(TraceSourceAugmentation),
SourceBaseSchema.omit(omissions).extend(SessionSourceAugmentation),
SourceBaseSchema.omit(omissions).extend(MetricSourceAugmentation),
]);
}
// Helper types for better union flattening
type AllKeys<T> = T extends any ? keyof T : never;
// This is Claude Opus's explanation of this type magic to extract the required
// parameters:
//
// 1. [K in keyof T]-?:
// Maps over all keys in T. The -? removes the optional modifier, making all
// properties required in this mapped type
// 2. {} extends Pick<T, K> ? never : K
// Pick<T, K> creates a type with just property K from T.
// {} extends Pick<T, K> checks if an empty object can satisfy the picked property.
// If the property is optional, {} can extend it (returns never)
// If the property is required, {} cannot extend it (returns K)
// 3. [keyof T]
// Indexes into the mapped type to get the union of all non-never values
type NonOptionalKeysPresentInEveryUnionBranch<T> = {
[K in keyof T]-?: {} extends Pick<T, K> ? never : K;
}[keyof T];
// Helper to check if a key is required in ALL branches of the union
type RequiredInAllBranches<T, K extends AllKeys<T>> = T extends any
? K extends NonOptionalKeysPresentInEveryUnionBranch<T>
? true
: false
: never;
// This type gathers the Required Keys across the discriminated union TSourceUnion
// and keeps them as required in a non-unionized type, and also gathers all possible
// optional keys from the union branches and brings them into one unified flattened type.
// This is done to maintain compatibility with the legacy zod schema.
type FlattenUnion<T> = {
// If a key is required in all branches of a union, make it a required key
[K in AllKeys<T> as RequiredInAllBranches<T, K> extends true
? K
: never]: T extends infer U ? (K extends keyof U ? U[K] : never) : never;
} & {
// If a key is not required in all branches of a union, make it an optional
// key and join the possible types
[K in AllKeys<T> as RequiredInAllBranches<T, K> extends true
? never
: K]?: T extends infer U ? (K extends keyof U ? U[K] : never) : never;
};
export type TSource = FlattenUnion<z.infer<typeof SourceSchema>>;