[leoparente]

Summary

Some label values come from packets: DNS query names in the top QNAME tables, and the hostname a DHCP client sends. A DNS name or DHCP hostname can hold any byte, and pktvisor passed those bytes unchanged into every output:

• OpenTelemetry. C++ protobuf serialises the string anyway, only logging contains invalid UTF-8 data when serializing. A strict decoder refuses the whole message. The orb-agent's OTLP bridge decodes with Go protobuf, so one such qname drops every pktvisor metric of that interval.
• Prometheus. The raw byte goes into the exposition text. Prometheus's text parser rejects it (invalid UTF-8 label value), failing the whole scrape.
• JSON. nlohmann::json::dump() throws type_error.316 on the top-N name, so the REST metrics response fails.

Every label value now goes through utf8_label_value:

• valid UTF-8, including non-ASCII names, is unchanged
• each byte that is not part of a well-formed UTF-8 sequence (Unicode Table 3-7, which matches Go and protobuf) is written as \DDD, its decimal value, as in DNS presentation format
• a backslash followed by three digits is written as \092, so a name holding the text \255 stays distinct from one holding the byte 0xFF; any other backslash is unchanged, so values such as Windows interface names keep their current form

A query for www.bad<0xff>name.test is now counted in every output as .bad\255name.test in dns_top_qname2_xacts, and as www.bad\255name.test in dns_top_qname3_xacts, dns_top_noerror_xacts and dns_top_nodata_xacts.

Where it applies:

• OTLP: one add_label_attributes helper replaces the five copied attribute loops (Counter, Cardinality, Histogram, Quantile, TopN).
• Prometheus: PrometheusSerializer::append_escaped escapes a value first, then applies the usual Prometheus escaping. Values that need no escaping take the old path with no extra allocation.
• JSON: TopN::to_json escapes string items.

Testing

• DNS v2 pcap test. It uses a new synthetic fixture: one query and its response for www.bad<0xff>name.test, between documentation addresses. The JSON, Prometheus and OTLP outputs all carry the escaped name, dump() no longer throws, and the serialised OTLP parses back with protobuf's strict UTF-8 check. Before the fix, all five checks fail.
• Unit tests for the helper. Valid input is unchanged, including U+0000, U+D7FF, U+E000 and U+10FFFF. The test also covers:
  • a lone continuation byte
  • a truncated sequence, including at the end of the value
  • overlong 2-, 3- and 4-byte forms
  • a surrogate
  • a code point above U+10FFFF
  • an invalid lead byte
  • a backslash, which is escaped only before three digits
• Unit tests for each output: the Prometheus serializer, TopN in JSON, Prometheus and OTLP, and a Counter label in OTLP.
• Mutation checks. Removing any one of the three output escapes, loosening any of the validator's three range limits (surrogates, overlong 3-byte forms, above U+10FFFF), or dropping the backslash rule makes tests fail.
• Against the agent's decoder. I dumped the DNS handler's OTLP bytes from the fixture and decoded them with Go's proto.Unmarshal, as the agent's bridge does:
  • before the fix: string field contains invalid UTF-8, and all 31 metrics are lost
  • with it: all 31 decode
• Full suite: ctest passes, 25 of 25 tests, on macOS arm64 (Apple clang 21, Release).

Not changed

• Label keys and policy_name. Neither is escaped. Keys come from code and config, and policy names must match pktvisor's module name pattern; neither comes from packets.
• --otel-interval. Separately: pktvisord parses --otel-interval but never copies it into OtelConfig, so OTLP pushes always run every 60 seconds. The orb-agent does not pass that flag. I left it for a separate change.

🤖 Generated with Claude Code

[nbl-ai-review]

AI Code Review — risk tier: Lite · dimensions run: correctness, security

Advisory only. A human owns the merge decision.

Summary — Net-positive change: packet-derived label values (DNS qnames, DHCP hostnames) are now correctly validated and \DDD-escaped before reaching OTLP, Prometheus, and JSON output paths, fixing dropped/failed metrics on malformed UTF-8. The backslash-before-digits disambiguation is sound, and escaping is consistently wired through every output path that carries raw packet strings. No correctness or security issues found.

_{🤖 AI Code Review · run · prompts: code-review/v1 · models: claude-sonnet-5/medium · context: description · 0 comment(s) · 0 prior AI review(s) · usage: $1.53 · 2935k in (96% cached) · 18.3k out}

[github-actions]

LCOV of commit 28989b7 during Debug Builds #242

  lines......: 85.3% (17523 of 20538 lines)
  functions..: 77.3% (1736 of 2247 functions)
  branches...: no data found

Files changed coverage rate: n/a

Full coverage report

[leoparente]

@codex review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-23T22:11:14.437494Z	28989b7	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 28989b733e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".