Skip to content

Commit 497f301

Browse files
committed
Nicer Slack flow. Improvement to evaluation scheduling
1 parent 1be6033 commit 497f301

File tree

9 files changed

+159
-25
lines changed

9 files changed

+159
-25
lines changed

apps/webapp/app/components/errors/ConfigureErrorAlerts.tsx

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,17 @@ export const ErrorAlertsFormSchema = z.object({
4242
}, z.string().url().array()),
4343
});
4444

45-
type ConfigureErrorAlertsProps = ErrorAlertChannelData;
45+
type ConfigureErrorAlertsProps = ErrorAlertChannelData & {
46+
connectToSlackHref?: string;
47+
};
4648

4749
export function ConfigureErrorAlerts({
4850
emails: existingEmails,
4951
webhooks: existingWebhooks,
5052
slackChannel: existingSlackChannel,
5153
slack,
5254
emailAlertsEnabled,
55+
connectToSlackHref,
5356
}: ConfigureErrorAlertsProps) {
5457
const fetcher = useFetcher();
5558
const location = useOptimisticLocation();
@@ -216,17 +219,43 @@ export function ConfigureErrorAlerts({
216219
/>
217220
</>
218221
) : slack.status === "NOT_CONFIGURED" ? (
219-
<Callout variant="info">
220-
Slack is not connected. Connect Slack from the{" "}
221-
<span className="font-medium text-text-bright">Alerts</span> page to enable
222-
Slack notifications.
223-
</Callout>
222+
connectToSlackHref ? (
223+
<LinkButton variant="tertiary/large" to={connectToSlackHref} fullWidth>
224+
<span className="flex items-center gap-2 text-text-bright">
225+
<SlackIcon className="size-5" /> Connect to Slack
226+
</span>
227+
</LinkButton>
228+
) : (
229+
<Callout variant="info">
230+
Slack is not connected. Connect Slack from the{" "}
231+
<span className="font-medium text-text-bright">Alerts</span> page to enable
232+
Slack notifications.
233+
</Callout>
234+
)
224235
) : slack.status === "TOKEN_REVOKED" || slack.status === "TOKEN_EXPIRED" ? (
225-
<Callout variant="info">
226-
The Slack integration in your workspace has been revoked or expired. Please
227-
re-connect from the <span className="font-medium text-text-bright">Alerts</span>{" "}
228-
page.
229-
</Callout>
236+
connectToSlackHref ? (
237+
<div className="flex flex-col gap-4">
238+
<Callout variant="info">
239+
The Slack integration in your workspace has been revoked or has expired.
240+
Please re-connect your Slack workspace.
241+
</Callout>
242+
<LinkButton
243+
variant="tertiary/large"
244+
to={`${connectToSlackHref}?reinstall=true`}
245+
fullWidth
246+
>
247+
<span className="flex items-center gap-2 text-text-bright">
248+
<SlackIcon className="size-5" /> Connect to Slack
249+
</span>
250+
</LinkButton>
251+
</div>
252+
) : (
253+
<Callout variant="info">
254+
The Slack integration in your workspace has been revoked or expired. Please
255+
re-connect from the{" "}
256+
<span className="font-medium text-text-bright">Alerts</span> page.
257+
</Callout>
258+
)
230259
) : slack.status === "FAILED_FETCHING_CHANNELS" ? (
231260
<Callout variant="warning">
232261
Failed loading channels from Slack. Please try again later.

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,11 +141,32 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
141141
return json({ ok: true });
142142
}
143143
case "ignore": {
144+
let occurrenceCountAtIgnoreTime: number | undefined;
145+
146+
if (submission.value.totalOccurrences) {
147+
const qb = clickhouseClient.errors.listQueryBuilder();
148+
qb.where("project_id = {projectId: String}", { projectId: project.id });
149+
qb.where("environment_id = {environmentId: String}", {
150+
environmentId: environment.id,
151+
});
152+
qb.where("error_fingerprint = {fingerprint: String}", { fingerprint });
153+
qb.where("task_identifier = {taskIdentifier: String}", {
154+
taskIdentifier: submission.value.taskIdentifier,
155+
});
156+
qb.groupBy("error_fingerprint, task_identifier");
157+
158+
const [err, results] = await qb.execute();
159+
if (!err && results && results.length > 0) {
160+
occurrenceCountAtIgnoreTime = results[0].occurrence_count;
161+
}
162+
}
163+
144164
await actions.ignoreError(identifier, {
145165
userId,
146166
duration: submission.value.duration,
147167
occurrenceRateThreshold: submission.value.occurrenceRate,
148168
totalOccurrencesThreshold: submission.value.totalOccurrences,
169+
occurrenceCountAtIgnoreTime,
149170
reason: submission.value.reason,
150171
});
151172
return json({ ok: true });
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { type LoaderFunctionArgs } from "@remix-run/server-runtime";
2+
import { prisma } from "~/db.server";
3+
import { redirectWithSuccessMessage } from "~/models/message.server";
4+
import { OrgIntegrationRepository } from "~/models/orgIntegration.server";
5+
import { findProjectBySlug } from "~/models/project.server";
6+
import { requireUserId } from "~/services/session.server";
7+
import {
8+
EnvironmentParamSchema,
9+
v3ErrorsPath,
10+
v3ErrorsConnectToSlackPath,
11+
} from "~/utils/pathBuilder";
12+
13+
export async function loader({ request, params }: LoaderFunctionArgs) {
14+
const userId = await requireUserId(request);
15+
const { organizationSlug, projectParam, envParam } = EnvironmentParamSchema.parse(params);
16+
17+
const url = new URL(request.url);
18+
const shouldReinstall = url.searchParams.get("reinstall") === "true";
19+
20+
const project = await findProjectBySlug(organizationSlug, projectParam, userId);
21+
22+
if (!project) {
23+
throw new Response("Project not found", { status: 404 });
24+
}
25+
26+
const integration = await prisma.organizationIntegration.findFirst({
27+
where: {
28+
service: "SLACK",
29+
organizationId: project.organizationId,
30+
},
31+
});
32+
33+
if (integration && !shouldReinstall) {
34+
return redirectWithSuccessMessage(
35+
`${v3ErrorsPath({ slug: organizationSlug }, project, { slug: envParam })}?alerts`,
36+
request,
37+
"Successfully connected your Slack workspace"
38+
);
39+
}
40+
41+
return await OrgIntegrationRepository.redirectToAuthService(
42+
"SLACK",
43+
project.organizationId,
44+
request,
45+
v3ErrorsConnectToSlackPath({ slug: organizationSlug }, project, { slug: envParam })
46+
);
47+
}

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors/route.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { ErrorAlertChannelPresenter } from "~/presenters/v3/ErrorAlertChannelPre
1717
import { findProjectBySlug } from "~/models/project.server";
1818
import { findEnvironmentBySlug } from "~/models/runtimeEnvironment.server";
1919
import { requireUserId } from "~/services/session.server";
20-
import { EnvironmentParamSchema } from "~/utils/pathBuilder";
20+
import { EnvironmentParamSchema, v3ErrorsConnectToSlackPath } from "~/utils/pathBuilder";
2121
import {
2222
type CreateAlertChannelOptions,
2323
CreateAlertChannelService,
@@ -40,11 +40,16 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
4040
const presenter = new ErrorAlertChannelPresenter();
4141
const alertData = await presenter.call(project.id, environment.type);
4242

43+
const connectToSlackHref = v3ErrorsConnectToSlackPath({ slug: organizationSlug }, project, {
44+
slug: envParam,
45+
});
46+
4347
return typedjson({
4448
alertData,
4549
projectRef: project.externalRef,
4650
projectId: project.id,
4751
environmentType: environment.type,
52+
connectToSlackHref,
4853
});
4954
};
5055

@@ -146,7 +151,7 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
146151
};
147152

148153
export default function Page() {
149-
const { alertData } = useTypedLoaderData<typeof loader>();
154+
const { alertData, connectToSlackHref } = useTypedLoaderData<typeof loader>();
150155
const [searchParams] = useSearchParams();
151156
const showAlerts = searchParams.has("alerts");
152157

@@ -160,7 +165,7 @@ export default function Page() {
160165
<>
161166
<ResizableHandle id="errors-alerts-handle" />
162167
<ResizablePanel id="errors-alerts" min="320px" default="420px" max="560px">
163-
<ConfigureErrorAlerts {...alertData} />
168+
<ConfigureErrorAlerts {...alertData} connectToSlackHref={connectToSlackHref} />
164169
</ResizablePanel>
165170
</>
166171
)}

apps/webapp/app/utils/pathBuilder.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,14 @@ export function v3ErrorsPath(
540540
return `${v3EnvironmentPath(organization, project, environment)}/errors`;
541541
}
542542

543+
export function v3ErrorsConnectToSlackPath(
544+
organization: OrgForPath,
545+
project: ProjectForPath,
546+
environment: EnvironmentForPath
547+
) {
548+
return `${v3ErrorsPath(organization, project, environment)}/connect-to-slack`;
549+
}
550+
543551
export function v3ErrorPath(
544552
organization: OrgForPath,
545553
project: ProjectForPath,

apps/webapp/app/v3/services/alerts/errorAlertEvaluator.server.ts

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ const DEFAULT_INTERVAL_MS = 300_000;
4444
*/
4545
export class ErrorAlertEvaluator {
4646
constructor(
47-
protected readonly _prisma: PrismaClientOrTransaction = $replica,
48-
protected readonly _replica: PrismaClientOrTransaction = prisma,
47+
protected readonly _prisma: PrismaClientOrTransaction = prisma,
48+
protected readonly _replica: PrismaClientOrTransaction = $replica,
4949
protected readonly _clickhouse: ClickHouse = clickhouseClient
5050
) {}
5151

@@ -61,6 +61,18 @@ export class ErrorAlertEvaluator {
6161
}
6262

6363
const minIntervalMs = this.computeMinInterval(channels);
64+
const windowMs = nextScheduledAt - scheduledAt;
65+
66+
if (windowMs > minIntervalMs * 2) {
67+
logger.info("[ErrorAlertEvaluator] Large evaluation window (gap detected)", {
68+
projectId,
69+
scheduledAt,
70+
nextScheduledAt,
71+
windowMs,
72+
minIntervalMs,
73+
});
74+
}
75+
6476
const allEnvTypes = this.collectEnvironmentTypes(channels);
6577
const environments = await this.resolveEnvironments(projectId, allEnvTypes);
6678

@@ -97,7 +109,8 @@ export class ErrorAlertEvaluator {
97109

98110
const classification = this.classifyError(error, state, firstSeenMs, scheduledAt, {
99111
occurrencesSince: occurrenceMap.get(key) ?? 0,
100-
windowMs: nextScheduledAt - scheduledAt,
112+
windowMs,
113+
totalOccurrenceCount: error.occurrence_count,
101114
});
102115

103116
if (classification) {
@@ -160,7 +173,7 @@ export class ErrorAlertEvaluator {
160173
state: ErrorGroupState | undefined,
161174
firstSeenMs: number,
162175
scheduledAt: number,
163-
thresholdContext: { occurrencesSince: number; windowMs: number }
176+
thresholdContext: { occurrencesSince: number; windowMs: number; totalOccurrenceCount: number }
164177
): ErrorClassification | null {
165178
if (!state) {
166179
return firstSeenMs > scheduledAt ? "new_issue" : null;
@@ -186,7 +199,7 @@ export class ErrorAlertEvaluator {
186199

187200
private isIgnoreBreached(
188201
state: ErrorGroupState,
189-
context: { occurrencesSince: number; windowMs: number }
202+
context: { occurrencesSince: number; windowMs: number; totalOccurrenceCount: number }
190203
): boolean {
191204
if (state.ignoredUntil && state.ignoredUntil.getTime() <= Date.now()) {
192205
return true;
@@ -204,11 +217,12 @@ export class ErrorAlertEvaluator {
204217
}
205218

206219
if (
207-
state.ignoredUntilTotalOccurrences !== null &&
208-
state.ignoredUntilTotalOccurrences !== undefined &&
209-
state.ignoredAt
220+
state.ignoredUntilTotalOccurrences != null &&
221+
state.ignoredAtOccurrenceCount != null
210222
) {
211-
if (context.occurrencesSince >= state.ignoredUntilTotalOccurrences) {
223+
const occurrencesSinceIgnored =
224+
context.totalOccurrenceCount - Number(state.ignoredAtOccurrenceCount);
225+
if (occurrencesSinceIgnored >= state.ignoredUntilTotalOccurrences) {
212226
return true;
213227
}
214228
}
@@ -387,14 +401,14 @@ export class ErrorAlertEvaluator {
387401
const state = stateMap.get(key);
388402
if (!state) continue;
389403

390-
await this,
391-
this._prisma.errorGroupState.update({
404+
await this._prisma.errorGroupState.update({
392405
where: { id: state.id },
393406
data: {
394407
status: "UNRESOLVED",
395408
ignoredUntil: null,
396409
ignoredUntilOccurrenceRate: null,
397410
ignoredUntilTotalOccurrences: null,
411+
ignoredAtOccurrenceCount: null,
398412
ignoredAt: null,
399413
ignoredReason: null,
400414
ignoredByUserId: null,

apps/webapp/app/v3/services/errorGroupActions.server.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export class ErrorGroupActions {
3838
ignoredUntil: null,
3939
ignoredUntilOccurrenceRate: null,
4040
ignoredUntilTotalOccurrences: null,
41+
ignoredAtOccurrenceCount: null,
4142
ignoredAt: null,
4243
ignoredReason: null,
4344
ignoredByUserId: null,
@@ -63,6 +64,7 @@ export class ErrorGroupActions {
6364
duration?: number;
6465
occurrenceRateThreshold?: number;
6566
totalOccurrencesThreshold?: number;
67+
occurrenceCountAtIgnoreTime?: number;
6668
reason?: string;
6769
}
6870
) {
@@ -83,6 +85,7 @@ export class ErrorGroupActions {
8385
ignoredUntil,
8486
ignoredUntilOccurrenceRate: params.occurrenceRateThreshold ?? null,
8587
ignoredUntilTotalOccurrences: params.totalOccurrencesThreshold ?? null,
88+
ignoredAtOccurrenceCount: params.occurrenceCountAtIgnoreTime ?? null,
8689
ignoredReason: params.reason ?? null,
8790
ignoredByUserId: params.userId,
8891
resolvedAt: null,
@@ -123,6 +126,7 @@ export class ErrorGroupActions {
123126
ignoredUntil: null,
124127
ignoredUntilOccurrenceRate: null,
125128
ignoredUntilTotalOccurrences: null,
129+
ignoredAtOccurrenceCount: null,
126130
ignoredAt: null,
127131
ignoredReason: null,
128132
ignoredByUserId: null,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
-- AlterTable
2+
ALTER TABLE "public"."ErrorGroupState" ADD COLUMN "ignoredAtOccurrenceCount" BIGINT;

internal-packages/database/prisma/schema.prisma

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2687,6 +2687,10 @@ model ErrorGroupState {
26872687
*/
26882688
ignoredUntilTotalOccurrences Int?
26892689
2690+
/// Total occurrence count at the time the error was ignored (from ClickHouse).
2691+
/// Used with ignoredUntilTotalOccurrences to compute occurrences since ignoring.
2692+
ignoredAtOccurrenceCount BigInt?
2693+
26902694
/**
26912695
* Error was ignored at this date
26922696
*/

0 commit comments

Comments
 (0)