Proposal: Enhance the stability of log alerts

[zdyj3170101136]

background

We are heavy users of log alerts, and internally we have spent a lot of time and effort on log alerts.

We referenced Grafana's code and restructured Datadog's functionality to completely overhaul the alerting process.

enhancement

async execute alert

Currently, if any log alert takes too long to run, it will block the execution of other alerts.

  const now = new Date();
  const alerts = await getAlerts();
  logger.info(`Going to process ${alerts.length} alerts`);
  await Promise.all(alerts.map(alert => processAlert(now, alert)));
};

Modified to:

• Ensure that only one alertID is executed at a time via runningAlerts.

• If an alert with the same ID has not finished executing, skip it.

Refer to Grafana: https://github.com/grafana/grafana/blob/v12.1.1/pkg/services/ngalert/schedule/schedule.go#L420

const runningAlerts = new Map<string, Date>();

export default async () => {
  const now = new Date();
  const alerts = await getAlerts();
  logger.info(`Going to process ${alerts.length} alerts`);
  alerts.forEach(alert => {
    const alertId = alert.id;
    if (runningAlerts.has(alertId)) {
      logger.error({
        message: 'Tick dropped because alert rule evaluation is too slow',
        alert_id: alertId,
        alertname: alert.name,
        time: now,
        lastEvaluation: runningAlerts.get(alertId),
      });
      scheduleRuleEvaluationsMissedTotal.inc({
        alertname: alert.name ?? 'unknown',
      });
      return;
    }
    runningAlerts.set(alertId, now);
    void processAlert(now, alert).finally(() => runningAlerts.delete(alertId));
  });
};

add retry

During alert execution, whether a ClickHouse query fails or a webhook request fails, an exception should be thrown upwards.

Retry should then be initiated at the top level.

See https://github.com/grafana/grafana/blob/v12.1.1/pkg/services/ngalert/schedule/alert_rule.go#L285

      const attempt = async (retries: number): Promise<void> => {
        try {
          await processAlert(now, alert);
        } catch (err) {
          logger.error({
            message: '"Failed to evaluate rule',
            retries,
            error: serializeError(err),
            alertname: alert.name,
            alertId,
          });
          if (retries < MAX_ATTEMPTS) {
            await new Promise<void>(resolve =>
              setTimeout(resolve, RETRY_DELAY_MS),
            );
            return attempt(retries + 1);
          }
          // no more retries
          return;
        }
      };
      void attempt(1).finally(() => {
        EvalDuration.observe(
          (new Date().getTime() - evalStart.getTime()) / 1000,
        );
        runningAlerts.delete(alertId);
      });

Alarms are executed evenly.

Execute evenly over 1 minute to reduce database load.

Refer to https://github.com/grafana/grafana/blob/v12.1.1/pkg/services/ngalert/schedule/schedule.go#L439

    const alertId = alert.id;
    const delayMs = (fnv.hash(alertId) % 60) * 1000;
    setTimeout((alert) => {
        processAlert(alert)
    }, delayMs))
    // 00:01:30 s 或 00:01:59 去执行告警，告警都查询的是 [00:00:00, 00:01:00) 的日志。

do not query history alert

Without querying historical data: In the old implementation, alert would query data from the last execution to the current time.

For example, if a machine has been down for a month, querying the logs for the most recent month would trigger many meaningless alerts.

OK -> alert -> recovery -> alert.

Referring to the Grafana implementation, this should be modified to only check the current time (1 minute past).

Observability.

Implemented according to https://github.com/grafana/grafana/blob/v12.1.1/pkg/services/ngalert/metrics/scheduler.go.

const EvaluationMissed = new Prometheus.Counter({
  name: 'schedule_rule_evaluations_missed_total',
  help: 'The total number of rule evaluations missed due to a slow rule evaluation or schedule problem.',
  labelNames: ['alertname'],
});

const EvalDuration = new Prometheus.Summary({
  name: 'rule_evaluation_duration_seconds',
  help: 'The time to evaluate a rule.',
  labelNames: ['type'],
  percentiles: [0.01, 0.05, 0.5, 0.9, 0.99],
});

const EvalRetry = new Prometheus.Counter({
  name: 'rule_evaluation_retry_total',
  help: 'The total number of rule retry.',
  labelNames: ['alertname'],
});

const EvalFailures = new Prometheus.Counter({
  name: 'rule_evaluation_failures_total',
  help: 'The total number of rule evaluation failures.',
  labelNames: ['alertname'],
});

run alert as service

The alerting service runs independently, with multiple pods electing a leader via MongoDB.

Currently, the API, UI, and AlertTask tasks all run in a single deployment.

The AlertTask task will be split into a separate deployment to reduce interference.

The alerting service runs in multiple pods, with a leader elected via MongoDB to ensure that only one pod is actually querying alerts.

MongoDB Tables:

lockValue: string; // Unique identifier of the lock holder (pod name)

acquiredAt: Date; // Lock acquisition time

expiresAt: Date; // Lock expiration time, automatically cleared with TTL

Acquiring a Lock:

• Query MongoDB to see if anyone holds the lock.

  • If the current service holds the lock, return true and extend the lock's expiration time.

  • If another service holds the lock, return false.

  • If no service holds the lock, create a record and set the expiration time to 2 minutes.

    • If 11000 is returned, it indicates that a concurrent service successfully acquired the lock; return false.

    • An error was returned.

[dhable]

Thanks for the thoughtful research into alerts. I do have some follow up questions that I'd love to flush out here.

re: async execute alert

We've made some changes into the evaluation loop for alerts around concurrently evaluating each alert rule. We fetch and group alert rules for evaluation synchronously but then we do create independent async tasks in a queue:

    for (const task of alertTasks) {
      const teamWebhooksById =
        teamToWebhooks.get(task.conn.team.toString()) ?? new Map();
      this.task_queue.add(async () =>
        this.processAlertTask(task, teamWebhooksById),
      );
    }

Each item in the queue maps to a single alert rule, so each alertId should be only exist once in the alertsTask list. By default, the queue only allows a single element to be evaluated from the queue just to retain the behavior of the older alert code. Using the --concurrency command line argument allows you to define a larger number. We don't want to necessarily start evaluation all of the alerts right away to avoid loading CH or Mongo.

This change seems to address this or is there something else that you've noticed with our approach?

re: add retry

The design we've had is that we've wanted to avoid throwing exceptions in the alert evaluation / firing code. Assume that a configuration has numerous alerts to evaluate each cycle. If the first alert in that set has a failure, e.g. expired credentials or transient webhook receiver error, we don't want to stop evaluation of the remaining rules. Using exceptions runs the risk of missing a catch and then we could find ourselves missing evaluating of a number of alerts.

Further abstracting the notification channel using a provider pattern. Tolerance for retires and durability of the event probably vary widely across users. Allowing a more modular code base would allow people to contribute channels like kafka or tunable retries would cover more use cases. A sub-issue to track that would be wonderful.

re: Alarms are executed evenly

Our use of p-queue should allow for rate limiting of evaluation. Does that cover your use case? We could create another sub-issue to expose those options in the alert parameters.

re: Observability

There were some recent metrics added to the code but we could use more. This would be a good sub-task to split off this proposal.

re: run alert as service

Yes, we do run the alert code as a separate process in the same container if you were to just start HyperDX code from the code tree. We do this as a convenience to get a full stack running locally for development or for anyone who's looking to just try out HyperDX. For those looking for a production install, our helm chart uses a Kubernetes CronJob to control evaluating the execution of the alert logic in a separate container. This provides separation of the alerts evaluation that can be managed using standard Kubernetes concepts.

Creating a standalone service introduces more complexity that the service would need to handle like a durable main loop, control commands to manipulate the service, a web server to run health and readniess probes through, connection recovery logic to deal with network issues, dealing with replicas, etc. Having a single process that Kubernetes will start and monitor on a schedule simplifies things.

Of course there is nothing to prevent building a container image for tasks and setting RUN_SCHEDULED_TASKS_EXTERNALLY to false. This will cause the container to behave like it does inside the dev stack - it will run like a service but only executes the eval loop on a 1 minute schedule. You could then write a wrapper around that task and use any coordination scheme that you would like. It just seems like that's a detail of your desired deployment and something that could be contained in a Dockerfile using the task execution code to do all of its heavy lifting.

[dhable]

@claude Create new sub-issues with the details from re: add retry response and re: Alarms are executed evenly response.

[zdyj3170101136]

task_queue

Re: Asynchronous Execution

Even with unlimited concurrency, because we wait for all alerts to complete, a long-running alert will still affect other alerts.

Assuming you've deployed a service-based checkAlerts, with waitForCompletion: true set, a cron job will wait for the previous cron job to complete before executing. Therefore, if the previous cron job takes too long, it will affect the overall operation of the alerting service.

Essentially, what I want is to change waitForCompletion to false. This makes cron jobs independent of each other.

Then, if an alert takes too long (more than 1 minute), it will only prevent that alert from starting in the next cron job run (and this will be observed through metrics).

Reply: Even Alert Execution

Limiting the evaluation rate isn't actually the goal.

Even execution aims to lower the service's average CPU usage.

We use AWS ClickHouse Cloud and have automatic VPA enabled. Lower average CPU usage allows for lower service specifications, thus reducing costs.

Reply: Run alerts using a cron job.

Running alerts via a cron job makes it difficult to expose as service metrics.

[zdyj3170101136]

I also recommend adding an alarm connection that is used exclusively for alarms.

This will allow you to fully guarantee the stability of the alarm service through ClickHouse Cloud's compute isolation.

[zdyj3170101136]

If we can reach an agreement, I would be happy to submit a merge request.