Repository navigation
Conversation
993ab45 to
657a300
Compare
|
@pront FYI, this should be ready for maintainer review now. Main scope:
|
|
Added the following tests: Unit tests (
Integration tests (
A new |
There was a problem hiding this comment.
Adding per-change rationale on why the syslog codec was modified rather than left alone. The integration tests in this PR exercise the codec through real rsyslog and syslog-ng for the first time, which surfaced these issues — each section below cites the RFC paragraph or upstream behavior that motivated the change.
(GitHub's inline-review API is currently returning a 422 pull_request_review_thread.base error on this PR, possibly because it's behind master, so consolidating into one comment with clickable line links.)
Edited 2026-06-11: refreshed line links after rebasing onto current master, and updated the sections marked ⟳ to match the latest revision.
syslog.rs#L86-L88 — non-log event rejection
Previously the if let Event::Log(...) guard returned Ok(()) with an empty buffer if a metric or trace ever reached the encoder. With octet-counting framing (RFC 6587) that puts a zero-length frame on the wire; with newline framing it's a bare delimiter. Both are malformed. Returning an error lets the sink emit a proper events_dropped. Locked in by test_non_log_event_rejected.
syslog.rs#L140-L141 — hostname sanitization (both RFCs)
Hostname was previously a raw passthrough. A bare CR/LF or NUL terminated the message early at the receiver. Sanitized to printable ASCII for both RFCs now. Test: test_rfc3164_hostname_sanitization.
⟳ syslog.rs#L122-L131 + encode_rfc_3164 — RFC 3164 TAG hardening
RFC 3164 §4.1.3's TAG ends at the first non-alphanumeric character — in practice rsyslog and syslog-ng terminate the programname at the first space — so app_name/proc_id now go through the same PRINTUSASCII sanitizer as the RFC 5424 header fields (spaces become _). RFC 3164 has no NILVALUE, so an app_name that is empty after sanitization falls back to vector rather than emitting a bare : tag. The 32-char TAG truncation now truncates the app_name while keeping [proc_id]: intact, or drops the proc_id entirely when it can't fit — never an unbalanced app[123:. An empty MSG no longer leaves a trailing space after the tag. Tests: test_rfc3164_tag_space_sanitization, test_rfc3164_empty_app_name_falls_back_to_default, test_rfc3164_tag_truncation, test_rfc3164_tag_oversized_proc_id_dropped, test_rfc3164_empty_message_no_trailing_space.
syslog.rs#L132-L138 — RFC 5424 PRINTUSASCII + NILVALUE
RFC 5424 §6 defines HOSTNAME / APP-NAME / PROCID / MSGID as 1*nPRINTUSASCII (%d33-126). The previous code only truncated, so a hostname containing a space, tab, newline, or non-ASCII char (emojis from a buggy upstream are not hypothetical) emitted an unparseable frame — rsyslog drops it, syslog-ng tags it omitted_invalid_chars. Sanitization happens before length truncation. Empty post-sanitize values collapse to NILVALUE (-) per the same section, otherwise the frame would be <14>1 ... host - (double space) which fails the ABNF in both daemons. Tests: test_rfc5424_header_field_sanitization, test_rfc5424_empty_header_fields_use_nilvalue.
syslog.rs#L209-L216 — MAX_LENGTH constants
RFC 5424 §6 ABNF length bounds: HOSTNAME = 1*255PRINTUSASCII, APP-NAME = 1*48PRINTUSASCII, PROCID = 1*128PRINTUSASCII, MSGID = 1*32PRINTUSASCII, and SD-NAME = 1*32PRINTUSASCII (§6.3.3).
syslog.rs#L374-L424 — parse_facility / parse_severity alias tables
The most consequential codec change. The previous code used strum(serialize_all = "kebab-case"), so it only accepted the full Rust variant names: emergency, critical, error, warning, informational, security (13), console (14), solaris-cron (15). Vector's syslog source is built on syslog_loose, which emits emerg, crit, err, warning, info for severities and audit, alert, clockd for facilities 13/14/15. Result: syslog source → vrl → syslog encoder silently rewrote PRI to <14> (the User/Informational default) for those values — real data loss on the most common syslog topology in Vector. The aliases here cover both the syslog_loose names and common rsyslog/syslog-ng forms (kernel, auth-priv, log-alert). Numeric strings (Value::Bytes("3")) are also accepted via parse_numeric_code since many sources/transforms preserve numbers as strings. Test: test_pri_aliases_from_syslog_decoder_names.
⟳ syslog.rs#L699 — Facility::LogAlert = 14
RFC 5424 §6.2.1 Table 4 calls facility 14 "log alert". The previous name Console is a Solaris/BSD-ism not in the spec — and notably not what syslog_loose emits for that facility. console is kept as an alias in parse_facility for backward compatibility, alongside alert and log-alert. The latest revision also drops the now-unused EnumString/VariantNames derives and strum alias attributes from Facility/Severity — the removed get_syslog_code was their last FromStr caller, so the parse_facility/parse_severity tables are the single source of truth instead of two alias lists that could drift.
syslog.rs#L587-L614 — structured-data validity & collision handling
Two RFC 5424 §6.3.3 compliance gaps surfaced by the integration tests:
- Empty SD-IDs were not skipped. SD-NAME is defined as
1*32PRINTUSASCII— at least one graphic ASCII char. Previously a{"": {...}}entry produced[ key="value"](leading space, no name) — unparseable. - Post-truncation collisions silently overwrote. Two distinct keys longer than 32 chars that differed only past char 32 collapsed to the same
BTreeMapkey, dropping the second.deduped_keysuffixes_1,_2, ... within the 32-char budget.
Tests: test_structured_data_empty_names_are_skipped, test_structured_data_param_name_length_limit_and_collision.
⟳ syslog.rs#L619 + join_sd_name#L318-L333 — flatten_object 32-char joined names
flatten_object joins sanitized segments with ., so a.b.c.d.e... can exceed the 32-char SD-NAME bound even when each segment is under it; previously the joined name went into the map as-is. The latest revision bounds the joined name via join_sd_name, which truncates the prefix and preserves the leaf segment — so sibling keys under a long path (…long-prefix….x vs …long-prefix….y) stay distinguishable instead of head-truncating to identical names; any remaining collisions still dedup with _N. Tests: test_structured_data_long_prefix_preserves_leaf_names, test_structured_data_prefix_truncation_collision_deduped.
Adds a new `syslog` sink that sends log events in syslog format over TCP, UDP, or Unix sockets. This wraps the existing socket transport utilities with the syslog codec hardcoded as the serializer, exposing syslog-specific options (rfc, facility, severity, app_name, proc_id, msg_id) at the top level for a first-class configuration experience. Closes vectordotdev#6863 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two issues prevented the rsyslog integration tests from running:
1. The `global(workDirectory="/tmp")` directive in the test rsyslog config
conflicts with the upstream `rsyslog/rsyslog:2026-02` image's base
config which already sets `workDirectory`. The duplicate parameter
failed config validation and the container never started. Drop the
line and let the image default stand.
2. The UDP tests passed wire-level but failed compliance with
`Missing metric component_sent_bytes_total{protocol}`. The UDP path
uses `register!(BytesSent::from(Protocol::UDP))` in `UdpSink::new`,
which records the event name when the sink is constructed. The test
helper built the sink before calling `run_and_assert_sink_compliance`,
so `init_test` cleared the recorded event name and `register!` never
fired again. Move the build inside the compliance future, mirroring
`socket.rs::test_datagram`. TCP was unaffected because it emits via
`emit!(SocketBytesSent)` inside the run loop.
Result: 6/6 syslog integration tests pass.
…and syslog-ng The previous suite verified happy-path correctness against rsyslog over plaintext UDP and TCP. Several realistic regression vectors weren't exercised end-to-end. This commit closes the gaps with a mix of unit and integration tests: Unit tests (src/sinks/syslog.rs): - tcp_finalizers_delivered_on_success: verifies BatchNotifier finalizers are marked Delivered after a successful TCP send. - udp_finalizers_delivered_on_success: same for the UDP datagram path, guarding `send_and_emit` in src/sinks/util/datagram.rs. - tcp_reconnect_after_server_close: forces a hard TCP disconnect mid- stream and confirms the sink reconnects to a freshly bound listener. Mirrors the existing socket sink reconnect test. Integration tests (src/sinks/syslog/integration_tests.rs): - tcp_octet_counting_rfc5424_with_proc_id_msg_id_structured_data_reaches_rsyslog: routes proc_id, msg_id, and a `structured_data` ObjectMap through the config layer to a real rsyslog and verifies the SD-element survives encoding intact. Catches regressions in `decant_config` that the codec-only unit tests can't see. - tcp_tls_octet_counting_rfc5424_to_syslog_ng: RFC 5425 (syslog over TLS) interop against syslog-ng's TLS network transport. The rsyslog/rsyslog:2026-02 image ships only `lmnsd_ptcp.so`, so TLS testing lives under syslog-ng which has OpenSSL built in. - udp_rfc5424_reaches_syslog_ng / tcp_octet_counting_rfc5424_reaches_syslog_ng: cross-daemon coverage. Different syslog implementations parse RFC 3164/5424 differently in practice; running our wire output through a second receiver guards against regressions that only one daemon would flag. Test infrastructure: - New balabit/syslog-ng:4.8.3 service in compose.yaml with TCP TLS using the project test certs. - tests/integration/syslog/data/syslog-ng.conf with `flags(store-raw-message)` on each source so `$RAWMSG` captures the wire-format line. - New SYSLOG_NG_* env vars in test.yaml for runner addressing. Result: 13/13 unit + 10/10 integration = 23/23 syslog sink tests pass.
57b58ad to
8b97ff3
Compare
Review fixes on top of the rebase onto upstream master: - Sanitize RFC 3164 app_name/proc_id with strict PRINTUSASCII rules (no spaces), fall back to "vector" for empty app names, and truncate long tags without corrupting the [proc_id] suffix - Drop the trailing space RFC 3164 emitted for empty messages - Truncate nested structured-data names prefix-first so sibling leaf keys stay distinguishable; log dropped empty SD names instead of silence - Remove dead strum alias tables; parse_facility/parse_severity are now the single source of truth - Avoid per-event Value clones in the encoder and the per-frame String allocation in the octet-counting framer - Make encoder tests immune to the decoder tests' global log-schema mutation (flaky under parallel runs); add UDP test read timeouts; assert integration output on the line matching the test's message - Add edge-case tests: oversized proc_id dropped instead of truncated, empty proc_id omits brackets, SD prefix-truncation collisions dedup - Use the syslog() driver for syslog-ng's octet-counting and TLS test ports so RFC 6587/5425 framing is actually parsed by the receiver (network() stored the frames unparsed, giving false coverage) - Remove invalid classes.commonly_used from the sink cue (broke the docs build) and widen the integration CI path filters Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # .github/workflows/ci-integration-review.yml # .github/workflows/integration.yml # lib/codecs/src/encoding/format/syslog.rs
- Port the sink to the validate/build split introduced on master, moving address parsing into `validate()` and adding coverage for it. - Adapt to master API changes: drop `#[configurable(derived)]`, return `serde_json::Value` from `generate_config`, and use `event_path!` in tests. - Parse facility/severity with a single string conversion and case-insensitive lookup tables instead of lowercasing per event. - Restore the `panic` severity alias added on master. - Avoid per-parameter `format!` allocations in structured-data encoding. - Move support notices to warnings (notices are no longer part of the docs schema) and mark the encoding feature as disabled since the sink exposes no encoding options. - Sort the `sinks-syslog` feature alphabetically.
rsyslog and syslog-ng previously logged only the raw message bytes, so the integration tests passed even if a receiver misparsed the header. Both receivers now log the fields they parsed (facility, severity, hostname, app-name, procid, msgid, structured data, and MSG) as JSON lines, and the tests assert exact values instead of substring matches on the raw line.
Add docs examples for the sink's framing and syslog options so the generated advanced example shows octet-counting framing and the syslog field mappings.
Reference the shared TLS and TCP keepalive schema definitions, matching the output of a full-feature docs generation.
|
@pront Possible to get a review on this in the near future or allow full test-suite run? Thanks! |
Resolve conflict in syslog encoder (keep PR logic, apply pedantic clippy fixes from upstream), regenerate syslog.cue with cue v0.17.1, and pick up domain 0.12.3 (RUSTSEC-2026-0310). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@pront fixed failing tests |
Summary
Adds a new
syslogsink that sends log events in syslog format (RFC 5424 or RFC 3164) over TCP, UDP, TCP with TLS, or Unix stream sockets.This wraps Vector's existing socket transport utilities (
TcpSinkConfig,UdpSinkConfig,UnixSinkConfig) with the syslog serializer from #23777/#24662. Syslog-specific options (rfc,facility,severity,app_name,proc_id,msg_id) are exposed undersyslogfor a first-class configuration experience. Addresses are parsed during config validation viaValidatedSink.The sink supports stream framing for TCP and Unix stream transports:
newline_delimitedfor RFC 6587 non-transparent framing.octet_countingfor RFC 6587 octet-counting framing.octet_countingfor RFC 5425 syslog over TLS.This also hardens the syslog serializer for enterprise interoperability:
Thanks to @vparfonov, @polarathene, and @syedriko for the syslog encoder work in #23777 that made this possible.
Vector configuration
TCP (RFC 5424):
UDP (RFC 3164):
TCP with TLS (RFC 5425):
Unix stream socket:
How did you test this PR?
Unit tests cover all transport modes, both RFC formats, custom PRI fields, TCP/TLS octet-counting, UDP datagram behavior, Unix stream delivery, reconnects, finalizer acknowledgement, address validation, and component compliance.
Codec tests cover RFC 5424/3164 formatting, PRI alias parsing (including the
panicalias from master), numeric PRI parsing, RFC 5424 header field sanitization, structured-data escaping, structured-data name truncation, empty-name handling, and non-log event rejection.Integration tests send to real rsyslog and syslog-ng receivers over UDP, TCP (newline and octet-counted framing), and TCP+TLS (RFC 5425). The receivers log the fields they parsed (facility, severity, hostname, app-name, procid, msgid, structured data, and MSG) as JSON, and the tests assert exact values, so a receiver misparsing any header field fails the test.
Validation commands run:
cargo test -p codecs --features syslog --lib encoding::format::syslogcargo test --no-default-features --features sinks-syslog -p vector --lib sinks::syslogcargo vdev int test syslogcargo vdev check rust(full-feature Clippy, as run bymake check-clippy)cargo fmt --all -- --checkcargo vdev check markdowncargo vdev build component-docs(from a full-feature schema) +cargo vdev check generated-docscargo vdev check component-examplescargo vdev check changelog-fragments./scripts/cue.sh vetChange Type
Is this a breaking change?
Does this PR include user facing changes?
References
syslogsink #6863Notes
socketsink withencoding.codec = "syslog".framing.method = "octet_counting"when messages can contain embedded newlines, such as stack traces or multiline JSON.framing.method = "octet_counting"./dev/logsockets, are not supported by this sink. Use thesocketsink withmode = "unix_datagram"for that case.max_message_lengthpolicy should define truncate/drop behavior, internal events/metrics, and transport-specific guidance.Intentionally out of scope for this initial implementation
Please do not block this PR on these items unless maintainers want to expand scope:
/dev/log) mode in this dedicated sink.