-
Notifications
You must be signed in to change notification settings - Fork 283
feat: Auto-link correlated sources bidirectionally #990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
372f039
c75b37a
9e7373d
41d9cb5
cf844fe
1aeffc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@hyperdx/app": minor | ||
--- | ||
|
||
Correlated source field links are bidirectional by default and no link exists. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,7 @@ import { | |
useCreateSource, | ||
useDeleteSource, | ||
useSource, | ||
useSources, | ||
useUpdateSource, | ||
} from '@/source'; | ||
|
||
|
@@ -56,6 +57,39 @@ const OTEL_CLICKHOUSE_EXPRESSIONS = { | |
resourceAttributesExpression: 'ResourceAttributes', | ||
}; | ||
|
||
const CORRELATION_FIELD_MAP: Record< | ||
SourceKind, | ||
Record<string, { targetKind: SourceKind; targetField: keyof TSource }[]> | ||
> = { | ||
[SourceKind.Log]: { | ||
metricSourceId: [ | ||
{ targetKind: SourceKind.Metric, targetField: 'logSourceId' }, | ||
], | ||
traceSourceId: [ | ||
{ targetKind: SourceKind.Trace, targetField: 'logSourceId' }, | ||
], | ||
}, | ||
[SourceKind.Trace]: { | ||
logSourceId: [{ targetKind: SourceKind.Log, targetField: 'traceSourceId' }], | ||
sessionSourceId: [ | ||
{ targetKind: SourceKind.Session, targetField: 'traceSourceId' }, | ||
], | ||
metricSourceId: [ | ||
{ targetKind: SourceKind.Metric, targetField: 'logSourceId' }, | ||
], | ||
}, | ||
[SourceKind.Session]: { | ||
traceSourceId: [ | ||
{ targetKind: SourceKind.Trace, targetField: 'sessionSourceId' }, | ||
], | ||
}, | ||
[SourceKind.Metric]: { | ||
logSourceId: [ | ||
{ targetKind: SourceKind.Log, targetField: 'metricSourceId' }, | ||
], | ||
}, | ||
}; | ||
|
||
function FormRow({ | ||
label, | ||
children, | ||
|
@@ -906,6 +940,62 @@ export function TableSourceForm({ | |
const updateSource = useUpdateSource(); | ||
const deleteSource = useDeleteSource(); | ||
|
||
// Bidirectional source linking | ||
const { data: sources } = useSources(); | ||
const currentSourceId = watch('id'); | ||
|
||
useEffect(() => { | ||
const { unsubscribe } = watch(async (_value, { name, type }) => { | ||
const value = _value as TSourceUnion; | ||
if (!currentSourceId || !sources || type !== 'change') return; | ||
|
||
const correlationFields = CORRELATION_FIELD_MAP[kind]; | ||
if (!correlationFields || !name || !(name in correlationFields)) return; | ||
|
||
const fieldName = name as keyof TSourceUnion; | ||
const newTargetSourceId = value[fieldName] as string | undefined; | ||
const targetConfigs = correlationFields[fieldName]; | ||
|
||
for (const { targetKind, targetField } of targetConfigs) { | ||
// Find the previously linked source if any | ||
const previouslyLinkedSource = sources.find( | ||
s => s.kind === targetKind && s[targetField] === currentSourceId, | ||
); | ||
|
||
// If there was a previously linked source and it's different from the new one, unlink it | ||
if ( | ||
previouslyLinkedSource && | ||
previouslyLinkedSource.id !== newTargetSourceId | ||
) { | ||
await updateSource.mutateAsync({ | ||
source: { | ||
...previouslyLinkedSource, | ||
[targetField]: undefined, | ||
} as TSource, | ||
}); | ||
} | ||
|
||
// If a new source is selected, link it back | ||
if (newTargetSourceId) { | ||
const targetSource = sources.find(s => s.id === newTargetSourceId); | ||
if (targetSource && targetSource.kind === targetKind) { | ||
// Only update if the target field is empty to avoid overwriting existing correlations | ||
if (!targetSource[targetField]) { | ||
await updateSource.mutateAsync({ | ||
source: { | ||
...targetSource, | ||
[targetField]: currentSourceId, | ||
} as TSource, | ||
}); | ||
} | ||
} | ||
} | ||
} | ||
}); | ||
|
||
return () => unsubscribe(); | ||
}, [watch, kind, currentSourceId, sources, updateSource]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just wondering if circular dependencies are possible here... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is code to only set the bidirectional link if the other side of the link doesn't have a value. That should prevent infinite update loops as well as allowing users to setup more complex correlation DAGs. Is that what you were getting at? |
||
|
||
const sourceFormSchema = sourceSchemaWithout({ id: true }); | ||
const handleError = (error: z.ZodError<TSourceUnion>) => { | ||
const errors = error.errors; | ||
|
@@ -933,18 +1023,47 @@ export function TableSourceForm({ | |
|
||
const _onCreate = useCallback(() => { | ||
clearErrors(); | ||
handleSubmit(data => { | ||
handleSubmit(async data => { | ||
const parseResult = sourceFormSchema.safeParse(data); | ||
if (parseResult.error) { | ||
handleError(parseResult.error); | ||
return; | ||
} | ||
|
||
createSource.mutate( | ||
// TODO: HDX-1768 get rid of this type assertion | ||
{ source: data as TSource }, | ||
{ | ||
onSuccess: data => { | ||
onCreate?.(data); | ||
onSuccess: async newSource => { | ||
// Handle bidirectional linking for new sources | ||
const correlationFields = CORRELATION_FIELD_MAP[newSource.kind]; | ||
if (correlationFields && sources) { | ||
for (const [fieldName, targetConfigs] of Object.entries( | ||
correlationFields, | ||
)) { | ||
const targetSourceId = (newSource as any)[fieldName]; | ||
if (targetSourceId) { | ||
for (const { targetKind, targetField } of targetConfigs) { | ||
const targetSource = sources.find( | ||
s => s.id === targetSourceId, | ||
); | ||
if (targetSource && targetSource.kind === targetKind) { | ||
// Only update if the target field is empty to avoid overwriting existing correlations | ||
if (!targetSource[targetField]) { | ||
await updateSource.mutateAsync({ | ||
source: { | ||
...targetSource, | ||
[targetField]: newSource.id, | ||
} as TSource, | ||
}); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
onCreate?.(newSource); | ||
notifications.show({ | ||
color: 'green', | ||
message: 'Source created', | ||
|
@@ -959,7 +1078,15 @@ export function TableSourceForm({ | |
}, | ||
); | ||
})(); | ||
}, [handleSubmit, createSource, onCreate, kind, formState]); | ||
}, [ | ||
handleSubmit, | ||
createSource, | ||
onCreate, | ||
kind, | ||
formState, | ||
sources, | ||
updateSource, | ||
]); | ||
|
||
const _onSave = useCallback(() => { | ||
clearErrors(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this particular
watch
signature is deprecated in favor ofsubscribe
https://react-hook-form.com/docs/useform/subscribeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's listed in the docs as deprecated and spelunking the react-hook-form releases this happened in v7.55.0 (commit 7f95b26). There is no
subscribe
handler available until that revision.We're currently using v7.43.8 so moving to the
subscribe
approach would require buying into 12 possible breaking version bumps. Do we want to tackle that upgrade effort in this PR or make it a separate effort?