Skip to content

Commit 1ed4fbf

Browse files
authored
fix: simplify alert provider load logic (#1220)
Dynamic module resolution causes issues in builds that use tools like esbuild; resulting paths are often not the same as they would in the source tree. Instead we can simplify the loading logic to use a defined object map of names to constructor functions. The mapping object should be small enough that merge conflicts with forks should be easy to resolve.
1 parent 270c6b4 commit 1ed4fbf

File tree

3 files changed

+13
-197
lines changed

3 files changed

+13
-197
lines changed

packages/api/src/tasks/__tests__/checkAlertsTask.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import { CheckAlertsTaskArgs } from '../types';
2020
jest.mock('@/tasks/providers', () => {
2121
return {
2222
...jest.requireActual('@/tasks/providers'),
23-
isValidProvider: jest.fn().mockReturnValue(true),
2423
loadProvider: jest.fn(),
2524
};
2625
});

packages/api/src/tasks/providers/__tests__/index.test.ts

Lines changed: 0 additions & 162 deletions
This file was deleted.

packages/api/src/tasks/providers/index.ts

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { ClickhouseClient } from '@hyperdx/common-utils/dist/clickhouse/node';
22
import { Tile } from '@hyperdx/common-utils/dist/types';
3+
import _ from 'lodash';
34

45
import { ObjectId } from '@/models';
56
import { IAlert } from '@/models/alert';
@@ -83,48 +84,26 @@ export interface AlertProvider {
8384
getClickHouseClient(connection: IConnection): Promise<ClickhouseClient>;
8485
}
8586

86-
export function isValidProvider(obj: any): obj is AlertProvider {
87-
return (
88-
obj != null &&
89-
typeof obj.init === 'function' &&
90-
typeof obj.asyncDispose === 'function' &&
91-
typeof obj.getAlertTasks === 'function' &&
92-
typeof obj.buildLogSearchLink === 'function' &&
93-
typeof obj.buildChartLink === 'function' &&
94-
typeof obj.updateAlertState === 'function' &&
95-
typeof obj.getWebhooks === 'function' &&
96-
typeof obj.getClickHouseClient === 'function'
97-
);
98-
}
87+
const ADDITIONAL_PROVIDERS: Record<string, () => AlertProvider> = {
88+
// Additional alert provider create functions should appear in this object
89+
// defined with a provider name.
90+
};
9991

10092
export async function loadProvider(
10193
providerName?: string | undefined | null,
10294
): Promise<AlertProvider> {
10395
if (providerName && providerName !== 'default') {
104-
try {
105-
const ProviderClass = (await import(`./${providerName}`)).default;
106-
if (typeof ProviderClass === 'function') {
107-
const providerInstance = new ProviderClass();
108-
if (isValidProvider(providerInstance)) {
109-
return providerInstance;
110-
} else {
111-
logger.warn({
112-
message: `"${providerName}" does not implement AlertProvider`,
113-
providerName,
114-
});
115-
}
116-
} else {
117-
logger.warn({
118-
message: `"${providerName}" does not export default constructor`,
96+
const providerFn = _.get(ADDITIONAL_PROVIDERS, providerName);
97+
if (providerFn) {
98+
try {
99+
return providerFn();
100+
} catch (err) {
101+
logger.error({
102+
message: `error creating instance of ${providerName} provider; using default`,
103+
cause: err,
119104
providerName,
120105
});
121106
}
122-
} catch (e) {
123-
logger.warn({
124-
message: `failed to load "${providerName}: ${e}"`,
125-
cause: e,
126-
providerName,
127-
});
128107
}
129108
}
130109

0 commit comments

Comments
 (0)