feat: S08.03 slice 10 honest hostname (RFC 5424 §6.2.4 IP fallback) + NILVALUE PROCID#316
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same-day post-merge audit of the remaining @freertoswip tags showed the slice 9 closure was premature: HOSTNAME (RFC 5424 §6.2.4 IP fallback) and PROCID (§6.2.6 NILVALUE) follow the same RFC-honest pattern as the no-RTC TIMESTAMP. Epic #268 reopened; slice 10 (#315) extends the pattern. Append-only — preserves the slice 9 entry verbatim and adds the correction as a sub-section so the temporal sequence is honest. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… NILVALUE PROCID on FreeRTOS Closes #315. Last slice of S08.03 (#268). Apply the slice-9 RFC-honest reference-example pattern to two more fields. The library already supports both via NULL-callback / IP-stack introspection; only the example wiring and the BDD step semantics change. HOSTNAME — RFC 5424 §6.2.4 walk 1. FQDN — n/a (no DNS resolver) 2. Static IP address — present (queried via FreeRTOS_GetEndPointConfiguration) ← emit 3. hostname — n/a (no integrator-supplied hostname) 4. Dynamic IP — n/a (no DHCP) 5. NILVALUE — fallthrough The example calls FreeRTOS_GetEndPointConfiguration + FreeRTOS_inet_ntoa on the running endpoint, so TEST_IP_ADDRESS is the single source of truth in C. No EXAMPLE_STATIC_IPV4_STR literal — the same byte array that configures Plus-TCP at boot is read back at log-time. PROCID — RFC 5424 §6.2.6 NILVALUE config.getProcessId = NULL drops through NilStringFunction → empty field → FormatStringField writes "-" on the wire (Core/Source/SolidSyslog.c). BDD steps become target-aware - step_check_system_hostname: gethostname() on linux/windows; EXAMPLE_FREERTOS_STATIC_IP="10.0.2.15" on freertos (Python-side spec value mirroring TEST_IP_ADDRESS). - step_check_example_pid: spawned PID on linux/windows; "" (NILVALUE) on freertos. - parse_syslog_ng_line: explicit `PROCID=(\S*)` capture so empty values land as "" instead of being silently dropped by the generic `(\w+)=(\S+)` regex. Untag the four @freertoswip scenarios - header_fields.feature: hostname + procid - syslog.feature: walking skeleton (asserts both) - message_fields.feature: complete RFC 5424 message (asserts both) The walking-skeleton and complete-message scenarios also assert "timestamp within 5 seconds of now"; on FreeRTOS this passes vacuously because the wire is NILVALUE and syslog-ng's ${ISODATE} substitutes receipt time. Honest on RTC runners; benign-but-vacuous on FreeRTOS. Drop the now-unused g_hostname / g_processId globals, the OnSet "hostname" / "procid" branches, and the GetProcessId callback function. The corresponding `set` commands disappear from the interactive shell — the FreeRTOS reference doesn't permit overriding fields that the device cannot honestly supply. FreeRTOS BDD: 20 → 24 scenarios green (no failures, 25 skipped). Linux/Windows BDD: unchanged. Epic #268 closes on this slice's merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements RFC 5424 §6.2.4 fallback behavior for FreeRTOS HOSTNAME and PROCID fields. The FreeRTOS example now derives HOSTNAME from its configured static IP address and emits NILVALUE for PROCID via a NULL callback, replacing hardcoded placeholder test values. BDD tests are updated to assert these RFC-honest values, and ChangesRFC 5424 honest hostname/PROCID fallbacks for FreeRTOS
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Bdd/features/steps/syslog_steps.py`:
- Around line 935-939: In the FreeRTOS branch (the block guarded by `if
context.target == "freertos"` in syslog_steps.py) assert that the "PROCID" key
is present in `context.fields` and then assert its value is the empty string;
replace the current `context.fields.get("PROCID", "")` usage with an explicit
presence check (e.g., `if "PROCID" not in context.fields: fail...`) followed by
`actual = context.fields["PROCID"]` and the existing empty-string assertion so
that missing vs present-but-empty are distinguished.
In `@DEVLOG.md`:
- Around line 5105-5110: Update the DEVLOG entry that currently states "Slice 10
(`#315`) is in flight" for Epic `#268` to record completion: change the wording to
indicate slice-10 has been merged (include the merge context PR `#316`) and, if
preferred, add a short follow-up entry noting that the slice-10 implementation
applies fallbacks and untags the four `@freertoswip` scenarios in
`header_fields.feature`, `syslog.feature`, and `message_fields.feature` while
library code remained unchanged (NULL-callback path continues to drive NILVALUE
TIMESTAMP/PROCID and HOSTNAME-IP fallback).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5736bf39-4de2-46b4-ab67-71a659643452
📒 Files selected for processing (6)
Bdd/features/header_fields.featureBdd/features/message_fields.featureBdd/features/steps/syslog_steps.pyBdd/features/syslog.featureDEVLOG.mdExample/FreeRtos/SingleTask/main.c
💤 Files with no reviewable changes (3)
- Bdd/features/header_fields.feature
- Bdd/features/syslog.feature
- Bdd/features/message_fields.feature
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1105 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
… 10 DEVLOG entry - syslog_steps.py: PROCID step now asserts presence-then-value (not .get(default)), so template-gap bugs can't masquerade as NILVALUE passes. The explicit `PROCID=(\S*)` parse already preserves the absent-vs-empty distinction; the step now honours it. - DEVLOG.md: append the slice 10 closure entry alongside the slice 9 one (both ride this PR rather than a follow-up). Records the DRY catch on the IP literal, the smart-step decision over a tag split, the bundled-scenario timestamp asymmetry, and the two process misses (network-namespace restart, clang-format-on-Python). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1105 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
1 participant
Purpose
Closes #315. Last slice of S08.03 (#268). Applies the slice-9 RFC-honest
reference-example pattern to HOSTNAME and PROCID — the post-merge audit
of slice 9 (#313) caught these as the same shape we'd just landed for
TIMESTAMP. The library already supports both via NULL-callback /
IP-stack introspection; only the example wiring and the BDD step
semantics change. Includes the slice-9 DEVLOG entry rebased onto this
branch (PR #314 was closed in favour of this).
Change Description
HOSTNAME — RFC 5424 §6.2.4 walk (
Example/FreeRtos/SingleTask/main.c)The §6.2.4 preference order is
FQDN → static IP → hostname → dynamic IP → NILVALUE. The reference target has no FQDN (no DNS resolver), nointegrator-supplied hostname, and no DHCP, so rung 2 (static IP) is the
highest-preference value it can supply.
GetHostnamequeries the IPback from the running endpoint via
FreeRTOS_GetEndPointConfigurationand formats it with
FreeRTOS_inet_ntoa—TEST_IP_ADDRESS(the bytearray passed to
FreeRTOS_FillEndPointat boot) is the single sourceof truth in C; no
EXAMPLE_STATIC_IPV4_STRliteral duplicating it.The walk lives in a comment block that reads as a literal §6.2.4
checklist, so a future slice introducing DHCP or an integrator-hostname
hook can satisfy a different rung without touching the function shape.
PROCID — RFC 5424 §6.2.6 NILVALUE
config.getProcessId = NULLfalls through to the library'sNilStringFunction(no-op write);FormatStringFieldthen sees anempty field and emits
-on the wire(Core/Source/SolidSyslog.c:320-336).
QEMU FreeRTOS has no process model, so this is the honest answer per
§6.2.6's "NILVALUE MAY be used when no value is provided".
Smart-step BDD pattern (
Bdd/features/steps/syslog_steps.py)Per design discussion: smart steps over a tag-split, since the same
wire field with the same RFC meaning resolves to different values per
runner. No new
@hostname/@procidtag taxonomy — the existingsteps branch on
context.targetand assert the RFC-correct value:step_check_system_hostname—socket.gethostname()onlinux/windows;
EXAMPLE_FREERTOS_STATIC_IP = "10.0.2.15"onfreertos. The Python-side constant mirrors
TEST_IP_ADDRESSin theC example; that's the one irreducible spec/code mirror.
step_check_example_pid— spawned PID on linux/windows;""(NILVALUE → empty after parsing) on freertos.
parse_syslog_ng_line— explicitPROCID=(\S*)capture so wireNILVALUE lands in
context.fields["PROCID"]as""rather thanbeing silently dropped by the generic
(\w+)=(\S+)regex.Untag four scenarios
header_fields.feature: "Hostname matches the system hostname" and"Process ID matches the example program PID".
syslog.feature: "SolidSyslog sends a valid RFC 5424 message" —walking-skeleton end-to-end, asserts both fields.
message_fields.feature: "Complete RFC 5424 message with allfields" — also asserts both.
Note on the bundled scenarios' timestamp assertion
syslog.featureandmessage_fields.featurealso assert "timestampwithin 5 seconds of now". On RTC runners (Linux/Windows),
${ISODATE}faithfully reflects the wire timestamp from the message — honest test.
On FreeRTOS the wire is NILVALUE (slice 9), and syslog-ng substitutes
receipt time for
${ISODATE}— the assertion passes vacuously("receipt time is within 5s of now" is tautological). This is the same
NILVALUE-substitution behaviour we explored in slice 9; preserving it
here is the right call because timestamp.feature is feature-level
@rtc(the dedicated RTC assertion), and the bundled scenarios'timestamp step exists to prove the full RFC 5424 message round-trips,
not to gate timestamp correctness specifically.
Drop dead code
g_hostnameglobal,g_processIdglobal,OnSet "hostname"/OnSet "procid"branches, and the now-unusedGetProcessIdcallbackall gone. The interactive
set hostname/set procidcommandsdisappear with them — the reference doesn't permit overriding fields
the device cannot honestly supply.
DEVLOG
Includes two cherry-picked commits from PR #314 (closed):
### Update — audit caught hostname/PROCID gapsub-sectionappended below it, noting epic S08.03: UDP syslog from FreeRTOS reaches the oracle #268 was reopened and recording
the lesson — when introducing an "RFC explicit fallback rather
than placeholder TEST value" pattern, audit all fields of
similar shape before declaring the epic done.
Test Evidence
cmake --build --preset freertos-cross --target SolidSyslogFreeRtosSingleTask— clean.behave --tags='not @wip and not @freertoswip and not @rtc and @udp' Bdd/features/inside
freertos-target: 8 features / 24 scenarios pass, 0 fail, 25 skipped(up from 20 in slice 9; 4 newly-untagged scenarios all green).
behave --tags='not @wip and not @no_rtc' Bdd/features/header_fields.feature Bdd/features/syslog.feature Bdd/features/message_fields.featurefor the Linuxpair: 7 scenarios pass, 0 fail.
clang-format --dry-run --WerroronCore/Interface Core/Source Tests Example— clean.
py_compileonsyslog_steps.py.Areas Affected
Example/FreeRtos/SingleTask/main.c(Tier 3 — example): dropsg_hostname,g_processId,GetProcessId, twoOnSetbranches;rewrites
GetHostnameto query the IP-stack; setsconfig.getProcessId = NULL.Bdd/features/steps/syslog_steps.py: target-awarestep_check_system_hostname/step_check_example_pid; explicitPROCIDcapture inparse_syslog_ng_line; newEXAMPLE_FREERTOS_STATIC_IPconstant.Bdd/features/{header_fields,syslog,message_fields}.feature: four@freertoswiptags removed.DEVLOG.md: slice-9 closure entry + audit-correction sub-section.No production library code changed. No CI tag-filter changes (smart
steps mean no new tags).
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores
Changes