You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "masaori335 (via GitHub)" <gi...@apache.org> on 2026/06/03 06:31:10 UTC

[PR] Self-Describing Binary Log Format (v3) [trafficserver]

masaori335 opened a new pull request, #13231:
URL: https://github.com/apache/trafficserver/pull/13231

   # Motivation
   
   In version 2, a segment header carries the field *symbols* (``fmt_fieldlist``,
   e.g. ``"chi cqu pssc"``) and a printf-style *template* (``fmt_printf``) but
   **not** the field types. To decode an entry a reader had to already know the
   type of each symbol, because the value encodings are only self-delimiting once
   the type is known (``IP`` is variable length, for example). That coupled every
   out-of-tree parser to the exact ATS build that wrote the log.
   
   Version 3 adds one thing: a per-segment **field-type schema** that lists the
   wire type of every field, in field order. Decoding then needs only the symbols
   (as keys) and the schema (for types).
   
   ----
   
   Depends on #13223


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#discussion_r3449231485


##########
src/proxy/logging/Log.cc:
##########
@@ -977,118 +1000,119 @@ Log::init_fields()
   Ptr<LogFieldAliasTable> cache_write_code_map = make_ptr(new LogFieldAliasTable);
   cache_write_code_map->init(N_LOG_CACHE_WRITE_TYPES, LOG_CACHE_WRITE_NONE, "-", LOG_CACHE_WRITE_LOCK_MISSED, "WL_MISS",
                              LOG_CACHE_WRITE_LOCK_ABORTED, "INTR", LOG_CACHE_WRITE_ERROR, "ERR", LOG_CACHE_WRITE_COMPLETE, "FIN");
-  field = new LogField("cache_write_result", "cwr", LogField::sINT, &LogAccess::marshal_cache_write_code,
+  field = new LogField("cache_write_result", "cwr", LogField::Type::sINT, &LogAccess::marshal_cache_write_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwr", field);
 
-  field = new LogField("cache_write_transform_result", "cwtr", LogField::sINT, &LogAccess::marshal_cache_write_transform_code,
+  field = new LogField("cache_write_transform_result", "cwtr", LogField::Type::sINT, &LogAccess::marshal_cache_write_transform_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwtr", field);
 
   // other fields
 
-  field = new LogField("transfer_time_ms", "ttms", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms", "ttms", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttms", field);
 
-  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsh", field);
 
-  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_ttmsf);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsf", field);
 
-  field =
-    new LogField("transfer_time_sec", "tts", LogField::sINT, &LogAccess::marshal_transfer_time_s, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("transfer_time_sec", "tts", LogField::Type::sINT, &LogAccess::marshal_transfer_time_s,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("tts", field);
 
-  field = new LogField("file_size", "fsiz", LogField::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("file_size", "fsiz", LogField::Type::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("fsiz", field);
 
-  field = new LogField("client_connection_id", "ccid", LogField::sINT, &LogAccess::marshal_client_http_connection_id,
+  field = new LogField("client_connection_id", "ccid", LogField::Type::sINT, &LogAccess::marshal_client_http_connection_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ccid", field);
 
-  field = new LogField("client_transaction_id", "ctid", LogField::sINT, &LogAccess::marshal_client_http_transaction_id,
+  field = new LogField("client_transaction_id", "ctid", LogField::Type::sINT, &LogAccess::marshal_client_http_transaction_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctid", field);
 
-  field = new LogField("cache_read_retry_attempts", "crra", LogField::sINT, &LogAccess::marshal_cache_read_retries,
+  field = new LogField("cache_read_retry_attempts", "crra", LogField::Type::sINT, &LogAccess::marshal_cache_read_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crra", field);
 
-  field = new LogField("cache_write_retry_attempts", "cwra", LogField::sINT, &LogAccess::marshal_cache_write_retries,
+  field = new LogField("cache_write_retry_attempts", "cwra", LogField::Type::sINT, &LogAccess::marshal_cache_write_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwra", field);
 
-  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::sINT,
+  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::Type::sINT,
                        &LogAccess::marshal_cache_collapsed_connection_success, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cccs", field);
 
-  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::sINT,
+  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_weight, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpw", field);
 
-  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::sINT,
+  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_dependence, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpd", field);
 
-  field = new LogField("proxy_protocol_version", "ppv", LogField::STRING, &LogAccess::marshal_proxy_protocol_version,
+  field = new LogField("proxy_protocol_version", "ppv", LogField::Type::STRING, &LogAccess::marshal_proxy_protocol_version,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppv", field);
 
-  field = new LogField("proxy_protocol_src_ip", "pps", LogField::IP, &LogAccess::marshal_proxy_protocol_src_ip,
+  field = new LogField("proxy_protocol_src_ip", "pps", LogField::Type::IP, &LogAccess::marshal_proxy_protocol_src_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppsip", field);

Review Comment:
   The field is defined with symbol "pps" but inserted into field_symbol_hash under key "ppsip". This makes %<pps> lookups fail (e.g. tests/gold_tests/proxy_protocol/proxy_protocol.test.py uses %<pps>).



##########
src/proxy/logging/LogAccess.cc:
##########
@@ -470,22 +470,25 @@ LogAccess::marshal_custom_field(char *buf, LogField::Type type, const LogField::
   void *sm = m_data->http_sm_for_plugins();
   if (sm == nullptr) {
     switch (type) {
-    case LogField::sINT:
-    case LogField::dINT:
+    case LogField::Type::sINT:
       if (buf) {
         marshal_int(buf, 0);
       }
       return INK_MIN_ALIGN;
-    case LogField::STRING: {
+    case LogField::Type::STRING: {
       int len = LogAccess::padded_strlen(nullptr);
       if (buf) {
         marshal_str(buf, nullptr, len);
       }
       return len;
     }
-    case LogField::IP:
+    case LogField::Type::IP:
       return marshal_ip(buf, nullptr);
-    case LogField::N_TYPES:
+    case LogField::Type::N_TYPES:
+      [[fallthrough]];
+    case LogField::Type::dINT:
+      [[fallthrough]];
+    case LogField::Type::INVALID:
       break;

Review Comment:
   In the sm==nullptr path, LogField::Type::dINT now falls through to the default path and calls plugin_marshal_func(sm, ...) with sm==nullptr, which can crash the callback. This is a behavioral regression from the previous behavior where dINT was handled as an integer placeholder when the SM is unavailable.



##########
doc/developer-guide/logging-architecture/binary-log-v3-format.en.rst:
##########
@@ -0,0 +1,216 @@
+.. Licensed to the Apache Software Foundation (ASF) under one
+   or more contributor license agreements.  See the NOTICE file
+   distributed with this work for additional information
+   regarding copyright ownership.  The ASF licenses this file
+   to you under the Apache License, Version 2.0 (the
+   "License"); you may not use this file except in compliance
+   with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing,
+   software distributed under the License is distributed on an
+   "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+   KIND, either express or implied.  See the License for the
+   specific language governing permissions and limitations
+   under the License.
+
+.. include:: ../../common.defs
+
+.. _binary-log-v3-format:
+
+Self-Describing Binary Log Format (v3)
+**************************************
+
+This page specifies the on-disk format of a binary log segment, version 3, in
+enough detail to implement a decoder *without* the Traffic Server source tree.
+A version 3 segment is **self-describing**: every field's type is published in
+the segment header, so a generic reader can decode each entry by dispatching on
+a small, stable set of type codes — no embedded copy of the ATS symbol-to-type
+table is required.
+
+Motivation
+==========
+
+In version 2, a segment header carries the field *symbols* (``fmt_fieldlist``,
+e.g. ``"chi cqu pssc"``) and a printf-style *template* (``fmt_printf``) but
+**not** the field types. To decode an entry a reader had to already know the
+type of each symbol, because the value encodings are only self-delimiting once
+the type is known (``IP`` is variable length, for example). That coupled every
+out-of-tree parser to the exact ATS build that wrote the log.
+
+Version 3 adds one thing: a per-segment **field-type schema** that lists the
+wire type of every field, in field order. Decoding then needs only the symbols
+(as keys) and the schema (for types).
+
+Segment layout
+==============
+
+A ``.blog`` file is a stream of segments, each a serialized ``LogBuffer``:
+
+::
+
+    LogBufferHeader            (per segment)
+      cookie       = 0xaceface
+      version      = 3
+      format_type, byte_count, entry_count, timestamps, flags, signature
+      fmt_name_offset
+      fmt_fieldlist_offset      -> "chi cqu pssc ..."   (symbols, space separated)

Review Comment:
   The spec describes fmt_fieldlist as space-separated, but ATS actually serializes it as a comma-separated symbol list (see LogFormat::parse_format_string(), which inserts commas). Out-of-tree decoders following this doc will split incorrectly unless they special-case commas.



##########
src/proxy/logging/YamlLogConfig.cc:
##########
@@ -233,11 +234,26 @@ YamlLogConfig::decodeLogObject(const YAML::Node &node)
     }
   }
 
+  // On-disk binary segment version (default: current). 2 emits the pre-v3
+  // layout for parsers that don't yet understand v3; binary logs only.
+  int binary_log_version = LOG_SEGMENT_VERSION;
+  if (node["binary_log_version"]) {
+    binary_log_version = node["binary_log_version"].as<int>();
+    if (!log_segment_version_supported(binary_log_version)) {
+      Warning("Invalid binary_log_version '%d' (supported %d-%d); using %d", binary_log_version, LOG_SEGMENT_VERSION_MIN_SUPPORTED,
+              LOG_SEGMENT_VERSION, LOG_SEGMENT_VERSION);
+      binary_log_version = LOG_SEGMENT_VERSION;
+    } else if (file_type != LOG_FILE_BINARY) {
+      Warning("binary_log_version only applies to binary logs; ignoring for this object");
+    }

Review Comment:
   When binary_log_version is set on a non-binary log object, this warns that it is ignored but still passes the value into the LogObject constructor. Either the warning should be removed or the value should actually be ignored (reset to default) to match the message.



##########
src/proxy/logging/Log.cc:
##########
@@ -977,118 +1000,119 @@ Log::init_fields()
   Ptr<LogFieldAliasTable> cache_write_code_map = make_ptr(new LogFieldAliasTable);
   cache_write_code_map->init(N_LOG_CACHE_WRITE_TYPES, LOG_CACHE_WRITE_NONE, "-", LOG_CACHE_WRITE_LOCK_MISSED, "WL_MISS",
                              LOG_CACHE_WRITE_LOCK_ABORTED, "INTR", LOG_CACHE_WRITE_ERROR, "ERR", LOG_CACHE_WRITE_COMPLETE, "FIN");
-  field = new LogField("cache_write_result", "cwr", LogField::sINT, &LogAccess::marshal_cache_write_code,
+  field = new LogField("cache_write_result", "cwr", LogField::Type::sINT, &LogAccess::marshal_cache_write_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwr", field);
 
-  field = new LogField("cache_write_transform_result", "cwtr", LogField::sINT, &LogAccess::marshal_cache_write_transform_code,
+  field = new LogField("cache_write_transform_result", "cwtr", LogField::Type::sINT, &LogAccess::marshal_cache_write_transform_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwtr", field);
 
   // other fields
 
-  field = new LogField("transfer_time_ms", "ttms", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms", "ttms", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttms", field);
 
-  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsh", field);
 
-  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_ttmsf);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsf", field);
 
-  field =
-    new LogField("transfer_time_sec", "tts", LogField::sINT, &LogAccess::marshal_transfer_time_s, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("transfer_time_sec", "tts", LogField::Type::sINT, &LogAccess::marshal_transfer_time_s,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("tts", field);
 
-  field = new LogField("file_size", "fsiz", LogField::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("file_size", "fsiz", LogField::Type::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("fsiz", field);
 
-  field = new LogField("client_connection_id", "ccid", LogField::sINT, &LogAccess::marshal_client_http_connection_id,
+  field = new LogField("client_connection_id", "ccid", LogField::Type::sINT, &LogAccess::marshal_client_http_connection_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ccid", field);
 
-  field = new LogField("client_transaction_id", "ctid", LogField::sINT, &LogAccess::marshal_client_http_transaction_id,
+  field = new LogField("client_transaction_id", "ctid", LogField::Type::sINT, &LogAccess::marshal_client_http_transaction_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctid", field);
 
-  field = new LogField("cache_read_retry_attempts", "crra", LogField::sINT, &LogAccess::marshal_cache_read_retries,
+  field = new LogField("cache_read_retry_attempts", "crra", LogField::Type::sINT, &LogAccess::marshal_cache_read_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crra", field);
 
-  field = new LogField("cache_write_retry_attempts", "cwra", LogField::sINT, &LogAccess::marshal_cache_write_retries,
+  field = new LogField("cache_write_retry_attempts", "cwra", LogField::Type::sINT, &LogAccess::marshal_cache_write_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwra", field);
 
-  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::sINT,
+  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::Type::sINT,
                        &LogAccess::marshal_cache_collapsed_connection_success, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cccs", field);
 
-  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::sINT,
+  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_weight, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpw", field);
 
-  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::sINT,
+  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_dependence, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpd", field);
 
-  field = new LogField("proxy_protocol_version", "ppv", LogField::STRING, &LogAccess::marshal_proxy_protocol_version,
+  field = new LogField("proxy_protocol_version", "ppv", LogField::Type::STRING, &LogAccess::marshal_proxy_protocol_version,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppv", field);
 
-  field = new LogField("proxy_protocol_src_ip", "pps", LogField::IP, &LogAccess::marshal_proxy_protocol_src_ip,
+  field = new LogField("proxy_protocol_src_ip", "pps", LogField::Type::IP, &LogAccess::marshal_proxy_protocol_src_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppsip", field);
 
-  field = new LogField("proxy_protocol_dst_ip", "ppd", LogField::IP, &LogAccess::marshal_proxy_protocol_dst_ip,
+  field = new LogField("proxy_protocol_dst_ip", "ppd", LogField::Type::IP, &LogAccess::marshal_proxy_protocol_dst_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppdip", field);

Review Comment:
   The field is defined with symbol "ppd" but inserted into field_symbol_hash under key "ppdip". This makes %<ppd> lookups fail and is inconsistent with the symbol stored in the LogField.



##########
doc/appendices/command-line/traffic_logcat.en.rst:
##########
@@ -25,14 +25,17 @@ traffic_logcat
 Synopsis
 ========
 
-:program:`traffic_logcat` [-o output-file | -a] [-CEhSVw2] [input-file ...]
+:program:`traffic_logcat` [-o output-file | -a] [-CEHhSVw2] [input-file ...]

Review Comment:
   The synopsis option list does not include `-j` even though `--json` is documented below and implemented in traffic_logcat. This makes the quick usage line inaccurate.



##########
src/proxy/logging/Log.cc:
##########
@@ -328,371 +328,389 @@ Log::init_fields()
   LogField::init_milestone_container();
 
   // client -> proxy fields
-  field = new LogField("client_host_ip", "chi", LogField::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_str);
+  field =
+    new LogField("client_host_ip", "chi", LogField::Type::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chi", field);
 
-  field =
-    new LogField("client_host_port", "chp", LogField::sINT, &LogAccess::marshal_client_host_port, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("client_host_port", "chp", LogField::Type::sINT, &LogAccess::marshal_client_host_port,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chp", field);
 
-  field =
-    new LogField("client_host_ip_hex", "chih", LogField::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_hex);
+  field = new LogField("client_host_ip_hex", "chih", LogField::Type::IP, &LogAccess::marshal_client_host_ip,
+                       &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chih", field);
 
-  field = new LogField("client_host_ip_verified", "chiv", LogField::IP, &LogAccess::marshal_client_host_ip_verified,
+  field = new LogField("client_host_ip_verified", "chiv", LogField::Type::IP, &LogAccess::marshal_client_host_ip_verified,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chiv", field);
 
   // remote client (Not necessarily the requesting client IP - See proxy protocol)
-  field = new LogField("remote_host_ip", "rchi", LogField::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_str);
+  field =
+    new LogField("remote_host_ip", "rchi", LogField::Type::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchi", field);
 
-  field = new LogField("remote_host_port", "rchp", LogField::sINT, &LogAccess::marshal_remote_host_port,
+  field = new LogField("remote_host_port", "rchp", LogField::Type::sINT, &LogAccess::marshal_remote_host_port,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchp", field);
 
-  field =
-    new LogField("remote_host_ip_hex", "rchh", LogField::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_hex);
+  field = new LogField("remote_host_ip_hex", "rchh", LogField::Type::IP, &LogAccess::marshal_remote_host_ip,
+                       &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchh", field);
 
   // interface ip
 
-  field =
-    new LogField("host_interface_ip", "hii", LogField::IP, &LogAccess::marshal_host_interface_ip, &LogAccess::unmarshal_ip_to_str);
+  field = new LogField("host_interface_ip", "hii", LogField::Type::IP, &LogAccess::marshal_host_interface_ip,
+                       &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("hii", field);
 
-  field = new LogField("host_interface_ip_hex", "hiih", LogField::IP, &LogAccess::marshal_host_interface_ip,
+  field = new LogField("host_interface_ip_hex", "hiih", LogField::Type::IP, &LogAccess::marshal_host_interface_ip,
                        &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("hiih", field);
   // interface ip end
-  field = new LogField("client_auth_user_name", "caun", LogField::STRING, &LogAccess::marshal_client_auth_user_name,
+  field = new LogField("client_auth_user_name", "caun", LogField::Type::STRING, &LogAccess::marshal_client_auth_user_name,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("caun", field);
 
-  field = new LogField("plugin_identity_id", "piid", LogField::sINT, &LogAccess::marshal_plugin_identity_id,
+  field = new LogField("plugin_identity_id", "piid", LogField::Type::sINT, &LogAccess::marshal_plugin_identity_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("piid", field);
 
-  field = new LogField("plugin_identity_tag", "pitag", LogField::STRING, &LogAccess::marshal_plugin_identity_tag,
+  field = new LogField("plugin_identity_tag", "pitag", LogField::Type::STRING, &LogAccess::marshal_plugin_identity_tag,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pitag", field);
 
-  field = new LogField("client_req_timestamp_sec", "cqts", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_sec", "cqts", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqts", field);
 
-  field = new LogField("client_req_timestamp_hex_sec", "cqth", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_hex_sec", "cqth", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_str_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqth", field);
 
-  field = new LogField("client_req_timestamp_squid", "cqtq", LogField::sINT, &LogAccess::marshal_client_req_timestamp_ms,
+  field = new LogField("client_req_timestamp_squid", "cqtq", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_ms,
                        &LogAccess::unmarshal_ttmsf);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtq", field);
 
-  field = new LogField("client_req_timestamp_netscape", "cqtn", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_netscape", "cqtn", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_netscape_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtn", field);
 
-  field = new LogField("client_req_timestamp_date", "cqtd", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_date", "cqtd", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_date_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtd", field);
 
-  field = new LogField("client_req_timestamp_time", "cqtt", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_time", "cqtt", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_time_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtt", field);
 
-  field = new LogField("client_req_http_method", "cqhm", LogField::STRING, &LogAccess::marshal_client_req_http_method,
+  field = new LogField("client_req_http_method", "cqhm", LogField::Type::STRING, &LogAccess::marshal_client_req_http_method,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqhm", field);
 
-  field = new LogField("client_req_url", "cqu", LogField::STRING, &LogAccess::marshal_client_req_url, &LogAccess::unmarshal_str,
-                       &LogAccess::set_client_req_url);
+  field = new LogField("client_req_url", "cqu", LogField::Type::STRING, &LogAccess::marshal_client_req_url,
+                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_url);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqu", field);
 
-  field = new LogField("client_req_url", "pqu", LogField::STRING, &LogAccess::marshal_client_req_url, &LogAccess::unmarshal_str,
-                       &LogAccess::set_client_req_url);
+  field = new LogField("client_req_url", "pqu", LogField::Type::STRING, &LogAccess::marshal_client_req_url,
+                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_url);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqu", field);
 
-  field = new LogField("client_req_url_canonical", "cquc", LogField::STRING, &LogAccess::marshal_client_req_url_canon,
+  field = new LogField("client_req_url_canonical", "cquc", LogField::Type::STRING, &LogAccess::marshal_client_req_url_canon,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquc", field);
 
-  field = new LogField("client_req_url_canonical", "pquc", LogField::STRING, &LogAccess::marshal_client_req_url_canon,
+  field = new LogField("client_req_url_canonical", "pquc", LogField::Type::STRING, &LogAccess::marshal_client_req_url_canon,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pquc", field);
 
-  field =
-    new LogField("client_req_unmapped_url_canonical", "cquuc", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_canon,
-                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_canon);
+  field = new LogField("client_req_unmapped_url_canonical", "cquuc", LogField::Type::STRING,
+                       &LogAccess::marshal_client_req_unmapped_url_canon, &LogAccess::unmarshal_str,
+                       &LogAccess::set_client_req_unmapped_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquuc", field);
 
-  field = new LogField("client_req_unmapped_url_path", "cquup", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_path,
-                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_path);
+  field =
+    new LogField("client_req_unmapped_url_path", "cquup", LogField::Type::STRING, &LogAccess::marshal_client_req_unmapped_url_path,
+                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquup", field);
 
-  field = new LogField("client_req_unmapped_url_host", "cquuh", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_host,
-                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_host);
+  field =
+    new LogField("client_req_unmapped_url_host", "cquuh", LogField::Type::STRING, &LogAccess::marshal_client_req_unmapped_url_host,
+                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_host);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquuh", field);
 
-  field = new LogField("client_req_url_scheme", "cqus", LogField::STRING, &LogAccess::marshal_client_req_url_scheme,
+  field = new LogField("client_req_url_scheme", "cqus", LogField::Type::STRING, &LogAccess::marshal_client_req_url_scheme,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqus", field);
 
-  field = new LogField("client_req_url_scheme", "pqus", LogField::STRING, &LogAccess::marshal_client_req_url_scheme,
+  field = new LogField("client_req_url_scheme", "pqus", LogField::Type::STRING, &LogAccess::marshal_client_req_url_scheme,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqus", field);
 
-  field = new LogField("client_req_url_path", "cqup", LogField::STRING, &LogAccess::marshal_client_req_url_path,
+  field = new LogField("client_req_url_path", "cqup", LogField::Type::STRING, &LogAccess::marshal_client_req_url_path,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqup", field);
 
-  field = new LogField("client_req_url_path", "pqup", LogField::STRING, &LogAccess::marshal_client_req_url_path,
+  field = new LogField("client_req_url_path", "pqup", LogField::Type::STRING, &LogAccess::marshal_client_req_url_path,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqup", field);
 
-  field = new LogField("client_req_protocol_version", "cqpv", LogField::STRING, &LogAccess::marshal_client_req_protocol_version,
-                       &LogAccess::unmarshal_str);
+  field = new LogField("client_req_protocol_version", "cqpv", LogField::Type::STRING,
+                       &LogAccess::marshal_client_req_protocol_version, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqpv", field);
 
-  field = new LogField("server_req_protocol_version", "sqpv", LogField::STRING, &LogAccess::marshal_server_req_protocol_version,
-                       &LogAccess::unmarshal_str);
+  field = new LogField("server_req_protocol_version", "sqpv", LogField::Type::STRING,
+                       &LogAccess::marshal_server_req_protocol_version, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("sqpv", field);
 
-  field = new LogField("client_req_header_len", "cqhl", LogField::sINT, &LogAccess::marshal_client_req_header_len,
+  field = new LogField("client_req_header_len", "cqhl", LogField::Type::sINT, &LogAccess::marshal_client_req_header_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqhl", field);
 
-  field = new LogField("client_req_squid_len", "cqql", LogField::sINT, &LogAccess::marshal_client_req_squid_len,
+  field = new LogField("client_req_squid_len", "cqql", LogField::Type::sINT, &LogAccess::marshal_client_req_squid_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqql", field);
 
   // Client request squid length plus TLS handshake bytes received for TLS connections
-  field = new LogField("client_req_squid_len_tls", "cqqtl", LogField::sINT, &LogAccess::marshal_client_req_squid_len_tls,
+  field = new LogField("client_req_squid_len_tls", "cqqtl", LogField::Type::sINT, &LogAccess::marshal_client_req_squid_len_tls,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqqtl", field);
 
-  field = new LogField("cache_lookup_url_canonical", "cluc", LogField::STRING, &LogAccess::marshal_cache_lookup_url_canon,
+  // Client request squid length plus TLS handshake bytes received for TLS connections
+  field = new LogField("client_req_squid_len_tls", "cqqtl", LogField::Type::sINT, &LogAccess::marshal_client_req_squid_len_tls,
+                       &LogAccess::unmarshal_int_to_str);
+  global_field_list.add(field, false);
+  field_symbol_hash.emplace("cqqtl", field);

Review Comment:
   The "cqqtl" (client_req_squid_len_tls) field is registered twice. This adds a duplicate LogField to the global list and the second field_symbol_hash.emplace() will fail (key already exists), leaving the global list inconsistent and leaking the duplicate field allocation.



##########
src/traffic_logcat/logcat.cc:
##########
@@ -120,6 +128,207 @@ follow_rotate(const char *input_file, ino_t old_inode_num)
   }
 }
 
+// Write all @a len bytes of @a buf to @a fd, retrying short writes and EINTR.
+// write() may transfer fewer bytes than requested (notably to a pipe), so a
+// single call is not enough to guarantee the whole line lands. Returns true
+// only if every byte was written.
+bool
+write_all(int fd, const char *buf, size_t len)
+{
+  size_t off = 0;
+  while (off < len) {
+    ssize_t w = write(fd, buf + off, len - off);
+    if (w < 0) {
+      if (errno == EINTR) {
+        continue;
+      }
+      return false;
+    }
+    off += static_cast<size_t>(w);
+  }
+  return true;
+}
+
+// Emit every entry of a v3 segment as a line of JSON. v2 segments lack the
+// field-type schema needed for self-describing decode, so they are skipped
+// with a note. Returns the number of entries written.
+int
+write_json_logbuffer(LogBufferHeader *header, int out_fd)
+{
+  if (header->fmt_fieldtypes() == nullptr) {
+    fprintf(stderr, "JSON output requires a v3 binary log with a field-type schema; skipping segment.\n");
+    return 0;
+  }
+
+  LogBufferIterator iter(header);
+  LogEntryHeader   *entry;
+  char              line[LOG_MAX_FORMATTED_LINE];
+  int               count = 0;
+
+  while ((entry = iter.next()) != nullptr) {
+    int n = log_entry_to_json(entry, header, line, sizeof(line) - 1);
+    if (n > 0) {
+      line[n] = '\n';
+      if (!write_all(out_fd, line, static_cast<size_t>(n) + 1)) {
+        fprintf(stderr, "Error writing JSON output: %s\n", strerror(errno));
+        return count;
+      }
+      ++count;
+    }
+  }
+  return count;
+}
+
+// Human-readable name for a LogBufferHeader::format_type value.
+const char *
+log_format_type_name(uint32_t format_type)
+{
+  switch (format_type) {
+  case LOG_FORMAT_CUSTOM:
+    return "CUSTOM";
+  case LOG_FORMAT_TEXT:
+    return "TEXT";
+  default:
+    return "UNKNOWN";
+  }
+}
+
+// Human-readable name for a v3 wire type code (LogField::Type).
+const char *
+log_field_type_name(uint8_t code)
+{
+  switch (static_cast<LogField::Type>(code)) {
+  case LogField::Type::sINT:
+    return "sINT";
+  case LogField::Type::dINT:
+    return "dINT";
+  case LogField::Type::STRING:
+    return "STRING";
+  case LogField::Type::IP:
+    return "IP";
+  case LogField::Type::INVALID:
+    return "INVALID";
+  default:
+    return "UNKNOWN";
+  }
+}
+
+// Return the NUL-terminated string living at @a offset within the segment, or
+// nullptr if the offset is zero, lands outside the segment, or is not
+// terminated before the segment ends. A .blog read here may be untrusted, so
+// every offset is validated against byte_count.
+const char *
+bounded_header_str(LogBufferHeader *header, uint32_t offset)
+{
+  if (offset == 0) {
+    return nullptr;
+  }
+
+  char *seg_start = reinterpret_cast<char *>(header);
+  char *seg_end   = seg_start + header->byte_count;
+  char *addr      = seg_start + offset;
+
+  if (addr < seg_start || addr >= seg_end) {
+    return nullptr;
+  }
+  if (memchr(addr, '\0', static_cast<size_t>(seg_end - addr)) == nullptr) {
+    return nullptr;
+  }
+  return addr;

Review Comment:
   bounded_header_str() computes `seg_start + offset` before validating that offset is within the segment. For a corrupt/untrusted header with a very large offset, this pointer arithmetic can overflow and is undefined behavior even if you later compare the pointer. Validate `offset < byte_count` before forming the pointer.



##########
src/traffic_logcat/LogEntryJson.cc:
##########
@@ -0,0 +1,291 @@
+/** @file
+
+  Reference decoder for the self-describing v3 binary log format.
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+
+#include "LogEntryJson.h"
+
+#include "proxy/logging/LogBuffer.h"
+#include "proxy/logging/LogAccess.h"
+#include "proxy/logging/LogField.h"
+
+// Deliberately not including the global field table (proxy/logging/Log.h):
+// decoding must depend only on the segment's schema (see LogEntryJson.h).
+
+#include "tscore/ink_inet.h"
+#include "tscore/ink_align.h"
+
+#include <cinttypes>
+#include <cstdint>
+#include <cstdio>
+#include <cstring>
+
+namespace
+{
+// fmt_fieldlist symbols are comma-separated (e.g. "chi,cqu,pssc"); tolerate
+// spaces too.
+bool
+is_field_sep(char c)
+{
+  return c == ',' || c == ' ';
+}
+} // namespace
+
+int
+log_entry_to_json(LogEntryHeader *entry, LogBufferHeader *header, char *buf, int buf_len)
+{
+  if (entry == nullptr || header == nullptr || buf == nullptr || buf_len <= 0) {
+    return -1;
+  }
+
+  // [seg_start, seg_end) is the only readable region (byte_count == segment size).
+  char *seg_start = reinterpret_cast<char *>(header);
+  char *seg_end   = seg_start + header->byte_count;
+
+  // v3 decode needs the field-type schema and the symbols.
+  char *schema_blob = header->fmt_fieldtypes();
+  char *symbols     = header->fmt_fieldlist();
+  if (schema_blob == nullptr || symbols == nullptr) {
+    return -1;
+  }
+  // Both must lie within the segment, including the fixed schema prefix.
+  if (schema_blob < seg_start || schema_blob + sizeof(LogFieldTypeSchema) > seg_end || symbols < seg_start || symbols >= seg_end) {
+    return -1;
+  }
+
+  // field_count is a uint16 that may sit at an unaligned offset (the writer
+  // places the schema right after the NUL-terminated header strings), so read
+  // it byte-wise rather than through the struct.
+  uint16_t fc16 = 0;
+  memcpy(&fc16, schema_blob, sizeof(fc16));
+  const unsigned field_count = fc16;
+  const uint8_t *codes       = reinterpret_cast<const uint8_t *>(schema_blob) + sizeof(LogFieldTypeSchema);
+  if (reinterpret_cast<const char *>(codes) + field_count > seg_end) {
+    return -1;
+  }
+
+  // field_count must match the symbol count; a mismatch is a corrupt segment.
+  unsigned symbol_count = 0;
+  bool     in_token     = false;
+  for (const char *p = symbols; p < seg_end && *p != '\0'; ++p) {
+    if (is_field_sep(*p)) {
+      in_token = false;
+    } else if (!in_token) {
+      in_token = true;
+      ++symbol_count;
+    }
+  }
+  if (symbol_count != field_count) {
+    return -1;
+  }
+
+  // Clamp value reads to this entry (or the segment, if entry_len is bogus).
+  char *entry_start = reinterpret_cast<char *>(entry);
+  if (entry_start < seg_start || entry_start + sizeof(LogEntryHeader) > seg_end) {
+    return -1;
+  }
+  char *read_from = entry_start + sizeof(LogEntryHeader);
+  char *read_end  = entry_start + entry->entry_len;
+  if (read_end < read_from || read_end > seg_end) {
+    read_end = seg_end;
+  }
+
+  int written = 0;
+
+  // Append n bytes, keeping one byte in reserve for the trailing NUL.
+  auto put = [&](const char *src, int n) -> bool {
+    if (n < 0 || written + n >= buf_len) {
+      return false;
+    }
+    memcpy(buf + written, src, n);
+    written += n;
+    return true;
+  };
+  auto put_ch = [&](char c) -> bool { return put(&c, 1); };
+
+  if (!put_ch('{')) {
+    return -1;
+  }
+
+  const char *sym = symbols;
+  for (unsigned i = 0; i < field_count; ++i) {
+    // Next symbol token (comma-separated in fmt_fieldlist), bounded by the segment.
+    while (sym < seg_end && is_field_sep(*sym)) {
+      ++sym;
+    }
+    const char *sym_start = sym;
+    while (sym < seg_end && *sym != '\0' && !is_field_sep(*sym)) {
+      ++sym;
+    }
+    int sym_len = static_cast<int>(sym - sym_start);
+
+    if (i > 0 && !put_ch(',')) {
+      return -1;
+    }
+    if (!put_ch('"') || !put(sym_start, sym_len) || !put_ch('"') || !put_ch(':')) {
+      return -1;
+    }

Review Comment:
   JSON keys are emitted from fmt_fieldlist without any escaping/validation. A malformed or hostile segment whose symbol contains '"', '\\', or control characters will produce invalid (or structurally different) JSON output. Since the segment is treated as untrusted, reject symbols containing non [A-Za-z0-9_] bytes (or escape them) before writing.



##########
doc/developer-guide/logging-architecture/binary-log-v3-format.en.rst:
##########
@@ -0,0 +1,216 @@
+.. Licensed to the Apache Software Foundation (ASF) under one
+   or more contributor license agreements.  See the NOTICE file
+   distributed with this work for additional information
+   regarding copyright ownership.  The ASF licenses this file
+   to you under the Apache License, Version 2.0 (the
+   "License"); you may not use this file except in compliance
+   with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing,
+   software distributed under the License is distributed on an
+   "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+   KIND, either express or implied.  See the License for the
+   specific language governing permissions and limitations
+   under the License.
+
+.. include:: ../../common.defs
+
+.. _binary-log-v3-format:
+
+Self-Describing Binary Log Format (v3)
+**************************************
+
+This page specifies the on-disk format of a binary log segment, version 3, in
+enough detail to implement a decoder *without* the Traffic Server source tree.
+A version 3 segment is **self-describing**: every field's type is published in
+the segment header, so a generic reader can decode each entry by dispatching on
+a small, stable set of type codes — no embedded copy of the ATS symbol-to-type
+table is required.
+
+Motivation
+==========
+
+In version 2, a segment header carries the field *symbols* (``fmt_fieldlist``,
+e.g. ``"chi cqu pssc"``) and a printf-style *template* (``fmt_printf``) but
+**not** the field types. To decode an entry a reader had to already know the
+type of each symbol, because the value encodings are only self-delimiting once
+the type is known (``IP`` is variable length, for example). That coupled every
+out-of-tree parser to the exact ATS build that wrote the log.
+
+Version 3 adds one thing: a per-segment **field-type schema** that lists the
+wire type of every field, in field order. Decoding then needs only the symbols
+(as keys) and the schema (for types).
+
+Segment layout
+==============
+
+A ``.blog`` file is a stream of segments, each a serialized ``LogBuffer``:
+
+::
+
+    LogBufferHeader            (per segment)
+      cookie       = 0xaceface
+      version      = 3
+      format_type, byte_count, entry_count, timestamps, flags, signature
+      fmt_name_offset
+      fmt_fieldlist_offset      -> "chi cqu pssc ..."   (symbols, space separated)
+      fmt_printf_offset         -> "%<chi> %<cqu> ..."
+      src_hostname_offset, log_filename_offset
+      data_offset               -> first entry
+      fmt_fieldtypes_offset     -> field-type schema     (NEW in v3)
+    [ LogEntryHeader | field0 field1 field2 ... ]   x entry_count
+      LogEntryHeader: timestamp(8) timestamp_usec(4) entry_len(4)
+      fields: concatenated in fieldlist order, no per-field tags
+
+All ``*_offset`` members are byte offsets from the start of the segment (the
+address of the ``LogBufferHeader``). ``fmt_fieldtypes_offset`` is appended
+**after** ``data_offset`` so that the layout through ``data_offset`` is
+byte-identical to version 2; a value of ``0`` means the schema is absent (e.g.
+a text-format segment, or a version 2 segment).
+
+Field-type schema
+=================
+
+At ``fmt_fieldtypes_offset`` the segment stores:
+
+::
+
+    uint16_t field_count;             // == number of symbols in fmt_fieldlist
+    uint8_t  type_code[field_count];  // one type code per field, in order
+
+``type_code[i]`` is the type of the i-th field, which corresponds to the i-th
+symbol in ``fmt_fieldlist`` and the i-th value in each entry. The ``uint16_t``
+``field_count`` prefix is written in **host byte order**, like the rest of
+``LogBufferHeader``. The blob is padded along with the header to an 8-byte
+boundary.
+
+The schema carries no independent version of its own: the segment ``version``
+(``3`` here) governs this layout, so a future schema change rides the same
+``LOG_SEGMENT_VERSION`` bump rather than a second, separate counter.
+
+Stable type codes
+=================
+
+The type codes are the values of the in-tree ``LogField::Type`` enumeration,
+serialized directly. They are part of the published format and are
+**append-only**: codes are never renumbered or reused.
+
+==== ========= ===========================================================
+Code Name      Wire encoding
+==== ========= ===========================================================
+0    INVALID   Reserved. Not emitted by a correct writer; a reader that
+               meets it -- or any code it does not recognize -- cannot
+               determine the field length and must stop decoding the entry.
+1    sINT      A single ``int64_t``, fixed 8 bytes, **host byte order**.
+2    dINT      Two ``int64_t`` (16 bytes), host byte order. Used for
+               values stored as two integers, e.g. HTTP version
+               major/minor.
+3    STRING    NUL-terminated bytes, then padded to an 8-byte boundary.
+4    IP        ``uint16_t`` address family followed by a family-sized
+               address, then padded to an 8-byte boundary (see below).
+==== ========= ===========================================================
+
+The code reflects how the value is *framed* on disk, i.e. how a reader walks
+(or skips) it -- not what the value means. (The ``sINT``/``dINT`` names are an
+ATS-internal distinction; on the wire ``sINT`` is one 8-byte integer and
+``dINT`` is two consecutive ones.) How a consumer *renders* a value -- mapping
+a cache-result integer to ``TCP_HIT``, or a ``dINT`` to ``1.1`` -- is layered
+on top by the consumer and is not part of the wire format.
+
+Value encodings
+===============
+
+sINT
+    An ``int64_t`` occupying exactly 8 bytes, in **host byte order** (as in
+    version 2). Integer values are not endianness-normalized, so a ``.blog`` is
+    not portable across hosts of differing endianness; cross-architecture
+    portability is future work.
+
+dINT
+    Two consecutive ``sINT`` values: 16 bytes total, in host byte order. Used
+    where one log field is stored as two integers, such as an HTTP version
+    (major then minor). The reference decoder renders it as a JSON array, e.g.
+    ``[1,1]``; turning that into ``1.1`` is a consumer concern.
+
+STRING
+    The string bytes followed by a single NUL, then zero padding up to the next
+    8-byte boundary. The on-wire length is therefore
+    ``align_up(strlen + 1, 8)``. An empty/absent string is written as ``"-"``.
+
+IP
+    A ``uint16_t`` address family in host byte order, then:
+
+    .. list-table::
+       :header-rows: 1
+       :widths: 30 70
+
+       * - Family
+         - Following bytes
+       * - ``AF_INET`` (IPv4)
+         - 4-byte ``in_addr``
+       * - ``AF_INET6`` (IPv6)
+         - 16-byte ``in6_addr``
+       * - ``AF_UNIX``
+         - fixed-size path buffer
+       * - ``AF_UNSPEC`` / other
+         - no address bytes
+
+    The whole field is padded to the next 8-byte boundary. Because the length
+    depends on the family byte *inside* the value, only a reader that knows the
+    field is an ``IP`` (from the schema) can compute its size — which is exactly
+    why the schema is required to skip or decode unknown fields safely.
+
+Decoding an entry
+=================
+
+Given a segment, a generic decoder:
+
+#. Reads ``field_count`` and the ``type_code[]`` array from the schema at
+   ``fmt_fieldtypes_offset``.
+#. Splits ``fmt_fieldlist`` into ``field_count`` whitespace-separated symbols.

Review Comment:
   fmt_fieldlist is comma-separated on disk (with optional spaces), not strictly whitespace-separated (LogFormat::parse_format_string inserts ',' between symbols). The decoding algorithm step should match the actual delimiter so out-of-tree readers implement the same split logic as traffic_logcat/log_entry_to_json().



##########
src/proxy/logging/Log.cc:
##########
@@ -328,371 +328,389 @@ Log::init_fields()
   LogField::init_milestone_container();
 
   // client -> proxy fields
-  field = new LogField("client_host_ip", "chi", LogField::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_str);
+  field =
+    new LogField("client_host_ip", "chi", LogField::Type::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chi", field);
 
-  field =
-    new LogField("client_host_port", "chp", LogField::sINT, &LogAccess::marshal_client_host_port, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("client_host_port", "chp", LogField::Type::sINT, &LogAccess::marshal_client_host_port,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chp", field);
 
-  field =
-    new LogField("client_host_ip_hex", "chih", LogField::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_hex);
+  field = new LogField("client_host_ip_hex", "chih", LogField::Type::IP, &LogAccess::marshal_client_host_ip,
+                       &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chih", field);
 
-  field = new LogField("client_host_ip_verified", "chiv", LogField::IP, &LogAccess::marshal_client_host_ip_verified,
+  field = new LogField("client_host_ip_verified", "chiv", LogField::Type::IP, &LogAccess::marshal_client_host_ip_verified,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chiv", field);
 
   // remote client (Not necessarily the requesting client IP - See proxy protocol)
-  field = new LogField("remote_host_ip", "rchi", LogField::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_str);
+  field =
+    new LogField("remote_host_ip", "rchi", LogField::Type::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchi", field);
 
-  field = new LogField("remote_host_port", "rchp", LogField::sINT, &LogAccess::marshal_remote_host_port,
+  field = new LogField("remote_host_port", "rchp", LogField::Type::sINT, &LogAccess::marshal_remote_host_port,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchp", field);
 
-  field =
-    new LogField("remote_host_ip_hex", "rchh", LogField::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_hex);
+  field = new LogField("remote_host_ip_hex", "rchh", LogField::Type::IP, &LogAccess::marshal_remote_host_ip,
+                       &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchh", field);
 
   // interface ip
 
-  field =
-    new LogField("host_interface_ip", "hii", LogField::IP, &LogAccess::marshal_host_interface_ip, &LogAccess::unmarshal_ip_to_str);
+  field = new LogField("host_interface_ip", "hii", LogField::Type::IP, &LogAccess::marshal_host_interface_ip,
+                       &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("hii", field);
 
-  field = new LogField("host_interface_ip_hex", "hiih", LogField::IP, &LogAccess::marshal_host_interface_ip,
+  field = new LogField("host_interface_ip_hex", "hiih", LogField::Type::IP, &LogAccess::marshal_host_interface_ip,
                        &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("hiih", field);
   // interface ip end
-  field = new LogField("client_auth_user_name", "caun", LogField::STRING, &LogAccess::marshal_client_auth_user_name,
+  field = new LogField("client_auth_user_name", "caun", LogField::Type::STRING, &LogAccess::marshal_client_auth_user_name,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("caun", field);
 
-  field = new LogField("plugin_identity_id", "piid", LogField::sINT, &LogAccess::marshal_plugin_identity_id,
+  field = new LogField("plugin_identity_id", "piid", LogField::Type::sINT, &LogAccess::marshal_plugin_identity_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("piid", field);
 
-  field = new LogField("plugin_identity_tag", "pitag", LogField::STRING, &LogAccess::marshal_plugin_identity_tag,
+  field = new LogField("plugin_identity_tag", "pitag", LogField::Type::STRING, &LogAccess::marshal_plugin_identity_tag,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pitag", field);
 
-  field = new LogField("client_req_timestamp_sec", "cqts", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_sec", "cqts", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqts", field);
 
-  field = new LogField("client_req_timestamp_hex_sec", "cqth", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_hex_sec", "cqth", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_str_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqth", field);
 
-  field = new LogField("client_req_timestamp_squid", "cqtq", LogField::sINT, &LogAccess::marshal_client_req_timestamp_ms,
+  field = new LogField("client_req_timestamp_squid", "cqtq", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_ms,
                        &LogAccess::unmarshal_ttmsf);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtq", field);
 
-  field = new LogField("client_req_timestamp_netscape", "cqtn", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_netscape", "cqtn", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_netscape_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtn", field);
 
-  field = new LogField("client_req_timestamp_date", "cqtd", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_date", "cqtd", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_date_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtd", field);
 
-  field = new LogField("client_req_timestamp_time", "cqtt", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_time", "cqtt", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_time_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtt", field);
 
-  field = new LogField("client_req_http_method", "cqhm", LogField::STRING, &LogAccess::marshal_client_req_http_method,
+  field = new LogField("client_req_http_method", "cqhm", LogField::Type::STRING, &LogAccess::marshal_client_req_http_method,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqhm", field);
 
-  field = new LogField("client_req_url", "cqu", LogField::STRING, &LogAccess::marshal_client_req_url, &LogAccess::unmarshal_str,
-                       &LogAccess::set_client_req_url);
+  field = new LogField("client_req_url", "cqu", LogField::Type::STRING, &LogAccess::marshal_client_req_url,
+                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_url);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqu", field);
 
-  field = new LogField("client_req_url", "pqu", LogField::STRING, &LogAccess::marshal_client_req_url, &LogAccess::unmarshal_str,
-                       &LogAccess::set_client_req_url);
+  field = new LogField("client_req_url", "pqu", LogField::Type::STRING, &LogAccess::marshal_client_req_url,
+                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_url);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqu", field);
 
-  field = new LogField("client_req_url_canonical", "cquc", LogField::STRING, &LogAccess::marshal_client_req_url_canon,
+  field = new LogField("client_req_url_canonical", "cquc", LogField::Type::STRING, &LogAccess::marshal_client_req_url_canon,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquc", field);
 
-  field = new LogField("client_req_url_canonical", "pquc", LogField::STRING, &LogAccess::marshal_client_req_url_canon,
+  field = new LogField("client_req_url_canonical", "pquc", LogField::Type::STRING, &LogAccess::marshal_client_req_url_canon,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pquc", field);
 
-  field =
-    new LogField("client_req_unmapped_url_canonical", "cquuc", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_canon,
-                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_canon);
+  field = new LogField("client_req_unmapped_url_canonical", "cquuc", LogField::Type::STRING,
+                       &LogAccess::marshal_client_req_unmapped_url_canon, &LogAccess::unmarshal_str,
+                       &LogAccess::set_client_req_unmapped_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquuc", field);
 
-  field = new LogField("client_req_unmapped_url_path", "cquup", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_path,
-                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_path);
+  field =
+    new LogField("client_req_unmapped_url_path", "cquup", LogField::Type::STRING, &LogAccess::marshal_client_req_unmapped_url_path,
+                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquup", field);
 
-  field = new LogField("client_req_unmapped_url_host", "cquuh", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_host,
-                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_host);
+  field =
+    new LogField("client_req_unmapped_url_host", "cquuh", LogField::Type::STRING, &LogAccess::marshal_client_req_unmapped_url_host,
+                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_host);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquuh", field);
 
-  field = new LogField("client_req_url_scheme", "cqus", LogField::STRING, &LogAccess::marshal_client_req_url_scheme,
+  field = new LogField("client_req_url_scheme", "cqus", LogField::Type::STRING, &LogAccess::marshal_client_req_url_scheme,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqus", field);
 
-  field = new LogField("client_req_url_scheme", "pqus", LogField::STRING, &LogAccess::marshal_client_req_url_scheme,
+  field = new LogField("client_req_url_scheme", "pqus", LogField::Type::STRING, &LogAccess::marshal_client_req_url_scheme,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqus", field);
 
-  field = new LogField("client_req_url_path", "cqup", LogField::STRING, &LogAccess::marshal_client_req_url_path,
+  field = new LogField("client_req_url_path", "cqup", LogField::Type::STRING, &LogAccess::marshal_client_req_url_path,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqup", field);
 
-  field = new LogField("client_req_url_path", "pqup", LogField::STRING, &LogAccess::marshal_client_req_url_path,
+  field = new LogField("client_req_url_path", "pqup", LogField::Type::STRING, &LogAccess::marshal_client_req_url_path,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqup", field);
 
-  field = new LogField("client_req_protocol_version", "cqpv", LogField::STRING, &LogAccess::marshal_client_req_protocol_version,
-                       &LogAccess::unmarshal_str);
+  field = new LogField("client_req_protocol_version", "cqpv", LogField::Type::STRING,
+                       &LogAccess::marshal_client_req_protocol_version, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqpv", field);
 
-  field = new LogField("server_req_protocol_version", "sqpv", LogField::STRING, &LogAccess::marshal_server_req_protocol_version,
-                       &LogAccess::unmarshal_str);
+  field = new LogField("server_req_protocol_version", "sqpv", LogField::Type::STRING,
+                       &LogAccess::marshal_server_req_protocol_version, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("sqpv", field);
 
-  field = new LogField("client_req_header_len", "cqhl", LogField::sINT, &LogAccess::marshal_client_req_header_len,
+  field = new LogField("client_req_header_len", "cqhl", LogField::Type::sINT, &LogAccess::marshal_client_req_header_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqhl", field);
 
-  field = new LogField("client_req_squid_len", "cqql", LogField::sINT, &LogAccess::marshal_client_req_squid_len,
+  field = new LogField("client_req_squid_len", "cqql", LogField::Type::sINT, &LogAccess::marshal_client_req_squid_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqql", field);
 
   // Client request squid length plus TLS handshake bytes received for TLS connections
-  field = new LogField("client_req_squid_len_tls", "cqqtl", LogField::sINT, &LogAccess::marshal_client_req_squid_len_tls,
+  field = new LogField("client_req_squid_len_tls", "cqqtl", LogField::Type::sINT, &LogAccess::marshal_client_req_squid_len_tls,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqqtl", field);
 
-  field = new LogField("cache_lookup_url_canonical", "cluc", LogField::STRING, &LogAccess::marshal_cache_lookup_url_canon,
+  // Client request squid length plus TLS handshake bytes received for TLS connections
+  field = new LogField("client_req_squid_len_tls", "cqqtl", LogField::Type::sINT, &LogAccess::marshal_client_req_squid_len_tls,
+                       &LogAccess::unmarshal_int_to_str);
+  global_field_list.add(field, false);
+  field_symbol_hash.emplace("cqqtl", field);
+
+  field = new LogField("cache_lookup_url_canonical", "cluc", LogField::Type::STRING, &LogAccess::marshal_cache_lookup_url_canon,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cluc", field);
 
-  field = new LogField("cache_key_hash", "ckh", LogField::STRING, &LogAccess::marshal_cache_key_hash, &LogAccess::unmarshal_str);
+  field =
+    new LogField("cache_key_hash", "ckh", LogField::Type::STRING, &LogAccess::marshal_cache_key_hash, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ckh", field);
 
-  field = new LogField("client_sni_server_name", "cssn", LogField::STRING, &LogAccess::marshal_client_sni_server_name,
+  field = new LogField("client_sni_server_name", "cssn", LogField::Type::STRING, &LogAccess::marshal_client_sni_server_name,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cssn", field);
 
-  field = new LogField("client_ssl_cert_provided", "cscert", LogField::sINT, &LogAccess::marshal_client_provided_cert,
+  field = new LogField("client_ssl_cert_provided", "cscert", LogField::Type::sINT, &LogAccess::marshal_client_provided_cert,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cscert", field);
 
-  field = new LogField("proxy_ssl_cert_provided", "pscert", LogField::sINT, &LogAccess::marshal_proxy_provided_cert,
+  field = new LogField("proxy_ssl_cert_provided", "pscert", LogField::Type::sINT, &LogAccess::marshal_proxy_provided_cert,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pscert", field);
 
-  field = new LogField("process_uuid", "puuid", LogField::STRING, &LogAccess::marshal_process_uuid, &LogAccess::unmarshal_str);
+  field =
+    new LogField("process_uuid", "puuid", LogField::Type::STRING, &LogAccess::marshal_process_uuid, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("puuid", field);
 
-  field =
-    new LogField("process_snowflake_id", "psfid", LogField::STRING, &LogAccess::marshal_process_sfid, &LogAccess::unmarshal_str);
+  field = new LogField("process_snowflake_id", "psfid", LogField::Type::STRING, &LogAccess::marshal_process_sfid,
+                       &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("psfid", field);
 
-  field = new LogField("client_req_content_len", "cqcl", LogField::sINT, &LogAccess::marshal_client_req_content_len,
+  field = new LogField("client_req_content_len", "cqcl", LogField::Type::sINT, &LogAccess::marshal_client_req_content_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqcl", field);
 
-  field = new LogField("client_req_tcp_reused", "cqtr", LogField::sINT, &LogAccess::marshal_client_req_tcp_reused,
+  field = new LogField("client_req_tcp_reused", "cqtr", LogField::Type::sINT, &LogAccess::marshal_client_req_tcp_reused,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtr", field);
 
-  field = new LogField("client_req_is_ssl", "cqssl", LogField::sINT, &LogAccess::marshal_client_req_is_ssl,
+  field = new LogField("client_req_is_ssl", "cqssl", LogField::Type::sINT, &LogAccess::marshal_client_req_is_ssl,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssl", field);
 
-  field = new LogField("client_req_ssl_reused", "cqssr", LogField::sINT, &LogAccess::marshal_client_req_ssl_reused,
+  field = new LogField("client_req_ssl_reused", "cqssr", LogField::Type::sINT, &LogAccess::marshal_client_req_ssl_reused,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssr", field);
 
-  field = new LogField("client_req_is_internal", "cqint", LogField::sINT, &LogAccess::marshal_client_req_is_internal,
+  field = new LogField("client_req_is_internal", "cqint", LogField::Type::sINT, &LogAccess::marshal_client_req_is_internal,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqint", field);
 
-  field = new LogField("client_req_mptcp", "cqmpt", LogField::sINT, &LogAccess::marshal_client_req_mptcp_state,
+  field = new LogField("client_req_mptcp", "cqmpt", LogField::Type::sINT, &LogAccess::marshal_client_req_mptcp_state,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqmpt", field);
 
-  field = new LogField("client_sec_protocol", "cqssv", LogField::STRING, &LogAccess::marshal_client_security_protocol,
+  field = new LogField("client_sec_protocol", "cqssv", LogField::Type::STRING, &LogAccess::marshal_client_security_protocol,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssv", field);
 
-  field = new LogField("client_cipher_suite", "cqssc", LogField::STRING, &LogAccess::marshal_client_security_cipher_suite,
+  field = new LogField("client_cipher_suite", "cqssc", LogField::Type::STRING, &LogAccess::marshal_client_security_cipher_suite,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssc", field);
 
-  field =
-    new LogField("client_curve", "cqssu", LogField::STRING, &LogAccess::marshal_client_security_curve, &LogAccess::unmarshal_str);
+  field = new LogField("client_curve", "cqssu", LogField::Type::STRING, &LogAccess::marshal_client_security_curve,
+                       &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssu", field);
 
-  field =
-    new LogField("client_group", "cqssg", LogField::STRING, &LogAccess::marshal_client_security_group, &LogAccess::unmarshal_str);
+  field = new LogField("client_group", "cqssg", LogField::Type::STRING, &LogAccess::marshal_client_security_group,
+                       &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssg", field);
 
-  field =
-    new LogField("client_sec_alpn", "cqssa", LogField::STRING, &LogAccess::marshal_client_security_alpn, &LogAccess::unmarshal_str);
+  field = new LogField("client_sec_alpn", "cqssa", LogField::Type::STRING, &LogAccess::marshal_client_security_alpn,
+                       &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssa", field);
 
   // TLS handshake bytes - bytes received from client during TLS handshake
-  field = new LogField("client_tls_handshake_bytes_rx", "cthbr", LogField::sINT, &LogAccess::marshal_client_tls_handshake_bytes_rx,
-                       &LogAccess::unmarshal_int_to_str);
+  field = new LogField("client_tls_handshake_bytes_rx", "cthbr", LogField::Type::sINT,
+                       &LogAccess::marshal_client_tls_handshake_bytes_rx, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cthbr", field);
 
   // TLS handshake bytes - bytes sent to client during TLS handshake
-  field = new LogField("client_tls_handshake_bytes_tx", "cthbt", LogField::sINT, &LogAccess::marshal_client_tls_handshake_bytes_tx,
-                       &LogAccess::unmarshal_int_to_str);
+  field = new LogField("client_tls_handshake_bytes_tx", "cthbt", LogField::Type::sINT,
+                       &LogAccess::marshal_client_tls_handshake_bytes_tx, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cthbt", field);
 
   // TLS handshake bytes - total (rx + tx) during TLS handshake
-  field = new LogField("client_tls_handshake_bytes", "cthb", LogField::sINT, &LogAccess::marshal_client_tls_handshake_bytes,
+  field = new LogField("client_tls_handshake_bytes", "cthb", LogField::Type::sINT, &LogAccess::marshal_client_tls_handshake_bytes,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cthb", field);
 
   Ptr<LogFieldAliasTable> finish_status_map = make_ptr(new LogFieldAliasTable);
   finish_status_map->init(N_LOG_FINISH_CODE_TYPES, LOG_FINISH_FIN, "FIN", LOG_FINISH_INTR, "INTR", LOG_FINISH_TIMEOUT, "TIMEOUT");
 
-  field = new LogField("client_finish_status_code", "cfsc", LogField::sINT, &LogAccess::marshal_client_finish_status_code,
+  field = new LogField("client_finish_status_code", "cfsc", LogField::Type::sINT, &LogAccess::marshal_client_finish_status_code,
                        &LogAccess::unmarshal_finish_status, make_alias_map(finish_status_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cfsc", field);
 
-  field =
-    new LogField("client_req_id", "crid", LogField::sINT, &LogAccess::marshal_client_req_id, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("client_req_id", "crid", LogField::Type::sINT, &LogAccess::marshal_client_req_id,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crid", field);
 
-  field =
-    new LogField("client_req_uuid", "cruuid", LogField::STRING, &LogAccess::marshal_client_req_uuid, &LogAccess::unmarshal_str);
+  field = new LogField("client_req_uuid", "cruuid", LogField::Type::STRING, &LogAccess::marshal_client_req_uuid,
+                       &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cruuid", field);
 
-  field = new LogField("client_rx_error_code", "crec", LogField::STRING, &LogAccess::marshal_client_rx_error_code,
+  field = new LogField("client_rx_error_code", "crec", LogField::Type::STRING, &LogAccess::marshal_client_rx_error_code,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crec", field);
 
-  field = new LogField("client_tx_error_code", "ctec", LogField::STRING, &LogAccess::marshal_client_tx_error_code,
+  field = new LogField("client_tx_error_code", "ctec", LogField::Type::STRING, &LogAccess::marshal_client_tx_error_code,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctec", field);
 
-  field = new LogField("client_request_all_header_fields", "cqah", LogField::STRING,
+  field = new LogField("client_request_all_header_fields", "cqah", LogField::Type::STRING,
                        &LogAccess::marshal_client_req_all_header_fields, &LogUtils::unmarshalMimeHdr);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqah", field);
 
   // proxy -> client fields
-  field = new LogField("proxy_resp_content_type", "psct", LogField::STRING, &LogAccess::marshal_proxy_resp_content_type,
+  field = new LogField("proxy_resp_content_type", "psct", LogField::Type::STRING, &LogAccess::marshal_proxy_resp_content_type,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("psct", field);
 
-  field = new LogField("proxy_resp_reason_phrase", "prrp", LogField::STRING, &LogAccess::marshal_proxy_resp_reason_phrase,
+  field = new LogField("proxy_resp_reason_phrase", "prrp", LogField::Type::STRING, &LogAccess::marshal_proxy_resp_reason_phrase,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("prrp", field);
 
-  field = new LogField("proxy_resp_squid_len", "psql", LogField::sINT, &LogAccess::marshal_proxy_resp_squid_len,
+  field = new LogField("proxy_resp_squid_len", "psql", LogField::Type::sINT, &LogAccess::marshal_proxy_resp_squid_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("psql", field);
 
   // Squid length plus TLS handshake bytes sent for TLS connections
-  field = new LogField("proxy_resp_squid_len_tls", "psqtl", LogField::sINT, &LogAccess::marshal_proxy_resp_squid_len_tls,
+  field = new LogField("proxy_resp_squid_len_tls", "psqtl", LogField::Type::sINT, &LogAccess::marshal_proxy_resp_squid_len_tls,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("psqtl", field);
 
-  field = new LogField("proxy_resp_content_len", "pscl", LogField::sINT, &LogAccess::marshal_proxy_resp_content_len,
+  // Squid length plus TLS handshake bytes sent for TLS connections
+  field = new LogField("proxy_resp_squid_len_tls", "psqtl", LogField::Type::sINT, &LogAccess::marshal_proxy_resp_squid_len_tls,
+                       &LogAccess::unmarshal_int_to_str);
+  global_field_list.add(field, false);
+  field_symbol_hash.emplace("psqtl", field);

Review Comment:
   The "psqtl" (proxy_resp_squid_len_tls) field is registered twice. This creates duplicate LogField entries and the second field_symbol_hash.emplace() will not insert due to the existing key, leaving the global field list inconsistent.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#discussion_r3379224175


##########
src/proxy/logging/Log.cc:
##########
@@ -740,58 +758,59 @@ Log::init_fields()
                            SQUID_MISS_ERROR, "MISS_ERROR", SQUID_MISS_CACHE_BYPASS, "MISS_CACHE_BYPASS",
                            SQUID_HIT_MISS_INVALID_ASSIGNED_CODE, "INVALID_CODE");
 
-  field = new LogField("cache_result_code", "crc", LogField::sINT, &LogAccess::marshal_cache_result_code,
+  field = new LogField("cache_result_code", "crc", LogField::Type::sINT, &LogAccess::marshal_cache_result_code,
                        &LogAccess::unmarshal_cache_code, make_alias_map(cache_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crc", field);
 
   // Reuse the unmarshalling code from crc
-  field = new LogField("cache_result_subcode", "crsc", LogField::sINT, &LogAccess::marshal_cache_result_subcode,
+  field = new LogField("cache_result_subcode", "crsc", LogField::Type::sINT, &LogAccess::marshal_cache_result_subcode,
                        &LogAccess::unmarshal_cache_code, make_alias_map(cache_subcode_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crsc", field);
 
-  field = new LogField("cache_hit_miss", "chm", LogField::sINT, &LogAccess::marshal_cache_hit_miss,
+  field = new LogField("cache_hit_miss", "chm", LogField::Type::sINT, &LogAccess::marshal_cache_hit_miss,
                        &LogAccess::unmarshal_cache_hit_miss, make_alias_map(cache_hit_miss_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chm", field);
 
-  field = new LogField("proxy_response_all_header_fields", "psah", LogField::STRING,
+  field = new LogField("proxy_response_all_header_fields", "psah", LogField::Type::STRING,
                        &LogAccess::marshal_proxy_resp_all_header_fields, &LogUtils::unmarshalMimeHdr);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("psah", field);
 
   // proxy -> server fields
-  field = new LogField("proxy_req_header_len", "pqhl", LogField::sINT, &LogAccess::marshal_proxy_req_header_len,
+  field = new LogField("proxy_req_header_len", "pqhl", LogField::Type::sINT, &LogAccess::marshal_proxy_req_header_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqhl", field);
 
-  field = new LogField("proxy_req_squid_len", "pqql", LogField::sINT, &LogAccess::marshal_proxy_req_squid_len,
+  field = new LogField("proxy_req_squid_len", "pqql", LogField::Type::sINT, &LogAccess::marshal_proxy_req_squid_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqql", field);
 
-  field = new LogField("proxy_req_content_len", "pqcl", LogField::sINT, &LogAccess::marshal_proxy_req_content_len,
+  field = new LogField("proxy_req_content_len", "pqcl", LogField::Type::sINT, &LogAccess::marshal_proxy_req_content_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqcl", field);
 
-  field = new LogField("proxy_req_server_ip", "pqsi", LogField::IP, &LogAccess::marshal_proxy_req_server_ip,
+  field = new LogField("proxy_req_server_ip", "pqsi", LogField::Type::IP, &LogAccess::marshal_proxy_req_server_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqsi", field);
 
-  field = new LogField("proxy_req_server_port", "pqsp", LogField::sINT, &LogAccess::marshal_proxy_req_server_port,
+  field = new LogField("proxy_req_server_port", "pqsp", LogField::Type::sINT, &LogAccess::marshal_proxy_req_server_port,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqsp", field);
 
-  field = new LogField("next_hop_ip", "nhi", LogField::IP, &LogAccess::marshal_next_hop_ip, &LogAccess::unmarshal_ip_to_str);
+  field = new LogField("next_hop_ip", "nhi", LogField::Type::IP, &LogAccess::marshal_next_hop_ip, &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("nhi", field);
 
-  field = new LogField("next_hop_port", "nhp", LogField::IP, &LogAccess::marshal_next_hop_port, &LogAccess::unmarshal_int_to_str);
+  field =
+    new LogField("next_hop_port", "nhp", LogField::Type::IP, &LogAccess::marshal_next_hop_port, &LogAccess::unmarshal_int_to_str);

Review Comment:
   "nhp" (next_hop_port) marshals an integer port (LogAccess::marshal_next_hop_port returns INK_MIN_ALIGN after marshal_int), so its LogField type must be sINT. Declaring it as IP will cause v3 schemas and type-driven consumers to mis-decode the field.



##########
src/proxy/logging/Log.cc:
##########
@@ -328,371 +328,389 @@ Log::init_fields()
   LogField::init_milestone_container();
 
   // client -> proxy fields
-  field = new LogField("client_host_ip", "chi", LogField::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_str);
+  field =
+    new LogField("client_host_ip", "chi", LogField::Type::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chi", field);
 
-  field =
-    new LogField("client_host_port", "chp", LogField::sINT, &LogAccess::marshal_client_host_port, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("client_host_port", "chp", LogField::Type::sINT, &LogAccess::marshal_client_host_port,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chp", field);
 
-  field =
-    new LogField("client_host_ip_hex", "chih", LogField::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_hex);
+  field = new LogField("client_host_ip_hex", "chih", LogField::Type::IP, &LogAccess::marshal_client_host_ip,
+                       &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chih", field);
 
-  field = new LogField("client_host_ip_verified", "chiv", LogField::IP, &LogAccess::marshal_client_host_ip_verified,
+  field = new LogField("client_host_ip_verified", "chiv", LogField::Type::IP, &LogAccess::marshal_client_host_ip_verified,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chiv", field);
 
   // remote client (Not necessarily the requesting client IP - See proxy protocol)
-  field = new LogField("remote_host_ip", "rchi", LogField::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_str);
+  field =
+    new LogField("remote_host_ip", "rchi", LogField::Type::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchi", field);
 
-  field = new LogField("remote_host_port", "rchp", LogField::sINT, &LogAccess::marshal_remote_host_port,
+  field = new LogField("remote_host_port", "rchp", LogField::Type::sINT, &LogAccess::marshal_remote_host_port,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchp", field);
 
-  field =
-    new LogField("remote_host_ip_hex", "rchh", LogField::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_hex);
+  field = new LogField("remote_host_ip_hex", "rchh", LogField::Type::IP, &LogAccess::marshal_remote_host_ip,
+                       &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchh", field);
 
   // interface ip
 
-  field =
-    new LogField("host_interface_ip", "hii", LogField::IP, &LogAccess::marshal_host_interface_ip, &LogAccess::unmarshal_ip_to_str);
+  field = new LogField("host_interface_ip", "hii", LogField::Type::IP, &LogAccess::marshal_host_interface_ip,
+                       &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("hii", field);
 
-  field = new LogField("host_interface_ip_hex", "hiih", LogField::IP, &LogAccess::marshal_host_interface_ip,
+  field = new LogField("host_interface_ip_hex", "hiih", LogField::Type::IP, &LogAccess::marshal_host_interface_ip,
                        &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("hiih", field);
   // interface ip end
-  field = new LogField("client_auth_user_name", "caun", LogField::STRING, &LogAccess::marshal_client_auth_user_name,
+  field = new LogField("client_auth_user_name", "caun", LogField::Type::STRING, &LogAccess::marshal_client_auth_user_name,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("caun", field);
 
-  field = new LogField("plugin_identity_id", "piid", LogField::sINT, &LogAccess::marshal_plugin_identity_id,
+  field = new LogField("plugin_identity_id", "piid", LogField::Type::sINT, &LogAccess::marshal_plugin_identity_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("piid", field);
 
-  field = new LogField("plugin_identity_tag", "pitag", LogField::STRING, &LogAccess::marshal_plugin_identity_tag,
+  field = new LogField("plugin_identity_tag", "pitag", LogField::Type::STRING, &LogAccess::marshal_plugin_identity_tag,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pitag", field);
 
-  field = new LogField("client_req_timestamp_sec", "cqts", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_sec", "cqts", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqts", field);
 
-  field = new LogField("client_req_timestamp_hex_sec", "cqth", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_hex_sec", "cqth", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_str_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqth", field);
 
-  field = new LogField("client_req_timestamp_squid", "cqtq", LogField::sINT, &LogAccess::marshal_client_req_timestamp_ms,
+  field = new LogField("client_req_timestamp_squid", "cqtq", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_ms,
                        &LogAccess::unmarshal_ttmsf);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtq", field);
 
-  field = new LogField("client_req_timestamp_netscape", "cqtn", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_netscape", "cqtn", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_netscape_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtn", field);
 
-  field = new LogField("client_req_timestamp_date", "cqtd", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_date", "cqtd", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_date_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtd", field);
 
-  field = new LogField("client_req_timestamp_time", "cqtt", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_time", "cqtt", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_time_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtt", field);
 
-  field = new LogField("client_req_http_method", "cqhm", LogField::STRING, &LogAccess::marshal_client_req_http_method,
+  field = new LogField("client_req_http_method", "cqhm", LogField::Type::STRING, &LogAccess::marshal_client_req_http_method,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqhm", field);
 
-  field = new LogField("client_req_url", "cqu", LogField::STRING, &LogAccess::marshal_client_req_url, &LogAccess::unmarshal_str,
-                       &LogAccess::set_client_req_url);
+  field = new LogField("client_req_url", "cqu", LogField::Type::STRING, &LogAccess::marshal_client_req_url,
+                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_url);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqu", field);
 
-  field = new LogField("client_req_url", "pqu", LogField::STRING, &LogAccess::marshal_client_req_url, &LogAccess::unmarshal_str,
-                       &LogAccess::set_client_req_url);
+  field = new LogField("client_req_url", "pqu", LogField::Type::STRING, &LogAccess::marshal_client_req_url,
+                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_url);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqu", field);
 
-  field = new LogField("client_req_url_canonical", "cquc", LogField::STRING, &LogAccess::marshal_client_req_url_canon,
+  field = new LogField("client_req_url_canonical", "cquc", LogField::Type::STRING, &LogAccess::marshal_client_req_url_canon,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquc", field);
 
-  field = new LogField("client_req_url_canonical", "pquc", LogField::STRING, &LogAccess::marshal_client_req_url_canon,
+  field = new LogField("client_req_url_canonical", "pquc", LogField::Type::STRING, &LogAccess::marshal_client_req_url_canon,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pquc", field);
 
-  field =
-    new LogField("client_req_unmapped_url_canonical", "cquuc", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_canon,
-                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_canon);
+  field = new LogField("client_req_unmapped_url_canonical", "cquuc", LogField::Type::STRING,
+                       &LogAccess::marshal_client_req_unmapped_url_canon, &LogAccess::unmarshal_str,
+                       &LogAccess::set_client_req_unmapped_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquuc", field);
 
-  field = new LogField("client_req_unmapped_url_path", "cquup", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_path,
-                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_path);
+  field =
+    new LogField("client_req_unmapped_url_path", "cquup", LogField::Type::STRING, &LogAccess::marshal_client_req_unmapped_url_path,
+                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquup", field);
 
-  field = new LogField("client_req_unmapped_url_host", "cquuh", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_host,
-                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_host);
+  field =
+    new LogField("client_req_unmapped_url_host", "cquuh", LogField::Type::STRING, &LogAccess::marshal_client_req_unmapped_url_host,
+                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_host);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquuh", field);
 
-  field = new LogField("client_req_url_scheme", "cqus", LogField::STRING, &LogAccess::marshal_client_req_url_scheme,
+  field = new LogField("client_req_url_scheme", "cqus", LogField::Type::STRING, &LogAccess::marshal_client_req_url_scheme,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqus", field);
 
-  field = new LogField("client_req_url_scheme", "pqus", LogField::STRING, &LogAccess::marshal_client_req_url_scheme,
+  field = new LogField("client_req_url_scheme", "pqus", LogField::Type::STRING, &LogAccess::marshal_client_req_url_scheme,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqus", field);
 
-  field = new LogField("client_req_url_path", "cqup", LogField::STRING, &LogAccess::marshal_client_req_url_path,
+  field = new LogField("client_req_url_path", "cqup", LogField::Type::STRING, &LogAccess::marshal_client_req_url_path,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqup", field);
 
-  field = new LogField("client_req_url_path", "pqup", LogField::STRING, &LogAccess::marshal_client_req_url_path,
+  field = new LogField("client_req_url_path", "pqup", LogField::Type::STRING, &LogAccess::marshal_client_req_url_path,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqup", field);
 
-  field = new LogField("client_req_protocol_version", "cqpv", LogField::STRING, &LogAccess::marshal_client_req_protocol_version,
-                       &LogAccess::unmarshal_str);
+  field = new LogField("client_req_protocol_version", "cqpv", LogField::Type::STRING,
+                       &LogAccess::marshal_client_req_protocol_version, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqpv", field);
 
-  field = new LogField("server_req_protocol_version", "sqpv", LogField::STRING, &LogAccess::marshal_server_req_protocol_version,
-                       &LogAccess::unmarshal_str);
+  field = new LogField("server_req_protocol_version", "sqpv", LogField::Type::STRING,
+                       &LogAccess::marshal_server_req_protocol_version, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("sqpv", field);
 
-  field = new LogField("client_req_header_len", "cqhl", LogField::sINT, &LogAccess::marshal_client_req_header_len,
+  field = new LogField("client_req_header_len", "cqhl", LogField::Type::sINT, &LogAccess::marshal_client_req_header_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqhl", field);
 
-  field = new LogField("client_req_squid_len", "cqql", LogField::sINT, &LogAccess::marshal_client_req_squid_len,
+  field = new LogField("client_req_squid_len", "cqql", LogField::Type::sINT, &LogAccess::marshal_client_req_squid_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqql", field);
 
   // Client request squid length plus TLS handshake bytes received for TLS connections
-  field = new LogField("client_req_squid_len_tls", "cqqtl", LogField::sINT, &LogAccess::marshal_client_req_squid_len_tls,
+  field = new LogField("client_req_squid_len_tls", "cqqtl", LogField::Type::sINT, &LogAccess::marshal_client_req_squid_len_tls,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqqtl", field);

Review Comment:
   Log field "cqqtl" is registered twice (duplicate LogField allocation, duplicate global_field_list entry, and duplicate field_symbol_hash key). The second registration leaks memory and can lead to inconsistent field iteration/lookup behavior.



##########
src/proxy/logging/Log.cc:
##########
@@ -977,118 +1000,119 @@ Log::init_fields()
   Ptr<LogFieldAliasTable> cache_write_code_map = make_ptr(new LogFieldAliasTable);
   cache_write_code_map->init(N_LOG_CACHE_WRITE_TYPES, LOG_CACHE_WRITE_NONE, "-", LOG_CACHE_WRITE_LOCK_MISSED, "WL_MISS",
                              LOG_CACHE_WRITE_LOCK_ABORTED, "INTR", LOG_CACHE_WRITE_ERROR, "ERR", LOG_CACHE_WRITE_COMPLETE, "FIN");
-  field = new LogField("cache_write_result", "cwr", LogField::sINT, &LogAccess::marshal_cache_write_code,
+  field = new LogField("cache_write_result", "cwr", LogField::Type::sINT, &LogAccess::marshal_cache_write_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwr", field);
 
-  field = new LogField("cache_write_transform_result", "cwtr", LogField::sINT, &LogAccess::marshal_cache_write_transform_code,
+  field = new LogField("cache_write_transform_result", "cwtr", LogField::Type::sINT, &LogAccess::marshal_cache_write_transform_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwtr", field);
 
   // other fields
 
-  field = new LogField("transfer_time_ms", "ttms", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms", "ttms", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttms", field);
 
-  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsh", field);
 
-  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_ttmsf);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsf", field);
 
-  field =
-    new LogField("transfer_time_sec", "tts", LogField::sINT, &LogAccess::marshal_transfer_time_s, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("transfer_time_sec", "tts", LogField::Type::sINT, &LogAccess::marshal_transfer_time_s,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("tts", field);
 
-  field = new LogField("file_size", "fsiz", LogField::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("file_size", "fsiz", LogField::Type::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("fsiz", field);
 
-  field = new LogField("client_connection_id", "ccid", LogField::sINT, &LogAccess::marshal_client_http_connection_id,
+  field = new LogField("client_connection_id", "ccid", LogField::Type::sINT, &LogAccess::marshal_client_http_connection_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ccid", field);
 
-  field = new LogField("client_transaction_id", "ctid", LogField::sINT, &LogAccess::marshal_client_http_transaction_id,
+  field = new LogField("client_transaction_id", "ctid", LogField::Type::sINT, &LogAccess::marshal_client_http_transaction_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctid", field);
 
-  field = new LogField("cache_read_retry_attempts", "crra", LogField::sINT, &LogAccess::marshal_cache_read_retries,
+  field = new LogField("cache_read_retry_attempts", "crra", LogField::Type::sINT, &LogAccess::marshal_cache_read_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crra", field);
 
-  field = new LogField("cache_write_retry_attempts", "cwra", LogField::sINT, &LogAccess::marshal_cache_write_retries,
+  field = new LogField("cache_write_retry_attempts", "cwra", LogField::Type::sINT, &LogAccess::marshal_cache_write_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwra", field);
 
-  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::sINT,
+  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::Type::sINT,
                        &LogAccess::marshal_cache_collapsed_connection_success, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cccs", field);
 
-  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::sINT,
+  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_weight, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpw", field);
 
-  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::sINT,
+  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_dependence, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpd", field);
 
-  field = new LogField("proxy_protocol_version", "ppv", LogField::STRING, &LogAccess::marshal_proxy_protocol_version,
+  field = new LogField("proxy_protocol_version", "ppv", LogField::Type::STRING, &LogAccess::marshal_proxy_protocol_version,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppv", field);
 
-  field = new LogField("proxy_protocol_src_ip", "pps", LogField::IP, &LogAccess::marshal_proxy_protocol_src_ip,
+  field = new LogField("proxy_protocol_src_ip", "pps", LogField::Type::IP, &LogAccess::marshal_proxy_protocol_src_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppsip", field);

Review Comment:
   field_symbol_hash key should match the field symbol ("pps"), otherwise log formats using %<pps> won't resolve this field. The current key "ppsip" makes the field effectively unreachable by its documented symbol.



##########
doc/appendices/command-line/traffic_logcat.en.rst:
##########
@@ -25,14 +25,17 @@ traffic_logcat
 Synopsis
 ========
 
-:program:`traffic_logcat` [-o output-file | -a] [-CEhSVw2] [input-file ...]
+:program:`traffic_logcat` [-o output-file | -a] [-CEHhSVw2] [input-file ...]

Review Comment:
   Synopsis flag list omits -j/--json even though it's documented and implemented below. This makes the top-level usage string inaccurate.



##########
doc/developer-guide/logging-architecture/binary-log-v3-format.en.rst:
##########
@@ -0,0 +1,216 @@
+.. Licensed to the Apache Software Foundation (ASF) under one
+   or more contributor license agreements.  See the NOTICE file
+   distributed with this work for additional information
+   regarding copyright ownership.  The ASF licenses this file
+   to you under the Apache License, Version 2.0 (the
+   "License"); you may not use this file except in compliance
+   with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing,
+   software distributed under the License is distributed on an
+   "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+   KIND, either express or implied.  See the License for the
+   specific language governing permissions and limitations
+   under the License.
+
+.. include:: ../../common.defs
+
+.. _binary-log-v3-format:
+
+Self-Describing Binary Log Format (v3)
+**************************************
+
+This page specifies the on-disk format of a binary log segment, version 3, in
+enough detail to implement a decoder *without* the Traffic Server source tree.
+A version 3 segment is **self-describing**: every field's type is published in
+the segment header, so a generic reader can decode each entry by dispatching on
+a small, stable set of type codes — no embedded copy of the ATS symbol-to-type
+table is required.
+
+Motivation
+==========
+
+In version 2, a segment header carries the field *symbols* (``fmt_fieldlist``,
+e.g. ``"chi cqu pssc"``) and a printf-style *template* (``fmt_printf``) but
+**not** the field types. To decode an entry a reader had to already know the
+type of each symbol, because the value encodings are only self-delimiting once
+the type is known (``IP`` is variable length, for example). That coupled every
+out-of-tree parser to the exact ATS build that wrote the log.
+
+Version 3 adds one thing: a per-segment **field-type schema** that lists the
+wire type of every field, in field order. Decoding then needs only the symbols
+(as keys) and the schema (for types).
+
+Segment layout
+==============
+
+A ``.blog`` file is a stream of segments, each a serialized ``LogBuffer``:
+
+::
+
+    LogBufferHeader            (per segment)
+      cookie       = 0xaceface
+      version      = 3
+      format_type, byte_count, entry_count, timestamps, flags, signature
+      fmt_name_offset
+      fmt_fieldlist_offset      -> "chi cqu pssc ..."   (symbols, space separated)
+      fmt_printf_offset         -> "%<chi> %<cqu> ..."
+      src_hostname_offset, log_filename_offset
+      data_offset               -> first entry
+      fmt_fieldtypes_offset     -> field-type schema     (NEW in v3)
+    [ LogEntryHeader | field0 field1 field2 ... ]   x entry_count
+      LogEntryHeader: timestamp(8) timestamp_usec(4) entry_len(4)
+      fields: concatenated in fieldlist order, no per-field tags
+
+All ``*_offset`` members are byte offsets from the start of the segment (the
+address of the ``LogBufferHeader``). ``fmt_fieldtypes_offset`` is appended
+**after** ``data_offset`` so that the layout through ``data_offset`` is
+byte-identical to version 2; a value of ``0`` means the schema is absent (e.g.
+a text-format segment, or a version 2 segment).
+
+Field-type schema
+=================
+
+At ``fmt_fieldtypes_offset`` the segment stores:
+
+::
+
+    uint16_t field_count;             // == number of symbols in fmt_fieldlist
+    uint8_t  type_code[field_count];  // one type code per field, in order
+
+``type_code[i]`` is the type of the i-th field, which corresponds to the i-th
+symbol in ``fmt_fieldlist`` and the i-th value in each entry. The ``uint16_t``
+``field_count`` prefix is written in **host byte order**, like the rest of
+``LogBufferHeader``. The blob is padded along with the header to an 8-byte
+boundary.
+
+The schema carries no independent version of its own: the segment ``version``
+(``3`` here) governs this layout, so a future schema change rides the same
+``LOG_SEGMENT_VERSION`` bump rather than a second, separate counter.
+
+Stable type codes
+=================
+
+The type codes are the values of the in-tree ``LogField::Type`` enumeration,
+serialized directly. They are part of the published format and are
+**append-only**: codes are never renumbered or reused.
+
+==== ========= ===========================================================
+Code Name      Wire encoding
+==== ========= ===========================================================
+0    INVALID   Reserved. Not emitted by a correct writer; a reader that
+               meets it -- or any code it does not recognize -- cannot
+               determine the field length and must stop decoding the entry.
+1    sINT      A single ``int64_t``, fixed 8 bytes, **host byte order**.
+2    dINT      Two ``int64_t`` (16 bytes), host byte order. Used for
+               values stored as two integers, e.g. HTTP version
+               major/minor.
+3    STRING    NUL-terminated bytes, then padded to an 8-byte boundary.
+4    IP        ``uint16_t`` address family followed by a family-sized
+               address, then padded to an 8-byte boundary (see below).
+==== ========= ===========================================================
+
+The code reflects how the value is *framed* on disk, i.e. how a reader walks
+(or skips) it -- not what the value means. (The ``sINT``/``dINT`` names are an
+ATS-internal distinction; on the wire ``sINT`` is one 8-byte integer and
+``dINT`` is two consecutive ones.) How a consumer *renders* a value -- mapping
+a cache-result integer to ``TCP_HIT``, or a ``dINT`` to ``1.1`` -- is layered
+on top by the consumer and is not part of the wire format.
+
+Value encodings
+===============
+
+sINT
+    An ``int64_t`` occupying exactly 8 bytes, in **host byte order** (as in
+    version 2). Integer values are not endianness-normalized, so a ``.blog`` is
+    not portable across hosts of differing endianness; cross-architecture
+    portability is future work.
+
+dINT
+    Two consecutive ``sINT`` values: 16 bytes total, in host byte order. Used
+    where one log field is stored as two integers, such as an HTTP version
+    (major then minor). The reference decoder renders it as a JSON array, e.g.
+    ``[1,1]``; turning that into ``1.1`` is a consumer concern.
+
+STRING
+    The string bytes followed by a single NUL, then zero padding up to the next
+    8-byte boundary. The on-wire length is therefore
+    ``align_up(strlen + 1, 8)``. An empty/absent string is written as ``"-"``.
+
+IP
+    A ``uint16_t`` address family in host byte order, then:
+
+    .. list-table::
+       :header-rows: 1
+       :widths: 30 70
+
+       * - Family
+         - Following bytes
+       * - ``AF_INET`` (IPv4)
+         - 4-byte ``in_addr``
+       * - ``AF_INET6`` (IPv6)
+         - 16-byte ``in6_addr``
+       * - ``AF_UNIX``
+         - fixed-size path buffer
+       * - ``AF_UNSPEC`` / other
+         - no address bytes
+
+    The whole field is padded to the next 8-byte boundary. Because the length
+    depends on the family byte *inside* the value, only a reader that knows the
+    field is an ``IP`` (from the schema) can compute its size — which is exactly
+    why the schema is required to skip or decode unknown fields safely.
+
+Decoding an entry
+=================
+
+Given a segment, a generic decoder:
+
+#. Reads ``field_count`` and the ``type_code[]`` array from the schema at
+   ``fmt_fieldtypes_offset``.
+#. Splits ``fmt_fieldlist`` into ``field_count`` whitespace-separated symbols.
+#. For each entry (located via ``data_offset`` and walked using

Review Comment:
   fmt_fieldlist is parsed as a comma-separated symbol list (commas optionally followed by spaces). The decode algorithm should reflect that delimiter rather than "whitespace-separated".



##########
src/proxy/logging/Log.cc:
##########
@@ -977,118 +1000,119 @@ Log::init_fields()
   Ptr<LogFieldAliasTable> cache_write_code_map = make_ptr(new LogFieldAliasTable);
   cache_write_code_map->init(N_LOG_CACHE_WRITE_TYPES, LOG_CACHE_WRITE_NONE, "-", LOG_CACHE_WRITE_LOCK_MISSED, "WL_MISS",
                              LOG_CACHE_WRITE_LOCK_ABORTED, "INTR", LOG_CACHE_WRITE_ERROR, "ERR", LOG_CACHE_WRITE_COMPLETE, "FIN");
-  field = new LogField("cache_write_result", "cwr", LogField::sINT, &LogAccess::marshal_cache_write_code,
+  field = new LogField("cache_write_result", "cwr", LogField::Type::sINT, &LogAccess::marshal_cache_write_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwr", field);
 
-  field = new LogField("cache_write_transform_result", "cwtr", LogField::sINT, &LogAccess::marshal_cache_write_transform_code,
+  field = new LogField("cache_write_transform_result", "cwtr", LogField::Type::sINT, &LogAccess::marshal_cache_write_transform_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwtr", field);
 
   // other fields
 
-  field = new LogField("transfer_time_ms", "ttms", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms", "ttms", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttms", field);
 
-  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsh", field);
 
-  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_ttmsf);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsf", field);
 
-  field =
-    new LogField("transfer_time_sec", "tts", LogField::sINT, &LogAccess::marshal_transfer_time_s, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("transfer_time_sec", "tts", LogField::Type::sINT, &LogAccess::marshal_transfer_time_s,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("tts", field);
 
-  field = new LogField("file_size", "fsiz", LogField::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("file_size", "fsiz", LogField::Type::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("fsiz", field);
 
-  field = new LogField("client_connection_id", "ccid", LogField::sINT, &LogAccess::marshal_client_http_connection_id,
+  field = new LogField("client_connection_id", "ccid", LogField::Type::sINT, &LogAccess::marshal_client_http_connection_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ccid", field);
 
-  field = new LogField("client_transaction_id", "ctid", LogField::sINT, &LogAccess::marshal_client_http_transaction_id,
+  field = new LogField("client_transaction_id", "ctid", LogField::Type::sINT, &LogAccess::marshal_client_http_transaction_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctid", field);
 
-  field = new LogField("cache_read_retry_attempts", "crra", LogField::sINT, &LogAccess::marshal_cache_read_retries,
+  field = new LogField("cache_read_retry_attempts", "crra", LogField::Type::sINT, &LogAccess::marshal_cache_read_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crra", field);
 
-  field = new LogField("cache_write_retry_attempts", "cwra", LogField::sINT, &LogAccess::marshal_cache_write_retries,
+  field = new LogField("cache_write_retry_attempts", "cwra", LogField::Type::sINT, &LogAccess::marshal_cache_write_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwra", field);
 
-  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::sINT,
+  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::Type::sINT,
                        &LogAccess::marshal_cache_collapsed_connection_success, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cccs", field);
 
-  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::sINT,
+  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_weight, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpw", field);
 
-  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::sINT,
+  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_dependence, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpd", field);
 
-  field = new LogField("proxy_protocol_version", "ppv", LogField::STRING, &LogAccess::marshal_proxy_protocol_version,
+  field = new LogField("proxy_protocol_version", "ppv", LogField::Type::STRING, &LogAccess::marshal_proxy_protocol_version,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppv", field);
 
-  field = new LogField("proxy_protocol_src_ip", "pps", LogField::IP, &LogAccess::marshal_proxy_protocol_src_ip,
+  field = new LogField("proxy_protocol_src_ip", "pps", LogField::Type::IP, &LogAccess::marshal_proxy_protocol_src_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppsip", field);
 
-  field = new LogField("proxy_protocol_dst_ip", "ppd", LogField::IP, &LogAccess::marshal_proxy_protocol_dst_ip,
+  field = new LogField("proxy_protocol_dst_ip", "ppd", LogField::Type::IP, &LogAccess::marshal_proxy_protocol_dst_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppdip", field);
 
-  field = new LogField("proxy_protocol_authority", "ppa", LogField::IP, &LogAccess::marshal_proxy_protocol_authority,
+  field = new LogField("proxy_protocol_authority", "ppa", LogField::Type::IP, &LogAccess::marshal_proxy_protocol_authority,

Review Comment:
   "ppa" (proxy_protocol_authority) is marshalled as a padded NUL-terminated byte string (marshal_proxy_protocol_authority uses marshal_mem/padded_length), so its LogField type must be STRING. Declaring it as IP will cause v3 schemas and type-driven readers to misframe subsequent fields.



##########
src/traffic_logcat/LogEntryJson.cc:
##########
@@ -0,0 +1,291 @@
+/** @file
+
+  Reference decoder for the self-describing v3 binary log format.
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+
+#include "LogEntryJson.h"
+
+#include "proxy/logging/LogBuffer.h"
+#include "proxy/logging/LogAccess.h"
+#include "proxy/logging/LogField.h"
+
+// Deliberately not including the global field table (proxy/logging/Log.h):
+// decoding must depend only on the segment's schema (see LogEntryJson.h).
+
+#include "tscore/ink_inet.h"
+#include "tscore/ink_align.h"
+
+#include <cinttypes>
+#include <cstdint>
+#include <cstdio>
+#include <cstring>
+
+namespace
+{
+// fmt_fieldlist symbols are comma-separated (e.g. "chi,cqu,pssc"); tolerate
+// spaces too.
+bool
+is_field_sep(char c)
+{
+  return c == ',' || c == ' ';
+}
+} // namespace
+
+int
+log_entry_to_json(LogEntryHeader *entry, LogBufferHeader *header, char *buf, int buf_len)
+{
+  if (entry == nullptr || header == nullptr || buf == nullptr || buf_len <= 0) {
+    return -1;
+  }
+
+  // [seg_start, seg_end) is the only readable region (byte_count == segment size).
+  char *seg_start = reinterpret_cast<char *>(header);
+  char *seg_end   = seg_start + header->byte_count;
+
+  // v3 decode needs the field-type schema and the symbols.
+  char *schema_blob = header->fmt_fieldtypes();
+  char *symbols     = header->fmt_fieldlist();
+  if (schema_blob == nullptr || symbols == nullptr) {
+    return -1;
+  }
+  // Both must lie within the segment, including the fixed schema prefix.
+  if (schema_blob < seg_start || schema_blob + sizeof(LogFieldTypeSchema) > seg_end || symbols < seg_start || symbols >= seg_end) {
+    return -1;
+  }
+
+  // field_count is a uint16 that may sit at an unaligned offset (the writer
+  // places the schema right after the NUL-terminated header strings), so read
+  // it byte-wise rather than through the struct.
+  uint16_t fc16 = 0;
+  memcpy(&fc16, schema_blob, sizeof(fc16));
+  const unsigned field_count = fc16;
+  const uint8_t *codes       = reinterpret_cast<const uint8_t *>(schema_blob) + sizeof(LogFieldTypeSchema);
+  if (reinterpret_cast<const char *>(codes) + field_count > seg_end) {
+    return -1;
+  }
+
+  // field_count must match the symbol count; a mismatch is a corrupt segment.
+  unsigned symbol_count = 0;
+  bool     in_token     = false;
+  for (const char *p = symbols; p < seg_end && *p != '\0'; ++p) {
+    if (is_field_sep(*p)) {
+      in_token = false;
+    } else if (!in_token) {
+      in_token = true;
+      ++symbol_count;
+    }
+  }
+  if (symbol_count != field_count) {
+    return -1;
+  }
+
+  // Clamp value reads to this entry (or the segment, if entry_len is bogus).
+  char *entry_start = reinterpret_cast<char *>(entry);
+  if (entry_start < seg_start || entry_start + sizeof(LogEntryHeader) > seg_end) {
+    return -1;
+  }
+  char *read_from = entry_start + sizeof(LogEntryHeader);
+  char *read_end  = entry_start + entry->entry_len;
+  if (read_end < read_from || read_end > seg_end) {
+    read_end = seg_end;
+  }
+
+  int written = 0;
+
+  // Append n bytes, keeping one byte in reserve for the trailing NUL.
+  auto put = [&](const char *src, int n) -> bool {
+    if (n < 0 || written + n >= buf_len) {
+      return false;
+    }
+    memcpy(buf + written, src, n);
+    written += n;
+    return true;
+  };
+  auto put_ch = [&](char c) -> bool { return put(&c, 1); };
+
+  if (!put_ch('{')) {
+    return -1;
+  }
+
+  const char *sym = symbols;
+  for (unsigned i = 0; i < field_count; ++i) {
+    // Next symbol token (comma-separated in fmt_fieldlist), bounded by the segment.
+    while (sym < seg_end && is_field_sep(*sym)) {
+      ++sym;
+    }
+    const char *sym_start = sym;
+    while (sym < seg_end && *sym != '\0' && !is_field_sep(*sym)) {
+      ++sym;
+    }
+    int sym_len = static_cast<int>(sym - sym_start);
+
+    if (i > 0 && !put_ch(',')) {
+      return -1;
+    }
+    if (!put_ch('"') || !put(sym_start, sym_len) || !put_ch('"') || !put_ch(':')) {
+      return -1;
+    }

Review Comment:
   JSON keys are emitted directly from fmt_fieldlist without any escaping. Since fmt_fieldlist is parsed from config (and may also come from an untrusted/corrupt .blog), a symbol containing a quote, backslash, or control byte will produce invalid JSON output (and can break downstream parsers). Keys should be JSON-escaped the same way STRING field values are.



##########
src/proxy/logging/Log.cc:
##########
@@ -977,118 +1000,119 @@ Log::init_fields()
   Ptr<LogFieldAliasTable> cache_write_code_map = make_ptr(new LogFieldAliasTable);
   cache_write_code_map->init(N_LOG_CACHE_WRITE_TYPES, LOG_CACHE_WRITE_NONE, "-", LOG_CACHE_WRITE_LOCK_MISSED, "WL_MISS",
                              LOG_CACHE_WRITE_LOCK_ABORTED, "INTR", LOG_CACHE_WRITE_ERROR, "ERR", LOG_CACHE_WRITE_COMPLETE, "FIN");
-  field = new LogField("cache_write_result", "cwr", LogField::sINT, &LogAccess::marshal_cache_write_code,
+  field = new LogField("cache_write_result", "cwr", LogField::Type::sINT, &LogAccess::marshal_cache_write_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwr", field);
 
-  field = new LogField("cache_write_transform_result", "cwtr", LogField::sINT, &LogAccess::marshal_cache_write_transform_code,
+  field = new LogField("cache_write_transform_result", "cwtr", LogField::Type::sINT, &LogAccess::marshal_cache_write_transform_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwtr", field);
 
   // other fields
 
-  field = new LogField("transfer_time_ms", "ttms", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms", "ttms", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttms", field);
 
-  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsh", field);
 
-  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_ttmsf);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsf", field);
 
-  field =
-    new LogField("transfer_time_sec", "tts", LogField::sINT, &LogAccess::marshal_transfer_time_s, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("transfer_time_sec", "tts", LogField::Type::sINT, &LogAccess::marshal_transfer_time_s,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("tts", field);
 
-  field = new LogField("file_size", "fsiz", LogField::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("file_size", "fsiz", LogField::Type::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("fsiz", field);
 
-  field = new LogField("client_connection_id", "ccid", LogField::sINT, &LogAccess::marshal_client_http_connection_id,
+  field = new LogField("client_connection_id", "ccid", LogField::Type::sINT, &LogAccess::marshal_client_http_connection_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ccid", field);
 
-  field = new LogField("client_transaction_id", "ctid", LogField::sINT, &LogAccess::marshal_client_http_transaction_id,
+  field = new LogField("client_transaction_id", "ctid", LogField::Type::sINT, &LogAccess::marshal_client_http_transaction_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctid", field);
 
-  field = new LogField("cache_read_retry_attempts", "crra", LogField::sINT, &LogAccess::marshal_cache_read_retries,
+  field = new LogField("cache_read_retry_attempts", "crra", LogField::Type::sINT, &LogAccess::marshal_cache_read_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crra", field);
 
-  field = new LogField("cache_write_retry_attempts", "cwra", LogField::sINT, &LogAccess::marshal_cache_write_retries,
+  field = new LogField("cache_write_retry_attempts", "cwra", LogField::Type::sINT, &LogAccess::marshal_cache_write_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwra", field);
 
-  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::sINT,
+  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::Type::sINT,
                        &LogAccess::marshal_cache_collapsed_connection_success, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cccs", field);
 
-  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::sINT,
+  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_weight, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpw", field);
 
-  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::sINT,
+  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_dependence, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpd", field);
 
-  field = new LogField("proxy_protocol_version", "ppv", LogField::STRING, &LogAccess::marshal_proxy_protocol_version,
+  field = new LogField("proxy_protocol_version", "ppv", LogField::Type::STRING, &LogAccess::marshal_proxy_protocol_version,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppv", field);
 
-  field = new LogField("proxy_protocol_src_ip", "pps", LogField::IP, &LogAccess::marshal_proxy_protocol_src_ip,
+  field = new LogField("proxy_protocol_src_ip", "pps", LogField::Type::IP, &LogAccess::marshal_proxy_protocol_src_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppsip", field);
 
-  field = new LogField("proxy_protocol_dst_ip", "ppd", LogField::IP, &LogAccess::marshal_proxy_protocol_dst_ip,
+  field = new LogField("proxy_protocol_dst_ip", "ppd", LogField::Type::IP, &LogAccess::marshal_proxy_protocol_dst_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppdip", field);

Review Comment:
   field_symbol_hash key should match the field symbol ("ppd"), otherwise log formats using %<ppd> won't resolve this field. The current key "ppdip" makes the field effectively unreachable by its documented symbol.



##########
doc/developer-guide/logging-architecture/binary-log-v3-format.en.rst:
##########
@@ -0,0 +1,216 @@
+.. Licensed to the Apache Software Foundation (ASF) under one
+   or more contributor license agreements.  See the NOTICE file
+   distributed with this work for additional information
+   regarding copyright ownership.  The ASF licenses this file
+   to you under the Apache License, Version 2.0 (the
+   "License"); you may not use this file except in compliance
+   with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing,
+   software distributed under the License is distributed on an
+   "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+   KIND, either express or implied.  See the License for the
+   specific language governing permissions and limitations
+   under the License.
+
+.. include:: ../../common.defs
+
+.. _binary-log-v3-format:
+
+Self-Describing Binary Log Format (v3)
+**************************************
+
+This page specifies the on-disk format of a binary log segment, version 3, in
+enough detail to implement a decoder *without* the Traffic Server source tree.
+A version 3 segment is **self-describing**: every field's type is published in
+the segment header, so a generic reader can decode each entry by dispatching on
+a small, stable set of type codes — no embedded copy of the ATS symbol-to-type
+table is required.
+
+Motivation
+==========
+
+In version 2, a segment header carries the field *symbols* (``fmt_fieldlist``,
+e.g. ``"chi cqu pssc"``) and a printf-style *template* (``fmt_printf``) but
+**not** the field types. To decode an entry a reader had to already know the
+type of each symbol, because the value encodings are only self-delimiting once
+the type is known (``IP`` is variable length, for example). That coupled every
+out-of-tree parser to the exact ATS build that wrote the log.
+
+Version 3 adds one thing: a per-segment **field-type schema** that lists the
+wire type of every field, in field order. Decoding then needs only the symbols
+(as keys) and the schema (for types).
+
+Segment layout
+==============
+
+A ``.blog`` file is a stream of segments, each a serialized ``LogBuffer``:
+
+::
+
+    LogBufferHeader            (per segment)
+      cookie       = 0xaceface
+      version      = 3
+      format_type, byte_count, entry_count, timestamps, flags, signature
+      fmt_name_offset
+      fmt_fieldlist_offset      -> "chi cqu pssc ..."   (symbols, space separated)
+      fmt_printf_offset         -> "%<chi> %<cqu> ..."

Review Comment:
   The on-disk fmt_fieldlist produced by LogFormat::parse_format_string is comma-separated (e.g. "chi,cqu,pssc"), not space-separated. Describing it as space-separated is misleading for out-of-tree readers that follow this spec verbatim.



##########
src/proxy/logging/Log.cc:
##########
@@ -328,371 +328,389 @@ Log::init_fields()
   LogField::init_milestone_container();
 
   // client -> proxy fields
-  field = new LogField("client_host_ip", "chi", LogField::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_str);
+  field =
+    new LogField("client_host_ip", "chi", LogField::Type::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chi", field);
 
-  field =
-    new LogField("client_host_port", "chp", LogField::sINT, &LogAccess::marshal_client_host_port, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("client_host_port", "chp", LogField::Type::sINT, &LogAccess::marshal_client_host_port,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chp", field);
 
-  field =
-    new LogField("client_host_ip_hex", "chih", LogField::IP, &LogAccess::marshal_client_host_ip, &LogAccess::unmarshal_ip_to_hex);
+  field = new LogField("client_host_ip_hex", "chih", LogField::Type::IP, &LogAccess::marshal_client_host_ip,
+                       &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chih", field);
 
-  field = new LogField("client_host_ip_verified", "chiv", LogField::IP, &LogAccess::marshal_client_host_ip_verified,
+  field = new LogField("client_host_ip_verified", "chiv", LogField::Type::IP, &LogAccess::marshal_client_host_ip_verified,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chiv", field);
 
   // remote client (Not necessarily the requesting client IP - See proxy protocol)
-  field = new LogField("remote_host_ip", "rchi", LogField::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_str);
+  field =
+    new LogField("remote_host_ip", "rchi", LogField::Type::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchi", field);
 
-  field = new LogField("remote_host_port", "rchp", LogField::sINT, &LogAccess::marshal_remote_host_port,
+  field = new LogField("remote_host_port", "rchp", LogField::Type::sINT, &LogAccess::marshal_remote_host_port,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchp", field);
 
-  field =
-    new LogField("remote_host_ip_hex", "rchh", LogField::IP, &LogAccess::marshal_remote_host_ip, &LogAccess::unmarshal_ip_to_hex);
+  field = new LogField("remote_host_ip_hex", "rchh", LogField::Type::IP, &LogAccess::marshal_remote_host_ip,
+                       &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("rchh", field);
 
   // interface ip
 
-  field =
-    new LogField("host_interface_ip", "hii", LogField::IP, &LogAccess::marshal_host_interface_ip, &LogAccess::unmarshal_ip_to_str);
+  field = new LogField("host_interface_ip", "hii", LogField::Type::IP, &LogAccess::marshal_host_interface_ip,
+                       &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("hii", field);
 
-  field = new LogField("host_interface_ip_hex", "hiih", LogField::IP, &LogAccess::marshal_host_interface_ip,
+  field = new LogField("host_interface_ip_hex", "hiih", LogField::Type::IP, &LogAccess::marshal_host_interface_ip,
                        &LogAccess::unmarshal_ip_to_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("hiih", field);
   // interface ip end
-  field = new LogField("client_auth_user_name", "caun", LogField::STRING, &LogAccess::marshal_client_auth_user_name,
+  field = new LogField("client_auth_user_name", "caun", LogField::Type::STRING, &LogAccess::marshal_client_auth_user_name,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("caun", field);
 
-  field = new LogField("plugin_identity_id", "piid", LogField::sINT, &LogAccess::marshal_plugin_identity_id,
+  field = new LogField("plugin_identity_id", "piid", LogField::Type::sINT, &LogAccess::marshal_plugin_identity_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("piid", field);
 
-  field = new LogField("plugin_identity_tag", "pitag", LogField::STRING, &LogAccess::marshal_plugin_identity_tag,
+  field = new LogField("plugin_identity_tag", "pitag", LogField::Type::STRING, &LogAccess::marshal_plugin_identity_tag,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pitag", field);
 
-  field = new LogField("client_req_timestamp_sec", "cqts", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_sec", "cqts", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqts", field);
 
-  field = new LogField("client_req_timestamp_hex_sec", "cqth", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_hex_sec", "cqth", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_str_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqth", field);
 
-  field = new LogField("client_req_timestamp_squid", "cqtq", LogField::sINT, &LogAccess::marshal_client_req_timestamp_ms,
+  field = new LogField("client_req_timestamp_squid", "cqtq", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_ms,
                        &LogAccess::unmarshal_ttmsf);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtq", field);
 
-  field = new LogField("client_req_timestamp_netscape", "cqtn", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_netscape", "cqtn", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_netscape_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtn", field);
 
-  field = new LogField("client_req_timestamp_date", "cqtd", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_date", "cqtd", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_date_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtd", field);
 
-  field = new LogField("client_req_timestamp_time", "cqtt", LogField::sINT, &LogAccess::marshal_client_req_timestamp_sec,
+  field = new LogField("client_req_timestamp_time", "cqtt", LogField::Type::sINT, &LogAccess::marshal_client_req_timestamp_sec,
                        &LogAccess::unmarshal_int_to_time_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtt", field);
 
-  field = new LogField("client_req_http_method", "cqhm", LogField::STRING, &LogAccess::marshal_client_req_http_method,
+  field = new LogField("client_req_http_method", "cqhm", LogField::Type::STRING, &LogAccess::marshal_client_req_http_method,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqhm", field);
 
-  field = new LogField("client_req_url", "cqu", LogField::STRING, &LogAccess::marshal_client_req_url, &LogAccess::unmarshal_str,
-                       &LogAccess::set_client_req_url);
+  field = new LogField("client_req_url", "cqu", LogField::Type::STRING, &LogAccess::marshal_client_req_url,
+                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_url);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqu", field);
 
-  field = new LogField("client_req_url", "pqu", LogField::STRING, &LogAccess::marshal_client_req_url, &LogAccess::unmarshal_str,
-                       &LogAccess::set_client_req_url);
+  field = new LogField("client_req_url", "pqu", LogField::Type::STRING, &LogAccess::marshal_client_req_url,
+                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_url);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqu", field);
 
-  field = new LogField("client_req_url_canonical", "cquc", LogField::STRING, &LogAccess::marshal_client_req_url_canon,
+  field = new LogField("client_req_url_canonical", "cquc", LogField::Type::STRING, &LogAccess::marshal_client_req_url_canon,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquc", field);
 
-  field = new LogField("client_req_url_canonical", "pquc", LogField::STRING, &LogAccess::marshal_client_req_url_canon,
+  field = new LogField("client_req_url_canonical", "pquc", LogField::Type::STRING, &LogAccess::marshal_client_req_url_canon,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pquc", field);
 
-  field =
-    new LogField("client_req_unmapped_url_canonical", "cquuc", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_canon,
-                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_canon);
+  field = new LogField("client_req_unmapped_url_canonical", "cquuc", LogField::Type::STRING,
+                       &LogAccess::marshal_client_req_unmapped_url_canon, &LogAccess::unmarshal_str,
+                       &LogAccess::set_client_req_unmapped_url_canon);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquuc", field);
 
-  field = new LogField("client_req_unmapped_url_path", "cquup", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_path,
-                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_path);
+  field =
+    new LogField("client_req_unmapped_url_path", "cquup", LogField::Type::STRING, &LogAccess::marshal_client_req_unmapped_url_path,
+                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquup", field);
 
-  field = new LogField("client_req_unmapped_url_host", "cquuh", LogField::STRING, &LogAccess::marshal_client_req_unmapped_url_host,
-                       &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_host);
+  field =
+    new LogField("client_req_unmapped_url_host", "cquuh", LogField::Type::STRING, &LogAccess::marshal_client_req_unmapped_url_host,
+                 &LogAccess::unmarshal_str, &LogAccess::set_client_req_unmapped_url_host);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cquuh", field);
 
-  field = new LogField("client_req_url_scheme", "cqus", LogField::STRING, &LogAccess::marshal_client_req_url_scheme,
+  field = new LogField("client_req_url_scheme", "cqus", LogField::Type::STRING, &LogAccess::marshal_client_req_url_scheme,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqus", field);
 
-  field = new LogField("client_req_url_scheme", "pqus", LogField::STRING, &LogAccess::marshal_client_req_url_scheme,
+  field = new LogField("client_req_url_scheme", "pqus", LogField::Type::STRING, &LogAccess::marshal_client_req_url_scheme,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqus", field);
 
-  field = new LogField("client_req_url_path", "cqup", LogField::STRING, &LogAccess::marshal_client_req_url_path,
+  field = new LogField("client_req_url_path", "cqup", LogField::Type::STRING, &LogAccess::marshal_client_req_url_path,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqup", field);
 
-  field = new LogField("client_req_url_path", "pqup", LogField::STRING, &LogAccess::marshal_client_req_url_path,
+  field = new LogField("client_req_url_path", "pqup", LogField::Type::STRING, &LogAccess::marshal_client_req_url_path,
                        &LogAccess::unmarshal_str, &LogAccess::set_client_req_url_path);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqup", field);
 
-  field = new LogField("client_req_protocol_version", "cqpv", LogField::STRING, &LogAccess::marshal_client_req_protocol_version,
-                       &LogAccess::unmarshal_str);
+  field = new LogField("client_req_protocol_version", "cqpv", LogField::Type::STRING,
+                       &LogAccess::marshal_client_req_protocol_version, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqpv", field);
 
-  field = new LogField("server_req_protocol_version", "sqpv", LogField::STRING, &LogAccess::marshal_server_req_protocol_version,
-                       &LogAccess::unmarshal_str);
+  field = new LogField("server_req_protocol_version", "sqpv", LogField::Type::STRING,
+                       &LogAccess::marshal_server_req_protocol_version, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("sqpv", field);
 
-  field = new LogField("client_req_header_len", "cqhl", LogField::sINT, &LogAccess::marshal_client_req_header_len,
+  field = new LogField("client_req_header_len", "cqhl", LogField::Type::sINT, &LogAccess::marshal_client_req_header_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqhl", field);
 
-  field = new LogField("client_req_squid_len", "cqql", LogField::sINT, &LogAccess::marshal_client_req_squid_len,
+  field = new LogField("client_req_squid_len", "cqql", LogField::Type::sINT, &LogAccess::marshal_client_req_squid_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqql", field);
 
   // Client request squid length plus TLS handshake bytes received for TLS connections
-  field = new LogField("client_req_squid_len_tls", "cqqtl", LogField::sINT, &LogAccess::marshal_client_req_squid_len_tls,
+  field = new LogField("client_req_squid_len_tls", "cqqtl", LogField::Type::sINT, &LogAccess::marshal_client_req_squid_len_tls,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqqtl", field);
 
-  field = new LogField("cache_lookup_url_canonical", "cluc", LogField::STRING, &LogAccess::marshal_cache_lookup_url_canon,
+  // Client request squid length plus TLS handshake bytes received for TLS connections
+  field = new LogField("client_req_squid_len_tls", "cqqtl", LogField::Type::sINT, &LogAccess::marshal_client_req_squid_len_tls,
+                       &LogAccess::unmarshal_int_to_str);
+  global_field_list.add(field, false);
+  field_symbol_hash.emplace("cqqtl", field);
+
+  field = new LogField("cache_lookup_url_canonical", "cluc", LogField::Type::STRING, &LogAccess::marshal_cache_lookup_url_canon,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cluc", field);
 
-  field = new LogField("cache_key_hash", "ckh", LogField::STRING, &LogAccess::marshal_cache_key_hash, &LogAccess::unmarshal_str);
+  field =
+    new LogField("cache_key_hash", "ckh", LogField::Type::STRING, &LogAccess::marshal_cache_key_hash, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ckh", field);
 
-  field = new LogField("client_sni_server_name", "cssn", LogField::STRING, &LogAccess::marshal_client_sni_server_name,
+  field = new LogField("client_sni_server_name", "cssn", LogField::Type::STRING, &LogAccess::marshal_client_sni_server_name,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cssn", field);
 
-  field = new LogField("client_ssl_cert_provided", "cscert", LogField::STRING, &LogAccess::marshal_client_provided_cert,
+  field = new LogField("client_ssl_cert_provided", "cscert", LogField::Type::STRING, &LogAccess::marshal_client_provided_cert,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cscert", field);
 
-  field = new LogField("proxy_ssl_cert_provided", "pscert", LogField::STRING, &LogAccess::marshal_proxy_provided_cert,
+  field = new LogField("proxy_ssl_cert_provided", "pscert", LogField::Type::STRING, &LogAccess::marshal_proxy_provided_cert,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pscert", field);
 
-  field = new LogField("process_uuid", "puuid", LogField::STRING, &LogAccess::marshal_process_uuid, &LogAccess::unmarshal_str);
+  field =
+    new LogField("process_uuid", "puuid", LogField::Type::STRING, &LogAccess::marshal_process_uuid, &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("puuid", field);
 
-  field =
-    new LogField("process_snowflake_id", "psfid", LogField::STRING, &LogAccess::marshal_process_sfid, &LogAccess::unmarshal_str);
+  field = new LogField("process_snowflake_id", "psfid", LogField::Type::STRING, &LogAccess::marshal_process_sfid,
+                       &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("psfid", field);
 
-  field = new LogField("client_req_content_len", "cqcl", LogField::sINT, &LogAccess::marshal_client_req_content_len,
+  field = new LogField("client_req_content_len", "cqcl", LogField::Type::sINT, &LogAccess::marshal_client_req_content_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqcl", field);
 
-  field = new LogField("client_req_tcp_reused", "cqtr", LogField::sINT, &LogAccess::marshal_client_req_tcp_reused,
+  field = new LogField("client_req_tcp_reused", "cqtr", LogField::Type::sINT, &LogAccess::marshal_client_req_tcp_reused,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqtr", field);
 
-  field = new LogField("client_req_is_ssl", "cqssl", LogField::sINT, &LogAccess::marshal_client_req_is_ssl,
+  field = new LogField("client_req_is_ssl", "cqssl", LogField::Type::sINT, &LogAccess::marshal_client_req_is_ssl,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssl", field);
 
-  field = new LogField("client_req_ssl_reused", "cqssr", LogField::sINT, &LogAccess::marshal_client_req_ssl_reused,
+  field = new LogField("client_req_ssl_reused", "cqssr", LogField::Type::sINT, &LogAccess::marshal_client_req_ssl_reused,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssr", field);
 
-  field = new LogField("client_req_is_internal", "cqint", LogField::sINT, &LogAccess::marshal_client_req_is_internal,
+  field = new LogField("client_req_is_internal", "cqint", LogField::Type::sINT, &LogAccess::marshal_client_req_is_internal,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqint", field);
 
-  field = new LogField("client_req_mptcp", "cqmpt", LogField::sINT, &LogAccess::marshal_client_req_mptcp_state,
+  field = new LogField("client_req_mptcp", "cqmpt", LogField::Type::sINT, &LogAccess::marshal_client_req_mptcp_state,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqmpt", field);
 
-  field = new LogField("client_sec_protocol", "cqssv", LogField::STRING, &LogAccess::marshal_client_security_protocol,
+  field = new LogField("client_sec_protocol", "cqssv", LogField::Type::STRING, &LogAccess::marshal_client_security_protocol,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssv", field);
 
-  field = new LogField("client_cipher_suite", "cqssc", LogField::STRING, &LogAccess::marshal_client_security_cipher_suite,
+  field = new LogField("client_cipher_suite", "cqssc", LogField::Type::STRING, &LogAccess::marshal_client_security_cipher_suite,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssc", field);
 
-  field =
-    new LogField("client_curve", "cqssu", LogField::STRING, &LogAccess::marshal_client_security_curve, &LogAccess::unmarshal_str);
+  field = new LogField("client_curve", "cqssu", LogField::Type::STRING, &LogAccess::marshal_client_security_curve,
+                       &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssu", field);
 
-  field =
-    new LogField("client_group", "cqssg", LogField::STRING, &LogAccess::marshal_client_security_group, &LogAccess::unmarshal_str);
+  field = new LogField("client_group", "cqssg", LogField::Type::STRING, &LogAccess::marshal_client_security_group,
+                       &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssg", field);
 
-  field =
-    new LogField("client_sec_alpn", "cqssa", LogField::STRING, &LogAccess::marshal_client_security_alpn, &LogAccess::unmarshal_str);
+  field = new LogField("client_sec_alpn", "cqssa", LogField::Type::STRING, &LogAccess::marshal_client_security_alpn,
+                       &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqssa", field);
 
   // TLS handshake bytes - bytes received from client during TLS handshake
-  field = new LogField("client_tls_handshake_bytes_rx", "cthbr", LogField::sINT, &LogAccess::marshal_client_tls_handshake_bytes_rx,
-                       &LogAccess::unmarshal_int_to_str);
+  field = new LogField("client_tls_handshake_bytes_rx", "cthbr", LogField::Type::sINT,
+                       &LogAccess::marshal_client_tls_handshake_bytes_rx, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cthbr", field);
 
   // TLS handshake bytes - bytes sent to client during TLS handshake
-  field = new LogField("client_tls_handshake_bytes_tx", "cthbt", LogField::sINT, &LogAccess::marshal_client_tls_handshake_bytes_tx,
-                       &LogAccess::unmarshal_int_to_str);
+  field = new LogField("client_tls_handshake_bytes_tx", "cthbt", LogField::Type::sINT,
+                       &LogAccess::marshal_client_tls_handshake_bytes_tx, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cthbt", field);
 
   // TLS handshake bytes - total (rx + tx) during TLS handshake
-  field = new LogField("client_tls_handshake_bytes", "cthb", LogField::sINT, &LogAccess::marshal_client_tls_handshake_bytes,
+  field = new LogField("client_tls_handshake_bytes", "cthb", LogField::Type::sINT, &LogAccess::marshal_client_tls_handshake_bytes,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cthb", field);
 
   Ptr<LogFieldAliasTable> finish_status_map = make_ptr(new LogFieldAliasTable);
   finish_status_map->init(N_LOG_FINISH_CODE_TYPES, LOG_FINISH_FIN, "FIN", LOG_FINISH_INTR, "INTR", LOG_FINISH_TIMEOUT, "TIMEOUT");
 
-  field = new LogField("client_finish_status_code", "cfsc", LogField::sINT, &LogAccess::marshal_client_finish_status_code,
+  field = new LogField("client_finish_status_code", "cfsc", LogField::Type::sINT, &LogAccess::marshal_client_finish_status_code,
                        &LogAccess::unmarshal_finish_status, make_alias_map(finish_status_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cfsc", field);
 
-  field =
-    new LogField("client_req_id", "crid", LogField::sINT, &LogAccess::marshal_client_req_id, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("client_req_id", "crid", LogField::Type::sINT, &LogAccess::marshal_client_req_id,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crid", field);
 
-  field =
-    new LogField("client_req_uuid", "cruuid", LogField::STRING, &LogAccess::marshal_client_req_uuid, &LogAccess::unmarshal_str);
+  field = new LogField("client_req_uuid", "cruuid", LogField::Type::STRING, &LogAccess::marshal_client_req_uuid,
+                       &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cruuid", field);
 
-  field = new LogField("client_rx_error_code", "crec", LogField::STRING, &LogAccess::marshal_client_rx_error_code,
+  field = new LogField("client_rx_error_code", "crec", LogField::Type::STRING, &LogAccess::marshal_client_rx_error_code,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crec", field);
 
-  field = new LogField("client_tx_error_code", "ctec", LogField::STRING, &LogAccess::marshal_client_tx_error_code,
+  field = new LogField("client_tx_error_code", "ctec", LogField::Type::STRING, &LogAccess::marshal_client_tx_error_code,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctec", field);
 
-  field = new LogField("client_request_all_header_fields", "cqah", LogField::STRING,
+  field = new LogField("client_request_all_header_fields", "cqah", LogField::Type::STRING,
                        &LogAccess::marshal_client_req_all_header_fields, &LogUtils::unmarshalMimeHdr);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cqah", field);
 
   // proxy -> client fields
-  field = new LogField("proxy_resp_content_type", "psct", LogField::STRING, &LogAccess::marshal_proxy_resp_content_type,
+  field = new LogField("proxy_resp_content_type", "psct", LogField::Type::STRING, &LogAccess::marshal_proxy_resp_content_type,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("psct", field);
 
-  field = new LogField("proxy_resp_reason_phrase", "prrp", LogField::STRING, &LogAccess::marshal_proxy_resp_reason_phrase,
+  field = new LogField("proxy_resp_reason_phrase", "prrp", LogField::Type::STRING, &LogAccess::marshal_proxy_resp_reason_phrase,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("prrp", field);
 
-  field = new LogField("proxy_resp_squid_len", "psql", LogField::sINT, &LogAccess::marshal_proxy_resp_squid_len,
+  field = new LogField("proxy_resp_squid_len", "psql", LogField::Type::sINT, &LogAccess::marshal_proxy_resp_squid_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("psql", field);
 
   // Squid length plus TLS handshake bytes sent for TLS connections
-  field = new LogField("proxy_resp_squid_len_tls", "psqtl", LogField::sINT, &LogAccess::marshal_proxy_resp_squid_len_tls,
+  field = new LogField("proxy_resp_squid_len_tls", "psqtl", LogField::Type::sINT, &LogAccess::marshal_proxy_resp_squid_len_tls,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("psqtl", field);

Review Comment:
   Log field "psqtl" is registered twice (duplicate LogField allocation, duplicate global_field_list entry, and duplicate field_symbol_hash key). The second registration leaks memory and can lead to inconsistent field iteration/lookup behavior.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#issuecomment-4784232088

   [approve ci autest 0of4 1of4]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#issuecomment-4774155816

   [approve ci]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#discussion_r3449280880


##########
src/proxy/logging/Log.cc:
##########
@@ -977,118 +1000,119 @@ Log::init_fields()
   Ptr<LogFieldAliasTable> cache_write_code_map = make_ptr(new LogFieldAliasTable);
   cache_write_code_map->init(N_LOG_CACHE_WRITE_TYPES, LOG_CACHE_WRITE_NONE, "-", LOG_CACHE_WRITE_LOCK_MISSED, "WL_MISS",
                              LOG_CACHE_WRITE_LOCK_ABORTED, "INTR", LOG_CACHE_WRITE_ERROR, "ERR", LOG_CACHE_WRITE_COMPLETE, "FIN");
-  field = new LogField("cache_write_result", "cwr", LogField::sINT, &LogAccess::marshal_cache_write_code,
+  field = new LogField("cache_write_result", "cwr", LogField::Type::sINT, &LogAccess::marshal_cache_write_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwr", field);
 
-  field = new LogField("cache_write_transform_result", "cwtr", LogField::sINT, &LogAccess::marshal_cache_write_transform_code,
+  field = new LogField("cache_write_transform_result", "cwtr", LogField::Type::sINT, &LogAccess::marshal_cache_write_transform_code,
                        &LogAccess::unmarshal_cache_write_code, make_alias_map(cache_write_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwtr", field);
 
   // other fields
 
-  field = new LogField("transfer_time_ms", "ttms", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms", "ttms", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttms", field);
 
-  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_hex", "ttmsh", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_int_to_str_hex);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsh", field);
 
-  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::sINT, &LogAccess::marshal_transfer_time_ms,
+  field = new LogField("transfer_time_ms_fractional", "ttmsf", LogField::Type::sINT, &LogAccess::marshal_transfer_time_ms,
                        &LogAccess::unmarshal_ttmsf);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ttmsf", field);
 
-  field =
-    new LogField("transfer_time_sec", "tts", LogField::sINT, &LogAccess::marshal_transfer_time_s, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("transfer_time_sec", "tts", LogField::Type::sINT, &LogAccess::marshal_transfer_time_s,
+                       &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("tts", field);
 
-  field = new LogField("file_size", "fsiz", LogField::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
+  field = new LogField("file_size", "fsiz", LogField::Type::sINT, &LogAccess::marshal_file_size, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("fsiz", field);
 
-  field = new LogField("client_connection_id", "ccid", LogField::sINT, &LogAccess::marshal_client_http_connection_id,
+  field = new LogField("client_connection_id", "ccid", LogField::Type::sINT, &LogAccess::marshal_client_http_connection_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ccid", field);
 
-  field = new LogField("client_transaction_id", "ctid", LogField::sINT, &LogAccess::marshal_client_http_transaction_id,
+  field = new LogField("client_transaction_id", "ctid", LogField::Type::sINT, &LogAccess::marshal_client_http_transaction_id,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctid", field);
 
-  field = new LogField("cache_read_retry_attempts", "crra", LogField::sINT, &LogAccess::marshal_cache_read_retries,
+  field = new LogField("cache_read_retry_attempts", "crra", LogField::Type::sINT, &LogAccess::marshal_cache_read_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crra", field);
 
-  field = new LogField("cache_write_retry_attempts", "cwra", LogField::sINT, &LogAccess::marshal_cache_write_retries,
+  field = new LogField("cache_write_retry_attempts", "cwra", LogField::Type::sINT, &LogAccess::marshal_cache_write_retries,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cwra", field);
 
-  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::sINT,
+  field = new LogField("cache_collapsed_connection_success", "cccs", LogField::Type::sINT,
                        &LogAccess::marshal_cache_collapsed_connection_success, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("cccs", field);
 
-  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::sINT,
+  field = new LogField("client_transaction_priority_weight", "ctpw", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_weight, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpw", field);
 
-  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::sINT,
+  field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::Type::sINT,
                        &LogAccess::marshal_client_http_transaction_priority_dependence, &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ctpd", field);
 
-  field = new LogField("proxy_protocol_version", "ppv", LogField::STRING, &LogAccess::marshal_proxy_protocol_version,
+  field = new LogField("proxy_protocol_version", "ppv", LogField::Type::STRING, &LogAccess::marshal_proxy_protocol_version,
                        &LogAccess::unmarshal_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppv", field);
 
-  field = new LogField("proxy_protocol_src_ip", "pps", LogField::IP, &LogAccess::marshal_proxy_protocol_src_ip,
+  field = new LogField("proxy_protocol_src_ip", "pps", LogField::Type::IP, &LogAccess::marshal_proxy_protocol_src_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppsip", field);
 
-  field = new LogField("proxy_protocol_dst_ip", "ppd", LogField::IP, &LogAccess::marshal_proxy_protocol_dst_ip,
+  field = new LogField("proxy_protocol_dst_ip", "ppd", LogField::Type::IP, &LogAccess::marshal_proxy_protocol_dst_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("ppdip", field);

Review Comment:
   Pre-exiting issue prior to this change. I'll fix it in other PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#issuecomment-4794227005

   [approve ci autest 0of4]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#issuecomment-4784466573

   [approve ci autest 1of4]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#discussion_r3376954438


##########
src/proxy/logging/LogAccess.cc:
##########
@@ -1075,6 +1075,7 @@ LogAccess::unmarshal_http_version(char **buf, char *dest, int len)
     DBG_UNMARSHAL_DEST_OVERRUN
     return -1;
   }
+  p += res2;

Review Comment:
   Good catch. I spawn out it from this PR. I'll update this with addressing some Copilot comments.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 merged PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#discussion_r3346413733


##########
tests/gold_tests/logging/binary_log_v3.test.py:
##########
@@ -0,0 +1,145 @@
+'''
+End-to-end test of the self-describing v3 binary log format.
+'''
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+

Review Comment:
   This new test file does not start with the Apache license header: the module docstring precedes it. ATS requires new source/test files to start with the license header, so please move the docstring below the header.



##########
src/proxy/logging/YamlLogConfig.cc:
##########
@@ -233,11 +234,26 @@ YamlLogConfig::decodeLogObject(const YAML::Node &node)
     }
   }
 
+  // On-disk binary segment version (default: current). 2 emits the pre-v3
+  // layout for parsers that don't yet understand v3; binary logs only.
+  int binary_log_version = LOG_SEGMENT_VERSION;
+  if (node["binary_log_version"]) {
+    binary_log_version = node["binary_log_version"].as<int>();
+    if (!log_segment_version_supported(binary_log_version)) {
+      Warning("Invalid binary_log_version '%d' (supported %d-%d); using %d", binary_log_version, LOG_SEGMENT_VERSION_MIN_SUPPORTED,
+              LOG_SEGMENT_VERSION, LOG_SEGMENT_VERSION);
+      binary_log_version = LOG_SEGMENT_VERSION;
+    } else if (file_type != LOG_FILE_BINARY) {
+      Warning("binary_log_version only applies to binary logs; ignoring for this object");
+    }
+  }

Review Comment:
   The code warns that binary_log_version is ignored for non-binary logs, but it still passes the value into the LogObject ctor. That makes the warning inaccurate and can propagate the setting to ASCII/pipe objects unintentionally.



##########
src/traffic_logcat/logcat.cc:
##########
@@ -120,6 +124,34 @@ follow_rotate(const char *input_file, ino_t old_inode_num)
   }
 }
 
+// Emit every entry of a v3 segment as a line of JSON. v2 segments lack the
+// field-type schema needed for self-describing decode, so they are skipped
+// with a note. Returns the number of entries written.
+int
+write_json_logbuffer(LogBufferHeader *header, int out_fd)
+{
+  if (header->fmt_fieldtypes() == nullptr) {
+    fprintf(stderr, "JSON output requires a v3 binary log with a field-type schema; skipping segment.\n");
+    return 0;
+  }
+
+  LogBufferIterator iter(header);
+  LogEntryHeader   *entry;
+  char              line[LOG_MAX_FORMATTED_LINE];
+  int               count = 0;
+
+  while ((entry = iter.next()) != nullptr) {
+    int n = log_entry_to_json(entry, header, line, sizeof(line) - 1);
+    if (n > 0) {
+      line[n] = '\n';
+      if (write(out_fd, line, n + 1) > 0) {
+        ++count;
+      }
+    }
+  }

Review Comment:
   write_json_logbuffer() treats any positive write() result as success, but write() can return a short write. This can silently truncate JSON output (especially when writing to a pipe) while still incrementing count.



##########
src/traffic_logcat/LogEntryJson.cc:
##########
@@ -0,0 +1,247 @@
+/** @file
+
+  Reference decoder for the self-describing v3 binary log format.
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+
+#include "LogEntryJson.h"
+
+#include "proxy/logging/LogBuffer.h"
+#include "proxy/logging/LogAccess.h"
+#include "proxy/logging/LogField.h"
+
+// Deliberately not including the global field table (proxy/logging/Log.h):
+// decoding must depend only on the segment's schema (see LogEntryJson.h).
+
+#include "tscore/ink_inet.h"
+#include "tscore/ink_align.h"
+
+#include <cinttypes>
+#include <cstdint>
+#include <cstdio>
+#include <cstring>
+
+namespace
+{
+// fmt_fieldlist symbols are comma-separated (e.g. "chi,cqu,pssc"); tolerate
+// spaces too.
+bool
+is_field_sep(char c)
+{
+  return c == ',' || c == ' ';
+}
+} // namespace
+
+int
+log_entry_to_json(LogEntryHeader *entry, LogBufferHeader *header, char *buf, int buf_len)
+{
+  if (entry == nullptr || header == nullptr || buf == nullptr || buf_len <= 0) {
+    return -1;
+  }
+
+  // [seg_start, seg_end) is the only readable region (byte_count == segment size).
+  char *seg_start = reinterpret_cast<char *>(header);
+  char *seg_end   = seg_start + header->byte_count;
+
+  // v3 decode needs the field-type schema and the symbols.
+  char *schema_blob = header->fmt_fieldtypes();
+  char *symbols     = header->fmt_fieldlist();
+  if (schema_blob == nullptr || symbols == nullptr) {
+    return -1;
+  }
+  // Both must lie within the segment, including the fixed schema prefix.
+  if (schema_blob < seg_start || schema_blob + sizeof(LogFieldTypeSchema) > seg_end || symbols < seg_start || symbols >= seg_end) {
+    return -1;
+  }
+
+  // field_count is a uint16 that may sit at an unaligned offset (the writer
+  // places the schema right after the NUL-terminated header strings), so read
+  // it byte-wise rather than through the struct.
+  uint16_t fc16 = 0;
+  memcpy(&fc16, schema_blob, sizeof(fc16));
+  const unsigned field_count = fc16;
+  const uint8_t *codes       = reinterpret_cast<const uint8_t *>(schema_blob) + sizeof(LogFieldTypeSchema);
+  if (reinterpret_cast<const char *>(codes) + field_count > seg_end) {
+    return -1;
+  }
+
+  // field_count must match the symbol count; a mismatch is a corrupt segment.
+  unsigned symbol_count = 0;
+  bool     in_token     = false;
+  for (const char *p = symbols; p < seg_end && *p != '\0'; ++p) {
+    if (is_field_sep(*p)) {
+      in_token = false;
+    } else if (!in_token) {
+      in_token = true;
+      ++symbol_count;
+    }
+  }
+  if (symbol_count != field_count) {
+    return -1;
+  }
+
+  // Clamp value reads to this entry (or the segment, if entry_len is bogus).
+  char *entry_start = reinterpret_cast<char *>(entry);
+  if (entry_start < seg_start || entry_start + sizeof(LogEntryHeader) > seg_end) {
+    return -1;
+  }
+  char *read_from = entry_start + sizeof(LogEntryHeader);
+  char *read_end  = entry_start + entry->entry_len;
+  if (read_end < read_from || read_end > seg_end) {
+    read_end = seg_end;
+  }
+
+  int written = 0;
+
+  // Append n bytes, keeping one byte in reserve for the trailing NUL.
+  auto put = [&](const char *src, int n) -> bool {
+    if (n < 0 || written + n >= buf_len) {
+      return false;
+    }
+    memcpy(buf + written, src, n);
+    written += n;
+    return true;
+  };
+  auto put_ch = [&](char c) -> bool { return put(&c, 1); };
+
+  if (!put_ch('{')) {
+    return -1;
+  }
+
+  const char *sym = symbols;
+  for (unsigned i = 0; i < field_count; ++i) {
+    // Next symbol token (comma-separated in fmt_fieldlist), bounded by the segment.
+    while (sym < seg_end && is_field_sep(*sym)) {
+      ++sym;
+    }
+    const char *sym_start = sym;
+    while (sym < seg_end && *sym != '\0' && !is_field_sep(*sym)) {
+      ++sym;
+    }
+    int sym_len = static_cast<int>(sym - sym_start);
+
+    if (i > 0 && !put_ch(',')) {
+      return -1;
+    }
+    if (!put_ch('"') || !put(sym_start, sym_len) || !put_ch('"') || !put_ch(':')) {
+      return -1;
+    }
+
+    // Dispatch on the framing type only, never the symbol; no field semantics
+    // (see the contract in LogEntryJson.h).
+    switch (static_cast<LogField::Type>(codes[i])) {
+    case LogField::Type::sINT: {
+      if (read_from + INK_MIN_ALIGN > read_end) {
+        return -1;
+      }
+      int64_t v = LogAccess::unmarshal_int(&read_from);
+      char    num[24]; // INT64_MIN is 20 digits + sign + NUL
+      int     n = snprintf(num, sizeof(num), "%" PRId64, v);
+      if (!put(num, n)) {
+        return -1;
+      }
+      break;
+    }
+    case LogField::Type::STRING: {
+      // NUL-terminated, 8-byte padded; require the terminator and padded field
+      // within the entry.
+      char *nul = static_cast<char *>(memchr(read_from, '\0', static_cast<size_t>(read_end - read_from)));
+      if (nul == nullptr) {
+        return -1;
+      }
+      const char *s          = read_from;
+      size_t      padded_len = INK_ALIGN_DEFAULT(static_cast<size_t>(nul - read_from) + 1);
+      if (read_from + padded_len > read_end) {
+        return -1;
+      }
+      read_from += padded_len;
+      if (!put_ch('"')) {
+        return -1;
+      }
+      for (const char *p = s; p < nul; ++p) {
+        // Minimal JSON escaping for structural characters.
+        if (*p == '"' || *p == '\\') {
+          if (!put_ch('\\')) {
+            return -1;
+          }
+        }
+        if (!put_ch(*p)) {
+          return -1;
+        }
+      }

Review Comment:
   log_entry_to_json() only escapes '"' and '\\' inside STRING fields. JSON also requires escaping control characters like newlines, tabs, and other bytes < 0x20; otherwise the output is not valid JSON for some log values.



##########
src/proxy/logging/LogBuffer.cc:
##########
@@ -88,6 +88,16 @@ LogBufferHeader::fmt_printf()
   return addr;
 }
 
+char *
+LogBufferHeader::fmt_fieldtypes()
+{
+  char *addr = nullptr;
+  if (fmt_fieldtypes_offset) {
+    addr = reinterpret_cast<char *>(this) + fmt_fieldtypes_offset;
+  }
+  return addr;
+}

Review Comment:
   LogBufferHeader::fmt_fieldtypes() is documented as returning nullptr for v2 segments, but the implementation only checks fmt_fieldtypes_offset. When reading a v2 header by version-sized read (not sizeof(LogBufferHeader)), fmt_fieldtypes_offset is not initialized from disk, so callers can get a non-null pointer for v2.



##########
include/ts/apidefs.h.in:
##########
@@ -1650,10 +1650,10 @@ struct TSResponseAction {
 };
 
 enum TSLogType {
-  TS_LOG_TYPE_INT,
+  TS_LOG_TYPE_INT = 1, ///< LogField::Type::sINT
   // DINT is omitted from the public API for now, until we decide whether we keep the type
-  TS_LOG_TYPE_STRING = 2,
-  TS_LOG_TYPE_ADDR   = 3,
+  TS_LOG_TYPE_STRING = 3, ///< LogField::Type::STRING
+  TS_LOG_TYPE_ADDR   = 4, ///< LogField::Type::IP
 };

Review Comment:
   TSLogType enum values are renumbered (INT=1, STRING=3, ADDR=4) to match LogField::Type. This is an ABI-breaking change for existing plugins compiled against older headers (which passed 0/1/2) and can cause TSLogFieldRegister() to reject or misinterpret the type.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#discussion_r3346543467


##########
include/ts/apidefs.h.in:
##########
@@ -1650,10 +1650,10 @@ struct TSResponseAction {
 };
 
 enum TSLogType {
-  TS_LOG_TYPE_INT,
+  TS_LOG_TYPE_INT = 1, ///< LogField::Type::sINT
   // DINT is omitted from the public API for now, until we decide whether we keep the type
-  TS_LOG_TYPE_STRING = 2,
-  TS_LOG_TYPE_ADDR   = 3,
+  TS_LOG_TYPE_STRING = 3, ///< LogField::Type::STRING
+  TS_LOG_TYPE_ADDR   = 4, ///< LogField::Type::IP
 };

Review Comment:
   This renumbering is fine because `TSLogType` is going to be released by 11.0.0, it's not published yet.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#issuecomment-4776496968

   [approve ci autest 0of4 1of4 debian]


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#issuecomment-4959152161

   Backported to 10.2.x by #13339 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#discussion_r3374271953


##########
src/proxy/logging/LogAccess.cc:
##########
@@ -1075,6 +1075,7 @@ LogAccess::unmarshal_http_version(char **buf, char *dest, int len)
     DBG_UNMARSHAL_DEST_OVERRUN
     return -1;
   }
+  p += res2;

Review Comment:
   This look like your #13236 fix. Probably want to keep that separate to avoid conflicts?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#discussion_r3379335206


##########
src/proxy/logging/Log.cc:
##########
@@ -740,58 +758,59 @@ Log::init_fields()
                            SQUID_MISS_ERROR, "MISS_ERROR", SQUID_MISS_CACHE_BYPASS, "MISS_CACHE_BYPASS",
                            SQUID_HIT_MISS_INVALID_ASSIGNED_CODE, "INVALID_CODE");
 
-  field = new LogField("cache_result_code", "crc", LogField::sINT, &LogAccess::marshal_cache_result_code,
+  field = new LogField("cache_result_code", "crc", LogField::Type::sINT, &LogAccess::marshal_cache_result_code,
                        &LogAccess::unmarshal_cache_code, make_alias_map(cache_code_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crc", field);
 
   // Reuse the unmarshalling code from crc
-  field = new LogField("cache_result_subcode", "crsc", LogField::sINT, &LogAccess::marshal_cache_result_subcode,
+  field = new LogField("cache_result_subcode", "crsc", LogField::Type::sINT, &LogAccess::marshal_cache_result_subcode,
                        &LogAccess::unmarshal_cache_code, make_alias_map(cache_subcode_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("crsc", field);
 
-  field = new LogField("cache_hit_miss", "chm", LogField::sINT, &LogAccess::marshal_cache_hit_miss,
+  field = new LogField("cache_hit_miss", "chm", LogField::Type::sINT, &LogAccess::marshal_cache_hit_miss,
                        &LogAccess::unmarshal_cache_hit_miss, make_alias_map(cache_hit_miss_map));
   global_field_list.add(field, false);
   field_symbol_hash.emplace("chm", field);
 
-  field = new LogField("proxy_response_all_header_fields", "psah", LogField::STRING,
+  field = new LogField("proxy_response_all_header_fields", "psah", LogField::Type::STRING,
                        &LogAccess::marshal_proxy_resp_all_header_fields, &LogUtils::unmarshalMimeHdr);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("psah", field);
 
   // proxy -> server fields
-  field = new LogField("proxy_req_header_len", "pqhl", LogField::sINT, &LogAccess::marshal_proxy_req_header_len,
+  field = new LogField("proxy_req_header_len", "pqhl", LogField::Type::sINT, &LogAccess::marshal_proxy_req_header_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqhl", field);
 
-  field = new LogField("proxy_req_squid_len", "pqql", LogField::sINT, &LogAccess::marshal_proxy_req_squid_len,
+  field = new LogField("proxy_req_squid_len", "pqql", LogField::Type::sINT, &LogAccess::marshal_proxy_req_squid_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqql", field);
 
-  field = new LogField("proxy_req_content_len", "pqcl", LogField::sINT, &LogAccess::marshal_proxy_req_content_len,
+  field = new LogField("proxy_req_content_len", "pqcl", LogField::Type::sINT, &LogAccess::marshal_proxy_req_content_len,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqcl", field);
 
-  field = new LogField("proxy_req_server_ip", "pqsi", LogField::IP, &LogAccess::marshal_proxy_req_server_ip,
+  field = new LogField("proxy_req_server_ip", "pqsi", LogField::Type::IP, &LogAccess::marshal_proxy_req_server_ip,
                        &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqsi", field);
 
-  field = new LogField("proxy_req_server_port", "pqsp", LogField::sINT, &LogAccess::marshal_proxy_req_server_port,
+  field = new LogField("proxy_req_server_port", "pqsp", LogField::Type::sINT, &LogAccess::marshal_proxy_req_server_port,
                        &LogAccess::unmarshal_int_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("pqsp", field);
 
-  field = new LogField("next_hop_ip", "nhi", LogField::IP, &LogAccess::marshal_next_hop_ip, &LogAccess::unmarshal_ip_to_str);
+  field = new LogField("next_hop_ip", "nhi", LogField::Type::IP, &LogAccess::marshal_next_hop_ip, &LogAccess::unmarshal_ip_to_str);
   global_field_list.add(field, false);
   field_symbol_hash.emplace("nhi", field);
 
-  field = new LogField("next_hop_port", "nhp", LogField::IP, &LogAccess::marshal_next_hop_port, &LogAccess::unmarshal_int_to_str);
+  field =
+    new LogField("next_hop_port", "nhp", LogField::Type::IP, &LogAccess::marshal_next_hop_port, &LogAccess::unmarshal_int_to_str);

Review Comment:
   Seems another bug of unmatched type.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Self-Describing Binary Log Format (v3) [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13231:
URL: https://github.com/apache/trafficserver/pull/13231#discussion_r3471352577


##########
src/traffic_logcat/logcat.cc:
##########
@@ -120,6 +130,227 @@ follow_rotate(const char *input_file, ino_t old_inode_num)
   }
 }
 
+// Write all @a len bytes of @a buf to @a fd, retrying short writes and EINTR.
+// write() may transfer fewer bytes than requested (notably to a pipe), so a
+// single call is not enough to guarantee the whole line lands. Returns true
+// only if every byte was written.
+bool
+write_all(int fd, const char *buf, size_t len)
+{
+  size_t off = 0;
+  while (off < len) {
+    ssize_t w = write(fd, buf + off, len - off);
+    if (w < 0) {
+      if (errno == EINTR) {
+        continue;
+      }
+      return false;
+    }
+    off += static_cast<size_t>(w);
+  }
+  return true;
+}
+
+// Emit every entry of a v3 segment as a line of JSON. v2 segments lack the
+// field-type schema needed for self-describing decode, so they are skipped
+// with a note. Returns the number of entries written.
+int
+write_json_logbuffer(LogBufferHeader *header, int out_fd)
+{
+  if (header->fmt_fieldtypes() == nullptr) {
+    fprintf(stderr, "JSON output requires a v3 binary log with a field-type schema; skipping segment.\n");
+    return 0;
+  }
+
+  LogBufferIterator iter(header);
+  LogEntryHeader   *entry;
+  char              line[LOG_MAX_FORMATTED_LINE];
+  int               count = 0;

Review Comment:
   write_json_logbuffer() probes schema presence via header->fmt_fieldtypes(). That helper does unchecked pointer arithmetic on fmt_fieldtypes_offset, which is undefined behavior for corrupt/untrusted segments with an out-of-range offset. Since process_file() treats .blog input as potentially untrusted, check version/offset bounds before any pointer arithmetic.



##########
src/traffic_logcat/LogEntryJson.cc:
##########
@@ -0,0 +1,294 @@
+/** @file
+
+  Reference decoder for the self-describing v3 binary log format.
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+
+#include "LogEntryJson.h"
+
+#include "proxy/logging/LogBuffer.h"
+#include "proxy/logging/LogAccess.h"
+#include "proxy/logging/LogField.h"
+
+// Deliberately not including the global field table (proxy/logging/Log.h):
+// decoding must depend only on the segment's schema (see LogEntryJson.h).
+
+#include "tscore/ink_inet.h"
+#include "tscore/ink_align.h"
+
+#include <cinttypes>
+#include <cstdint>
+#include <cstdio>
+#include <cstring>
+
+namespace
+{
+// fmt_fieldlist symbols are comma-separated (e.g. "chi,cqu,pssc"); tolerate
+// spaces too.
+bool
+is_field_sep(char c)
+{
+  return c == ',' || c == ' ';
+}
+} // namespace
+
+int
+log_entry_to_json(LogEntryHeader *entry, LogBufferHeader *header, char *buf, int buf_len)
+{
+  if (entry == nullptr || header == nullptr || buf == nullptr || buf_len <= 0) {
+    return -1;
+  }
+
+  // [seg_start, seg_end) is the only readable region (byte_count == segment size).
+  char *seg_start = reinterpret_cast<char *>(header);
+  char *seg_end   = seg_start + header->byte_count;
+
+  // v3 decode needs the field-type schema and the symbols.
+  char *schema_blob = header->fmt_fieldtypes();
+  char *symbols     = header->fmt_fieldlist();
+  if (schema_blob == nullptr || symbols == nullptr) {
+    return -1;
+  }

Review Comment:
   log_entry_to_json() calls LogBufferHeader::fmt_fieldtypes()/fmt_fieldlist() before validating the offsets. Those helpers do unchecked pointer arithmetic (reinterpret_cast<char*>(this) + offset), which is undefined behavior if a corrupt/untrusted segment has an out-of-range offset. Since this decoder is explicitly for potentially untrusted .blog data, compute these pointers from offsets only after range-checking against byte_count.



##########
src/traffic_logcat/logcat.cc:
##########
@@ -120,6 +130,227 @@ follow_rotate(const char *input_file, ino_t old_inode_num)
   }
 }
 
+// Write all @a len bytes of @a buf to @a fd, retrying short writes and EINTR.
+// write() may transfer fewer bytes than requested (notably to a pipe), so a
+// single call is not enough to guarantee the whole line lands. Returns true
+// only if every byte was written.
+bool
+write_all(int fd, const char *buf, size_t len)
+{
+  size_t off = 0;
+  while (off < len) {
+    ssize_t w = write(fd, buf + off, len - off);
+    if (w < 0) {
+      if (errno == EINTR) {
+        continue;
+      }
+      return false;
+    }
+    off += static_cast<size_t>(w);
+  }
+  return true;
+}
+
+// Emit every entry of a v3 segment as a line of JSON. v2 segments lack the
+// field-type schema needed for self-describing decode, so they are skipped
+// with a note. Returns the number of entries written.
+int
+write_json_logbuffer(LogBufferHeader *header, int out_fd)
+{
+  if (header->fmt_fieldtypes() == nullptr) {
+    fprintf(stderr, "JSON output requires a v3 binary log with a field-type schema; skipping segment.\n");
+    return 0;
+  }
+
+  LogBufferIterator iter(header);
+  LogEntryHeader   *entry;
+  char              line[LOG_MAX_FORMATTED_LINE];
+  int               count = 0;
+
+  while ((entry = iter.next()) != nullptr) {
+    int n = log_entry_to_json(entry, header, line, sizeof(line) - 1);
+    if (n > 0) {
+      line[n] = '\n';
+      if (!write_all(out_fd, line, static_cast<size_t>(n) + 1)) {
+        fprintf(stderr, "Error writing JSON output: %s\n", strerror(errno));
+        return count;
+      }
+      ++count;
+    }
+  }
+  return count;
+}
+
+// Human-readable name for a LogBufferHeader::format_type value.
+const char *
+log_format_type_name(uint32_t format_type)
+{
+  switch (format_type) {
+  case LOG_FORMAT_CUSTOM:
+    return "CUSTOM";
+  case LOG_FORMAT_TEXT:
+    return "TEXT";
+  default:
+    return "UNKNOWN";
+  }
+}
+
+// Human-readable name for a v3 wire type code (LogField::Type).
+const char *
+log_field_type_name(uint8_t code)
+{
+  switch (static_cast<LogField::Type>(code)) {
+  case LogField::Type::sINT:
+    return "sINT";
+  case LogField::Type::dINT:
+    return "dINT";
+  case LogField::Type::STRING:
+    return "STRING";
+  case LogField::Type::IP:
+    return "IP";
+  case LogField::Type::INVALID:
+    return "INVALID";
+  default:
+    return "UNKNOWN";
+  }
+}
+
+// Return the NUL-terminated string living at @a offset within the segment, or
+// nullptr if the offset is zero, lands outside the segment, or is not
+// terminated before the segment ends. A .blog read here may be untrusted, so
+// every offset is validated against byte_count.
+const char *
+bounded_header_str(LogBufferHeader *header, uint32_t offset)
+{
+  if (offset == 0) {
+    return nullptr;
+  }
+
+  // Validate the offset against the segment size *before* forming a pointer:
+  // pointer arithmetic landing outside the object is undefined behavior even if
+  // the result is never dereferenced, and offset is untrusted here.
+  if (offset >= header->byte_count) {
+    return nullptr;
+  }
+
+  char *seg_start = reinterpret_cast<char *>(header);
+  char *seg_end   = seg_start + header->byte_count;
+  char *addr      = seg_start + offset;
+
+  if (memchr(addr, '\0', static_cast<size_t>(seg_end - addr)) == nullptr) {
+    return nullptr;
+  }
+  return addr;
+}
+
+// Print the v3 field-type schema: the field_count and, for each field, its
+// fmt_fieldlist symbol paired with its framing type. Every read is bounded
+// against the segment (mirrors the bounds checks in log_entry_to_json).
+void
+print_field_type_schema(LogBufferHeader *header, int out_fd)
+{
+  char *seg_start   = reinterpret_cast<char *>(header);
+  char *seg_end     = seg_start + header->byte_count;
+  char *schema_blob = header->fmt_fieldtypes();
+

Review Comment:
   print_field_type_schema() fetches schema_blob via header->fmt_fieldtypes(), which does unchecked pointer arithmetic on an untrusted offset. Even though you bounds-check schema_blob afterward, the UB may already have occurred. Derive schema_blob from fmt_fieldtypes_offset only after validating it against byte_count.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org