refactor: unify source types and remove ambiguity by knudtty · Pull Request #1194 · hyperdxio/hyperdx

knudtty · 2025-09-22T20:21:31Z

Closes HDX-2468

changeset-bot · 2025-09-22T20:21:35Z

⚠️ No Changeset found

Latest commit: feabfd3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel · 2025-09-22T20:21:36Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
hyperdx-v2-oss-app	Ready	Preview	Comment	Oct 9, 2025 9:37pm

github-actions · 2025-09-22T21:22:32Z

E2E Test Results

✅ All tests passed • 25 passed • 3 skipped • 226s

Status	Count
✅ Passed	25
❌ Failed	0
⚠️ Flaky	0
⏭️ Skipped	3

View full report →

knudtty · 2025-09-22T23:28:47Z

 export function createSource(team: string, source: Omit<ISource, 'id'>) {
-  return Source.create({ ...source, team });
+  switch (source.kind) {
+    case SourceKind.Log:
+      return LogSource.create({ ...source, team });
+    case SourceKind.Trace:
+      return TraceSource.create({ ...source, team });
+    case SourceKind.Metric:
+      return MetricSource.create({ ...source, team });
+    case SourceKind.Session:
+      return SessionSource.create({ ...source, team });
+  }


Mongoose will strip out any unnecessary fields, so create source must use the new broken out discriminated models

knudtty · 2025-09-22T23:29:01Z

+  switch (source.kind) {
+    case SourceKind.Log:
+      return LogSource.findOneAndUpdate({ _id: sourceId, team }, source, {
+        new: true,
+      });
+    case SourceKind.Trace:
+      return TraceSource.findOneAndUpdate({ _id: sourceId, team }, source, {
+        new: true,
+      });
+    case SourceKind.Metric:
+      return MetricSource.findOneAndUpdate({ _id: sourceId, team }, source, {
+        new: true,
+      });
+    case SourceKind.Session:
+      return SessionSource.findOneAndUpdate({ _id: sourceId, team }, source, {
+        new: true,
+      });
+  }


Same, using discriminated models

knudtty · 2025-09-22T23:30:01Z

-      durationPrecision: Number,
-      parentSpanIdExpression: String,
-      spanNameExpression: String,
+export const LogSource = Source.discriminator<


Broke out the mongoose models into the base model (Source) and a discriminated model for each type

knudtty · 2025-09-22T23:30:36Z

@@ -1,10 +1,10 @@
-import { SourceKind, TSourceUnion } from '@hyperdx/common-utils/dist/types';
+import { SourceKind, TSource } from '@hyperdx/common-utils/dist/types';


TSourceUnion is renamed TSource

knudtty · 2025-09-22T23:34:02Z

+export function useSource(params: {
+  id?: string | null;
+  connection?: string | null;
+}): UseQueryResult<TSource | undefined>;
+export function useSource<K extends TSource['kind']>(params: {
+  id?: string | null;
+  connection?: string | null;
+  kind: K;
+}): UseQueryResult<Extract<TSource, { kind: K }> | undefined>;


The type signature without specifying kind returns the TSource type, but if you specify kind then the type signature understands the returned source is a particular kind. Super useful if you always know you'll be using a specific kind of source

knudtty · 2025-09-22T23:35:04Z

-    mutationFn: async ({ source }: { source: Omit<TSource, 'id'> }) => {
+    mutationFn: async ({ source }: { source: TSourceNoId }) => {


TS's built in Omit does not work well with discriminated unions, so it's better to construct with zod and type assert.

wrn14897 · 2025-09-23T05:08:15Z

The refactoring makes me a bit nervous, as the changes affect the core of the entire app and I’m not very confident in our test coverage in this area. To minimize the risk of breaking existing behavior, I’d suggest migrating to the new sources model gradually, one source type at a time. If it’s not that easy, I’d recommend adding more unit tests for different sources.

Overall, I really like these changes, which will help us catch more source type–related bugs :)

wrn14897 · 2025-09-23T05:09:13Z

+      return LogSource.create({ ...source, team });
+    case SourceKind.Trace:
+      return TraceSource.create({ ...source, team });
+    case SourceKind.Metric:
+      return MetricSource.create({ ...source, team });
+    case SourceKind.Session:
+      return SessionSource.create({ ...source, team });


we should also handle default case

wrn14897 · 2025-09-23T05:11:17Z

+export const SessionSource = Source.discriminator<
+  Extract<TSource, { kind: SourceKind.Session }>
+>(
+  SourceKind.Session,
+  new mongoose.Schema<Extract<TSource, { kind: SourceKind.Session }>>({
+    traceSourceId: String,
+  }),
 );


Will those non-public fields (e.g., ResourceAttributes) be inserted or updated?

Let's talk about the requirements for Sessions, because ResourceAttributes isn't currently listed in the schema

Hmm you are right. Sorry, I was thinking about the different thing. The mapping should only require a few fields

wrn14897 · 2025-09-23T05:12:38Z

+        case SourceKind.Log:
+          return s.toJSON({ getters: true });
+        case SourceKind.Trace:
+          return s.toJSON({ getters: true });
+        case SourceKind.Metric:
+          return s.toJSON({ getters: true });
+        case SourceKind.Session:
+          return s.toJSON({ getters: true });


nit: this can be

case SourceKind.Log: case SourceKind.Trace: case SourceKind.Metric: case SourceKind.Session: return s.toJSON({ getters: true });

also we need to handle default case

For some reason Typescript gets confused about the union so that does not work, it has to be broken out like this. I'll add an error for the default case

wrn14897 · 2025-09-23T05:21:17Z

+  const { data: logSource, isLoading: isLogSourceLoading } = useSource({
+    id: source,
+    kind: SourceKind.Log,
+  });
+  const { data: traceSource, isLoading: isTraceSourceLoading } = useSource({
    id: source,
+    kind: SourceKind.Trace,
  });


Is there any chance we could avoid duplicate requests here? I see the point, but the source ID should already give us the right source

Under the hood they all are using the same query key to fetch all the sources and then just running find over the elements of the array. They are using the same query key, so only one request should be generated

knudtty · 2025-09-23T13:46:50Z

To minimize the risk of breaking existing behavior, I’d suggest migrating to the new sources model gradually, one source type at a time. If it’s not that easy, I’d recommend adding more unit tests for different sources.

Unfortunately it is not easy. More unit tests makes sense, I'll seek guidance for which ones specifically we are interested in

teeohhem · 2026-01-15T15:03:19Z

Closed due to staleness. Please re-open with updates if needed.

unify source types and remove ambiguity

23d5fde

vercel Bot deployed to Preview September 22, 2025 20:24 View deployment

Merge branch 'main' into aaron/source-tech-debt

5b3ed93

vercel Bot deployed to Preview September 22, 2025 21:22 View deployment

fix linting

e25a223

vercel Bot deployed to Preview September 22, 2025 21:44 View deployment

fix: sources not searching properly

0d65ec6

vercel Bot deployed to Preview September 22, 2025 23:21 View deployment

knudtty requested a review from wrn14897 September 22, 2025 23:24

knudtty commented Sep 22, 2025

View reviewed changes

wrn14897 reviewed Sep 23, 2025

View reviewed changes

add defaults to source switches

2108f6c

vercel Bot deployed to Preview September 23, 2025 16:02 View deployment

testing: add robust source testing

5403a87

vercel Bot deployed to Preview October 9, 2025 17:56 View deployment

Merge branch 'main' into aaron/source-tech-debt

e6330c2

vercel Bot deployed to Preview October 9, 2025 21:02 View deployment

fix source lint for ai

feabfd3

vercel Bot deployed to Preview October 9, 2025 21:37 View deployment

teeohhem closed this Jan 15, 2026

		@@ -1,10 +1,10 @@
		import { SourceKind, TSourceUnion } from '@hyperdx/common-utils/dist/types';
		import { SourceKind, TSource } from '@hyperdx/common-utils/dist/types';

		mutationFn: async ({ source }: { source: Omit<TSource, 'id'> }) => {
		mutationFn: async ({ source }: { source: TSourceNoId }) => {

Conversation

knudtty commented Sep 22, 2025

Uh oh!

changeset-bot Bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

vercel Bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

E2E Test Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wrn14897 commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knudtty commented Sep 23, 2025

Uh oh!

teeohhem commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

changeset-bot Bot commented Sep 22, 2025 •

edited

Loading

vercel Bot commented Sep 22, 2025 •

edited

Loading

github-actions Bot commented Sep 22, 2025 •

edited

Loading

wrn14897 commented Sep 23, 2025 •

edited

Loading