Add agent heartbeat monitoring and fix a query-language lexer bug

Agents now send an independent "still alive" record on a configurable
schedule (seconds/minutes/hours, [heartbeat] in agent.toml), separate
from real log traffic and tagged with a sentry.heartbeat attribute.
No new wire protocol -- it's an ordinary record through the same
PushBatch RPC/mTLS identity every log line already uses. Unavailability
alerting reuses the existing absence-condition alert rule type
unchanged; no new alerting code was needed. See
/docs/agent-heartbeat-monitoring.md for the design and how to build the
alert rule.

While verifying the alert rule live, found that the query language's
lexer never treated '-' as part of an identifier, so any unquoted
hyphenated filter value -- including the reference doc's own canonical
example, `host!=host-03` -- failed to parse at all. Fixed in
api/internal/querylang/lexer/lexer.go with regression tests; a leading
'-' still lexes as its own token so earliest=-1h/sort -count are
unaffected.
This commit is contained in:
2026-08-16 18:08:05 -07:00
parent 7d316f92db
commit 4df6931869
8 changed files with 367 additions and 3 deletions
+18
View File
@@ -135,6 +135,24 @@ See `config/agent.example.toml` for all fields.
./sentry-agent --config /path/to/agent.toml ./sentry-agent --config /path/to/agent.toml
``` ```
## Heartbeat and unavailability alerting
Every agent sends a small, independent "still alive" record on its own
schedule (`[heartbeat]` in the config, default every 60s), separate from
whatever real log traffic is flowing — see `config/agent.example.toml`.
This isn't a new wire protocol: it's an ordinary record through the same
`PushBatch` RPC and mTLS identity every log line uses, tagged with a
`sentry.heartbeat=true` attribute so it's easy to filter for and doesn't
show up as noise in normal log views. Set `interval` to a plain number
plus `s`/`m`/`h` (matches the query language's own `earliest=`/`latest=`
units); `enabled = false` turns it off entirely.
The platform has no separate "agent status" concept — an agent going
quiet is just the absence of its heartbeat records, which the existing
alerting engine already detects natively via an `absence`-condition
alert rule. See `/docs/agent-heartbeat-monitoring.md` for the exact rule
to create.
## Running as a Windows service ## Running as a Windows service
"A native Windows service, not a WSL wrapper" means implementing the Win32 "A native Windows service, not a WSL wrapper" means implementing the Win32
@@ -38,6 +38,19 @@ kind = "journald"
max_size = 500 max_size = 500
flush_interval_ms = 2000 flush_interval_ms = 2000
[heartbeat]
# How often this agent proves it's still alive to the platform, sent as
# its own record independent of whatever real log traffic is flowing --
# pair with an "absence" alert rule on the sentry.heartbeat attribute to
# get paged when a host goes quiet. Accepts a plain number + unit: s
# (seconds), m (minutes), or h (hours) -- same vocabulary as
# earliest=/latest= in the query language. See
# /docs/agent-heartbeat-monitoring.md.
enabled = true
interval = "60s"
# interval = "5m"
# interval = "1h"
[ingest] [ingest]
endpoint = "https://ingest.internal:4317" endpoint = "https://ingest.internal:4317"
+97 -1
View File
@@ -1,6 +1,7 @@
use anyhow::{Context, Result}; use anyhow::{Context, Result};
use serde::Deserialize; use serde::{Deserialize, Deserializer};
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::time::Duration;
#[cfg(not(windows))] #[cfg(not(windows))]
const DEFAULT_CONFIG_PATH: &str = "/etc/sentry-agent/agent.toml"; const DEFAULT_CONFIG_PATH: &str = "/etc/sentry-agent/agent.toml";
@@ -13,6 +14,7 @@ pub struct Config {
pub agent: AgentConfig, pub agent: AgentConfig,
pub source: SourceConfig, pub source: SourceConfig,
pub batch: BatchConfig, pub batch: BatchConfig,
pub heartbeat: HeartbeatConfig,
pub ingest: IngestConfig, pub ingest: IngestConfig,
pub tls: TlsConfig, pub tls: TlsConfig,
} }
@@ -141,6 +143,100 @@ impl Default for BatchConfig {
} }
} }
/// Sent independently of `batch` -- a heartbeat is a punctual liveness
/// signal, not log data, so it bypasses `Batcher` entirely (see
/// main.rs's `send_heartbeat`) rather than waiting on `max_size`/
/// `flush_interval_ms` like real records do. This is the operator-facing
/// "polling resolution" knob: how often this agent proves it's still
/// alive, which a `condition_type = "absence"` alert rule on the
/// `sentry.heartbeat` attribute (see /docs/agent-heartbeat-monitoring.md)
/// turns into "alert when this host goes quiet."
#[derive(Debug, Clone, Deserialize)]
#[serde(default)]
pub struct HeartbeatConfig {
pub enabled: bool,
#[serde(deserialize_with = "deserialize_duration")]
pub interval: Duration,
}
impl Default for HeartbeatConfig {
fn default() -> Self {
Self {
enabled: true,
interval: Duration::from_secs(60),
}
}
}
/// Parses a human-friendly duration string with an explicit unit suffix
/// -- "30s", "5m", "1h" -- deliberately the same s/m/h vocabulary
/// `earliest=`/`latest=` use in the query language
/// (/docs/query-language-reference.md), so the interval you set here and
/// the window you write in the matching alert rule's query read the same
/// way. Kept as a small hand-rolled parser rather than pulling in a
/// duration-parsing crate for this one field -- this is the
/// statically-linked edge agent every "no glibc runtime deps" constraint
/// in CLAUDE.md is about keeping lean, and the grammar needed here is a
/// handful of lines.
fn deserialize_duration<'de, D>(deserializer: D) -> Result<Duration, D::Error>
where
D: Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
parse_duration(&s).map_err(serde::de::Error::custom)
}
fn parse_duration(s: &str) -> Result<Duration, String> {
let s = s.trim();
let (num, unit) = s.split_at(s.len().saturating_sub(1));
let n: u64 = num
.parse()
.map_err(|_| format!("expected a duration like \"30s\", \"5m\", or \"1h\", got {s:?}"))?;
match unit {
"s" => Ok(Duration::from_secs(n)),
"m" => Ok(Duration::from_secs(n * 60)),
"h" => Ok(Duration::from_secs(n * 3600)),
_ => Err(format!("expected a time unit of s, m, or h after {n}, got {s:?}")),
}
}
#[cfg(test)]
mod heartbeat_config_tests {
use super::*;
#[test]
fn parses_seconds_minutes_hours() {
assert_eq!(parse_duration("30s").unwrap(), Duration::from_secs(30));
assert_eq!(parse_duration("5m").unwrap(), Duration::from_secs(300));
assert_eq!(parse_duration("2h").unwrap(), Duration::from_secs(7200));
}
#[test]
fn rejects_missing_or_unknown_unit() {
assert!(parse_duration("30").is_err());
assert!(parse_duration("30x").is_err());
assert!(parse_duration("").is_err());
}
#[test]
fn default_is_60_seconds_and_enabled() {
let cfg = HeartbeatConfig::default();
assert!(cfg.enabled);
assert_eq!(cfg.interval, Duration::from_secs(60));
}
#[test]
fn toml_field_parses_via_deserialize() {
#[derive(Deserialize)]
struct Wrapper {
#[serde(default)]
heartbeat: HeartbeatConfig,
}
let w: Wrapper = toml::from_str("[heartbeat]\nenabled = true\ninterval = \"90s\"\n").unwrap();
assert_eq!(w.heartbeat.interval, Duration::from_secs(90));
}
}
#[derive(Debug, Clone, Deserialize)] #[derive(Debug, Clone, Deserialize)]
#[serde(default)] #[serde(default)]
pub struct IngestConfig { pub struct IngestConfig {
+46
View File
@@ -97,8 +97,20 @@ pub async fn run_agent(config_path: Option<PathBuf>) -> Result<()> {
let mut batcher = Batcher::new(cfg.batch.max_size, flush_interval); let mut batcher = Batcher::new(cfg.batch.max_size, flush_interval);
let mut ticker = tokio::time::interval(flush_interval.max(Duration::from_millis(50))); let mut ticker = tokio::time::interval(flush_interval.max(Duration::from_millis(50)));
// Heartbeat's own ticker, independent of the batch flush ticker above
// -- it always fires on cfg.heartbeat.interval regardless of
// cfg.batch's settings or whether any real log traffic is flowing.
// Built unconditionally even when disabled (tokio::time::interval
// doesn't fail on construction); the `if cfg.heartbeat.enabled`
// select! guard is what actually turns it off, so a disabled
// heartbeat costs nothing beyond one idle timer.
let mut heartbeat_ticker = tokio::time::interval(cfg.heartbeat.interval.max(Duration::from_millis(50)));
loop { loop {
tokio::select! { tokio::select! {
_ = heartbeat_ticker.tick(), if cfg.heartbeat.enabled => {
send_heartbeat(&mut client, &host, &service).await;
}
maybe_line = rx.recv() => { maybe_line = rx.recv() => {
let Some(raw) = maybe_line else { let Some(raw) = maybe_line else {
tracing::warn!("source exited, flushing remaining batch and shutting down"); tracing::warn!("source exited, flushing remaining batch and shutting down");
@@ -191,6 +203,40 @@ async fn spawn_source(source: config::SourceConfig, tx: source::LineSender) {
} }
} }
/// Sends a single synthetic record through the same `PushBatch` RPC and
/// mTLS identity as real log data -- no new proto message, no new ingest
/// code, no new ClickHouse schema. Bypasses `Batcher` (see the heartbeat
/// ticker's own comment above): a heartbeat that got queued behind
/// `batch.max_size` or `batch.flush_interval_ms` would defeat the point
/// of a punctual "still alive" signal. Distinguished from a real log
/// record purely by the `sentry.heartbeat` attribute -- `service` stays
/// the agent's real configured service so it doesn't pollute
/// service-based dashboards/faceting with a fake value. See
/// /docs/agent-heartbeat-monitoring.md for how an absence alert rule
/// turns a run of missed heartbeats into a notification.
async fn send_heartbeat(client: &mut LogIngestClient<Channel>, host: &str, service: &str) {
let record = LogRecord {
timestamp_unix_nano: now_unix_nanos(),
host: host.to_string(),
service: service.to_string(),
severity: Severity::Info as i32,
message: "agent heartbeat".to_string(),
attributes: std::collections::HashMap::from([("sentry.heartbeat".to_string(), "true".to_string())]),
record_id: String::new(),
};
match grpc::send_batch(client, format!("heartbeat-{}", batch_id()), vec![record]).await {
Ok(_) => tracing::debug!(host, "heartbeat sent"),
Err(e) => tracing::warn!(error = %e, host, "heartbeat send failed"),
}
}
fn now_unix_nanos() -> i64 {
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_nanos() as i64)
.unwrap_or(0)
}
async fn flush(client: &mut LogIngestClient<Channel>, batch: Vec<LogRecord>) { async fn flush(client: &mut LogIngestClient<Channel>, batch: Vec<LogRecord>) {
let n = batch.len(); let n = batch.len();
let batch_id = batch_id(); let batch_id = batch_id();
+8 -2
View File
@@ -232,7 +232,13 @@ func isIdentStart(c rune) bool {
} }
// Identifiers allow dots (e.g. winevt.event_id, a real attribute key // Identifiers allow dots (e.g. winevt.event_id, a real attribute key
// shape from Phase 1) and digits after the first character. // shape from Phase 1), digits, and internal hyphens (e.g. host-03,
// api-service -- real, common bare-word values with no need for
// quoting) after the first character. A leading hyphen is deliberately
// NOT part of isIdentStart -- Minus has to stay its own token there so
// `earliest=-1h` and `sort -count`'s leading sign still lex correctly;
// this only affects a hyphen once a token has already started with a
// real identifier character.
func isIdentPart(c rune) bool { func isIdentPart(c rune) bool {
return isIdentStart(c) || isDigit(c) || c == '.' || c == '_' return isIdentStart(c) || isDigit(c) || c == '.' || c == '_' || c == '-'
} }
@@ -76,6 +76,34 @@ func TestLexFieldWithDots(t *testing.T) {
} }
} }
// TestLexIdentWithInternalHyphens is a regression test for a real bug:
// isIdentPart didn't include '-', so a bare (unquoted) hyphenated value
// like host-03 -- the exact example /docs/query-language-reference.md
// itself uses (`host!=host-03`) -- lexed as IDENT("host") MINUS
// IDENT("03") and failed to parse at all. A leading hyphen must still
// lex as its own MINUS token (earliest=-1h, sort -count depend on it) --
// only an internal hyphen, once a real identifier character has already
// started the token, should be absorbed.
func TestLexIdentWithInternalHyphens(t *testing.T) {
cases := []string{"host-03", "api-service", "heartbeat-test-host", "multi-hyphen-value"}
for _, c := range cases {
l := New(c)
tok := l.Next()
if tok.Kind != Ident || tok.Value != c {
t.Errorf("lexing %q: got %v, want Ident(%s)", c, tok, c)
}
if end := l.Next(); end.Kind != EOF {
t.Errorf("lexing %q: expected EOF after the identifier, got %v", c, end)
}
}
}
func TestLexFilterWithHyphenatedValue(t *testing.T) {
got := collectKinds(`host!=host-03`)
want := []Kind{Ident, Neq, Ident, EOF}
assertKinds(t, got, want)
}
func TestLexNumber(t *testing.T) { func TestLexNumber(t *testing.T) {
cases := []string{"123", "1.5", "0"} cases := []string{"123", "1.5", "0"}
for _, c := range cases { for _, c := range cases {
@@ -26,6 +26,39 @@ func TestParseSimpleFilter(t *testing.T) {
} }
} }
// TestParseFilterWithHyphenatedValue is a regression test for a real
// bug in the lexer (isIdentPart excluded '-'): this exact query is
// /docs/query-language-reference.md's own canonical unquoted example
// and used to fail with "unexpected MINUS after query".
func TestParseFilterWithHyphenatedValue(t *testing.T) {
q, err := Parse(`host!=host-03`)
if err != nil {
t.Fatalf("Parse() error = %v", err)
}
cmp, ok := q.Base.Terms[0].(ast.Comparison)
if !ok {
t.Fatalf("expected Comparison, got %T", q.Base.Terms[0])
}
if cmp.Field != "host" || cmp.Op != "!=" || cmp.Value != "host-03" {
t.Fatalf("unexpected comparison: %+v", cmp)
}
}
// TestParseNegativeTimeExprStillWorks guards against the hyphenated-
// identifier fix above accidentally swallowing the leading sign
// earliest=/latest= depend on -- a leading '-' must stay its own token.
func TestParseNegativeTimeExprStillWorks(t *testing.T) {
q, err := Parse(`earliest=-1h`)
if err != nil {
t.Fatalf("Parse() error = %v", err)
}
earliest := q.Base.Terms[0].(ast.TimeBound)
if earliest.Kind != "earliest" || !earliest.Expr.IsRelative || earliest.Expr.RelativeSign != -1 ||
earliest.Expr.RelativeN != 1 || earliest.Expr.RelativeUnit != "h" {
t.Fatalf("unexpected earliest: %+v", earliest)
}
}
func TestParseFullPipeline(t *testing.T) { func TestParseFullPipeline(t *testing.T) {
q, err := Parse(`service=api | where status>=500 | stats count(*) as errors by host | sort -errors`) q, err := Parse(`service=api | where status>=500 | stats count(*) as errors by host | sort -errors`)
if err != nil { if err != nil {
+124
View File
@@ -0,0 +1,124 @@
# Agent heartbeat and unavailability alerting
Every Linux/Windows agent (`/agent`) sends a small, independent "still
alive" record on its own schedule, in addition to whatever real log
traffic is flowing. This is what "define polling resolution in seconds,
minutes, hours" means in practice: how often an agent proves it's still
reachable, and how quickly the platform notices when it stops.
## Design: why this is a heartbeat, not a true pull
The agent's transport has always been push-only, by design — it dials
*out* to `ingest` over mTLS; nothing in the platform ever dials into an
agent (see `agent/sentry-agent/src/grpc.rs`'s doc comment). Making
liveness detection a true pull (the platform reaching into every remote
host on a schedule) would mean every agent needs a reachable address and
an open inbound port — a real problem for hosts behind NAT or with
dynamic IPs, which is the common case for a "remote" fleet, and exactly
the class of problem push was chosen to avoid.
A heartbeat gets the same outcome — the platform notices an agent going
quiet, on a configurable cadence — without any of that: the agent keeps
its one existing egress path, and "unavailable" is just the *absence* of
its heartbeat records, which the alerting engine (Phase 3) already
detects natively via `condition_type: "absence"`. No new RPC, no new
ingest code, no new ClickHouse schema, no new alert rule type — see
`/docs/phase-3-alerting-design.md` for the existing absence-condition
model this reuses unchanged.
## Configuring the heartbeat
`agent/sentry-agent/config/agent.example.toml`:
```toml
[heartbeat]
enabled = true
interval = "60s" # or "5m", "1h" -- same s/m/h vocabulary as earliest=/latest=
```
The heartbeat record is sent through the exact same `PushBatch` RPC and
mTLS identity every log line uses, bypassing the batch buffer (`[batch]`
`max_size`/`flush_interval_ms`) so it's punctual rather than subject to
batching delay. It's distinguished from real log data purely by an
attribute — `sentry.heartbeat=true` — not by a fake `service` value, so
it never pollutes service-based dashboards or faceting. `message` is the
literal string `"agent heartbeat"`.
## Building the alert rule
There's no dedicated "agent monitor" rule type — you create an ordinary
absence alert rule, scoped to one host, with a query window a little
wider than the heartbeat interval (so an evaluation landing just after a
heartbeat doesn't look like a false absence):
```sh
curl -X POST http://localhost:8081/rules -H 'Content-Type: application/json' -d '{
"name": "web-01 unavailable",
"description": "fires when web-01 misses its heartbeat window",
"query": "earliest=-3m host=web-01 sentry.heartbeat=true",
"query_language": "spl",
"condition_type": "absence",
"eval_interval_seconds": 60,
"for_minutes": 0,
"notification_target_id": "<your notification target id>",
"enabled": true
}'
```
- **`query`**: `earliest=-Ns/m/h` should comfortably exceed the agent's
configured `[heartbeat] interval` — 2-3x it is a reasonable default,
the same margin any liveness check needs against jitter.
`host=<hostname>` scopes the rule to one specific agent — the
evaluator's absence check only asks "did any row come back," so a
query spanning multiple hosts would only fire when *every* host in it
goes quiet at once, not when one specific host does (this is the same
"no per-group/multi-row alerting" limitation `/docs/phase-3-alerting-
design.md` already documents for threshold rules — one rule per
resource, not a fleet-wide wildcard, is real future work, not an
oversight here).
- **`eval_interval_seconds`**: the alerting-side "polling resolution" —
how often this specific rule is re-checked. Already second-granular
(any multiple of 30, the engine's documented floor — see below); a
value in minutes or hours is just a larger number of seconds, no
separate unit field needed.
- **`for_minutes`**: `0` fires on the very first absent evaluation, no
debounce. A real fleet might prefer `1` or `2` to ride out a single
missed evaluation before paging anyone — the same tradeoff any other
absence rule makes.
**`eval_interval_seconds` has a real floor of 30**, enforced by
`alerting`'s rule-creation validation (`eval_interval_seconds must be at
least 30`) — a rule can't be checked more often than every 30 seconds
regardless of how fast the agent's own heartbeat is. An agent heartbeat
interval faster than that (this doc's own live verification used 5s) is
still useful — it tightens how quickly *evidence* of an outage
accumulates in the query window — but the alert itself can't fire on a
tighter cadence than 30s.
## Verified live
This exact flow was run end-to-end against a live stack in this repo: a
real `sentry-agent` binary, heartbeat interval 5s, connected to a real
`ingest`; an absence rule (`earliest=-45s host=... sentry.heartbeat=true`,
`eval_interval_seconds=30`, `for_minutes=0`) created via the REST API
above; the agent process killed; the rule transitioned `ok``firing`
within one evaluation cycle (`condition_true_since`/`fired_at` both set
at the same timestamp the query window's silence became detectable);
and a real webhook delivery landed (`delivery_log` row: `status: sent,
response_status: 200`).
**A real, independent bug was found and fixed during this
verification**: the query language's lexer never treated `-` as part of
an identifier, so any unquoted hyphenated filter value —
`host=heartbeat-test-host`, or even the reference doc's own canonical
example `host!=host-03` — failed to parse at all
(`unexpected MINUS after query`). This wasn't specific to heartbeat
monitoring; it affected any hyphenated host/service name filtered
unquoted, which is extremely common. Fixed in
`api/internal/querylang/lexer/lexer.go`'s `isIdentPart` (now includes
`-`, but only mid-identifier — a leading `-` still lexes as its own
`Minus` token, so `earliest=-1h` and `sort -count` are unaffected).
Regression tests: `TestLexIdentWithInternalHyphens`,
`TestLexFilterWithHyphenatedValue` (lexer),
`TestParseFilterWithHyphenatedValue`,
`TestParseNegativeTimeExprStillWorks` (parser).