Copilot commented on code in PR #12872:
URL: https://github.com/apache/trafficserver/pull/12872#discussion_r2818873416
##########
src/traffic_server/traffic_server.cc:
##########
@@ -2228,6 +2228,16 @@ main(int /* argc ATS_UNUSED */, const char **argv)
pluginInitCheck.notify_one();
}
+ // Give plugins a chance to customize log fields
+ APIHook *hook = g_lifecycle_hooks->get(TS_LIFECYCLE_LOG_INITIAZLIED_HOOK);
+ while (hook) {
+ hook->invoke(TS_EVENT_LIFECYCLE_LOG_INITIAZLIED, nullptr);
Review Comment:
The event constant contains a spelling error: "INITIAZLIED" should be
"INITIALIZED". This should be corrected to TS_EVENT_LIFECYCLE_LOG_INITIALIZED
to match the documentation.
```suggestion
APIHook *hook =
g_lifecycle_hooks->get(TS_LIFECYCLE_LOG_INITIALIZED_HOOK);
while (hook) {
hook->invoke(TS_EVENT_LIFECYCLE_LOG_INITIALIZED, nullptr);
```
##########
include/ts/apidefs.h.in:
##########
@@ -361,6 +362,7 @@ enum TSEvent {
TS_EVENT_LIFECYCLE_MSG = 60105,
TS_EVENT_LIFECYCLE_TASK_THREADS_READY = 60106,
TS_EVENT_LIFECYCLE_SHUTDOWN = 60107,
+ TS_EVENT_LIFECYCLE_LOG_INITIAZLIED = 60108,
Review Comment:
The event name contains a spelling error: "INITIAZLIED" should be
"INITIALIZED". This typo appears in the constant name
TS_EVENT_LIFECYCLE_LOG_INITIAZLIED and needs to be corrected for consistency
with the documentation which uses the correct spelling.
```suggestion
TS_EVENT_LIFECYCLE_LOG_INITIALIZED = 60108,
```
##########
include/ts/apidefs.h.in:
##########
@@ -578,6 +580,7 @@ enum TSLifecycleHookID {
TS_LIFECYCLE_TASK_THREADS_READY_HOOK,
TS_LIFECYCLE_SHUTDOWN_HOOK,
TS_LIFECYCLE_SSL_SECRET_HOOK,
+ TS_LIFECYCLE_LOG_INITIAZLIED_HOOK,
Review Comment:
The hook name contains a spelling error: "INITIAZLIED" should be
"INITIALIZED". This typo appears in the constant name
TS_LIFECYCLE_LOG_INITIAZLIED_HOOK and needs to be corrected for consistency
with the documentation which uses the correct spelling.
```suggestion
TS_LIFECYCLE_LOG_INITIALIZED_HOOK,
```
##########
src/proxy/logging/LogField.cc:
##########
@@ -287,6 +287,34 @@ LogField::LogField(const char *name, const char *symbol,
Type type, MarshalFunc
strcmp(m_symbol, "cqtn") == 0 || strcmp(m_symbol, "cqtd") ==
0 || strcmp(m_symbol, "cqtt") == 0);
}
+LogField::LogField(const char *name, const char *symbol, Type type,
CustomMarshalFunc custom_marshal,
+ CustomUnmarshalFunc custom_unmarshal)
+ : m_name(ats_strdup(name)),
+ m_symbol(ats_strdup(symbol)),
+ m_type(type),
+ m_container(NO_CONTAINER),
+ m_marshal_func(nullptr),
+ m_unmarshal_func(VarUnmarshalFunc(nullptr)),
+ m_agg_op(NO_AGGREGATE),
+ m_agg_cnt(0),
+ m_agg_val(0),
+ m_milestone1(TS_MILESTONE_LAST_ENTRY),
+ m_milestone2(TS_MILESTONE_LAST_ENTRY),
+ m_time_field(false),
+ m_alias_map(nullptr),
+ m_set_func(nullptr),
+ m_custom_marshal_func(custom_marshal),
+ m_custom_unmarshal_func(custom_unmarshal)
+{
+ ink_assert(m_name != nullptr);
+ ink_assert(m_symbol != nullptr);
+ ink_assert(m_type >= 0 && m_type < N_TYPES);
+ ink_assert(m_marshal_func != (MarshalFunc) nullptr);
Review Comment:
This assertion will always fail for custom fields because m_marshal_func is
explicitly set to nullptr on line 296. The assertion should either be removed
or changed to check m_custom_marshal_func instead when m_marshal_func is
nullptr.
```suggestion
ink_assert(m_custom_marshal_func != nullptr);
```
##########
doc/developer-guide/api/functions/TSLogFieldRegister.en.rst:
##########
@@ -0,0 +1,99 @@
+.. 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
+
+.. default-domain:: cpp
+
+TSLogFieldRegister
+******************
+
+Registers a custom log field, or modifies an existing log field with a new
definition.
+
+Synopsis
+========
+
+.. code-block:: cpp
+
+ #include <ts/ts.h>
+
+.. function:: TSReturnCode TSLogFieldRegister(std::string_view name,
std::string_view symbol, TSLogType type, TSLogMarshalCallback marshal_cb,
TSLogUnmarshalCallback unmarshal_cb, bool replace = false);
+
+.. enum:: TSLogType
+
+ Specify the type of a log field
+
+ .. enumerator:: TS_LOG_TYPE_INT
+
+ Integer field.
+
+ .. enumerator:: TS_LOG_TYPE_STRING
+
+ String field.
+
+ .. enumerator:: TS_LOG_TYPE_ADDR
+
+ Address field. It supports IPv4 address, IPv6 address, and Unix Domain
Socket address (path).
+
+.. type:: int (*TSLogMarshalCallback)(TSHttpTxn, char *);
+
+ Callback sginature for functions to marshal log fields.
+
+.. type:: std::tuple<int, int> (*TSLogUnmarshalCallback)(char **, char *, int);
+
+ Callback sginature for functions to unmarshal log fields.
Review Comment:
Spelling error: "sginature" should be "signature".
```suggestion
Callback signature for functions to marshal log fields.
.. type:: std::tuple<int, int> (*TSLogUnmarshalCallback)(char **, char *,
int);
Callback signature for functions to unmarshal log fields.
```
##########
example/plugins/c-api/custom_logfield/custom_logfield.cc:
##########
@@ -0,0 +1,230 @@
+/** @file
+
+ This plugin counts the number of times every header has appeared.
+ Maintains separate counts for client and origin headers.
+
+ @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 <inttypes.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+
+#include <ts/ts.h>
+#include <ts/remap.h>
+
+DbgCtl dbg_ctl{"custom_logfield"};
+
+char PLUGIN_NAME[] = "custom_logfield";
+char VENDOR_NAME[] = "Apache Software Foundation";
+char SUPPORT_EMAIL[] = "[email protected]";
+char USER_ARG_CSTM[] = "cstm_field";
+char USER_ARG_CSTMI[] = "cstmi_field";
+char USER_ARG_CSSN[] = "cssn_field";
+
+int
+write_text_from_user_arg(TSHttpTxn txnp, char *buf, const char *user_arg_name)
+{
+ int len = 0;
+ int index;
+
+ if (TSUserArgIndexNameLookup(TS_USER_ARGS_TXN, user_arg_name, &index,
nullptr) == TS_SUCCESS) {
+ Dbg(dbg_ctl, "User Arg Index: %d", index);
+ if (char *value = static_cast<char *>(TSUserArgGet(txnp, index)); value) {
+ Dbg(dbg_ctl, "Value: %s", value);
+ len = strlen(value);
+ if (buf) {
+ TSstrlcpy(buf, value, len + 1);
+ }
+ }
+ }
+ return len + 1;
+}
+
+int
+marshal_function_cstm(TSHttpTxn txnp, char *buf)
+{
+ if (buf) {
+ Dbg(dbg_ctl, "Marshaling a custom field cstm");
+ } else {
+ Dbg(dbg_ctl, "Marshaling a custom field cstm for size calculation");
+ }
+ return write_text_from_user_arg(txnp, buf, USER_ARG_CSTM);
+}
+
+int
+marshal_function_cssn(TSHttpTxn txnp, char *buf)
+{
+ if (buf) {
+ Dbg(dbg_ctl, "Marshaling a built-in field cssn");
+ } else {
+ Dbg(dbg_ctl, "Marshaling a built-in field cssn for size calculation");
+ }
+ return write_text_from_user_arg(txnp, buf, USER_ARG_CSSN);
+}
+
+int
+marshal_function_cstmi(TSHttpTxn txnp, char *buf)
+{
+ // This implementation is just to demonstrate marshaling an integer value.
+ // Predefined marshal function, TSLogIntMarshal, works for simple integer
values
+
+ int index;
+
+ if (buf) {
+ Dbg(dbg_ctl, "Marshaling a custom field cstmi");
+ } else {
+ Dbg(dbg_ctl, "Marshaling a custom field cstmi for size calculation");
+ }
+
+ if (buf) {
+ if (TSUserArgIndexNameLookup(TS_USER_ARGS_TXN, USER_ARG_CSTMI, &index,
nullptr) == TS_SUCCESS) {
+ Dbg(dbg_ctl, "User Arg Index: %d", index);
+ if (int64_t value = reinterpret_cast<int64_t>(TSUserArgGet(txnp,
index)); value) {
+ Dbg(dbg_ctl, "Value: %" PRId64, value);
+ *(reinterpret_cast<int64_t *>(buf)) = value;
+ }
+ }
+ }
+ return sizeof(int64_t);
+}
+
+std::tuple<int, int>
+unmarshal_function_string(char **buf, char *dest, int len)
+{
+ Dbg(dbg_ctl, "Unmarshaling a string field");
+
+ // This implementation is just to demonstrate unmarshaling a string value.
+ // Predefined unmarshal function, TSLogStringUnmarshal, works for simple
string values
+
+ int l = strlen(*buf);
+ Dbg(dbg_ctl, "Dest buf size: %d", len);
+ Dbg(dbg_ctl, "Unmarshaled value length: %d", l);
+ if (l < len) {
+ memcpy(dest, *buf, l);
+ Dbg(dbg_ctl, "Unmarshaled value: %.*s", l, dest);
+ return {
+ l, // The length of data read from buf
+ l // The length of data written to dest
+ };
+ } else {
+ return {-1, -1};
+ }
+}
+
+int
+lifecycle_event_handler(TSCont /* contp ATS_UNUSED */, TSEvent event, void *
/* edata ATS_UNUSED */)
+{
+ TSAssert(event == TS_EVENT_LIFECYCLE_LOG_INITIAZLIED);
Review Comment:
The event constant contains a spelling error: "INITIAZLIED" should be
"INITIALIZED". This should be corrected to TS_EVENT_LIFECYCLE_LOG_INITIALIZED.
```suggestion
TSAssert(event == TS_EVENT_LIFECYCLE_LOG_INITIALIZED);
```
##########
include/ts/apidefs.h.in:
##########
@@ -1570,6 +1575,13 @@ struct TSResponseAction {
bool no_cache;
};
+enum TSLogType {
+ TS_LOG_TYPE_INT,
+ TS_LOG_TYPE_DINT,
+ TS_LOG_TYPE_STRING,
+ TS_LOG_TYPE_ADDR,
Review Comment:
TS_LOG_TYPE_DINT is defined in the enum but is not documented in the
TSLogFieldRegister API documentation. Either this type should be documented (if
it's intended for use) or it should be removed from the public API enum if it's
not meant to be used by plugins.
```suggestion
/**
* @brief Log field data types for use with TSLogFieldRegister.
*
* These values describe the type of data stored in a custom log field.
*
* - TS_LOG_TYPE_INT: Signed integer value.
* - TS_LOG_TYPE_DINT: Deprecated; reserved for backward compatibility and
* not intended for use by new plugins.
* - TS_LOG_TYPE_STRING: NUL-terminated string value.
* - TS_LOG_TYPE_ADDR: Network address value (e.g., IP address).
*/
enum TSLogType {
TS_LOG_TYPE_INT, ///< Signed integer value.
TS_LOG_TYPE_DINT, ///< Deprecated; reserved for backward compatibility;
do not use in new plugins.
TS_LOG_TYPE_STRING, ///< NUL-terminated string value.
TS_LOG_TYPE_ADDR, ///< Network address value (e.g., IP address).
```
##########
src/api/InkAPI.cc:
##########
@@ -8996,3 +8997,143 @@ TSConnectionLimitExemptListClear()
{
ConnectionTracker::clear_client_exempt_list();
}
+
+TSReturnCode
+TSLogFieldRegister(std::string_view name, std::string_view symbol, TSLogType
type, TSLogMarshalCallback marshal_cb,
+ TSLogUnmarshalCallback unmarshal_cb, bool replace)
+{
+ if (auto ite = Log::field_symbol_hash.find(symbol.data()); ite !=
Log::field_symbol_hash.end()) {
+ if (replace) {
+ // Symbol is registered and the plugin wants to replace it.
+ // Need to unregister the existing entry first.
+ Log::global_field_list.remove(ite->second);
+ Log::field_symbol_hash.erase(ite);
+ } else {
+ // Symbol conflict.
+ return TS_ERROR;
+ }
+ }
+
+ LogField *field = new LogField(name.data(), symbol.data(),
static_cast<LogField::Type>(type),
+
reinterpret_cast<LogField::CustomMarshalFunc>(marshal_cb), unmarshal_cb);
+ Log::global_field_list.add(field, false);
+ Log::field_symbol_hash.emplace(symbol.data(), field);
+
+ return TS_SUCCESS;
+}
+
+int
+TSLogStringMarshal(char *buf, std::string_view str)
+{
+ if (buf) {
+ ink_strlcpy(buf, str.data(), str.length() + 1);
+ }
+ return str.length() + 1;
+}
+
+std::tuple<int, int>
+TSLogStringUnmarshal(char **buf, char *dest, int len)
+{
+ // We cannot use LogAccess::unmarshal_str, etc. here because those internal
+ // functions take care of log buffer alignment. This function needs to be
+ // implemented as if it's a piece of code in plugin code, which is unaware
+ // of the alignment.
+ if (int l = strlen(*buf); l < len) {
+ memcpy(dest, *buf, l);
+ return {l, l};
+ } else {
+ return {-1, -1};
+ }
+}
+
+int
+TSLogIntMarshal(char *buf, int64_t value)
+{
+ if (buf) {
+ *(reinterpret_cast<int64_t *>(buf)) = value;
+ }
+ return sizeof(int64_t);
+}
+
+std::tuple<int, int>
+TSLogIntUnmarshal(char **buf, char *dest, int len)
+{
+ int64_t val = *(reinterpret_cast<int64_t *>(*buf));
+ auto [end, err] = std::to_chars(dest, dest + len, val);
+ if (err == std::errc()) {
+ *end = '\0';
+ return {sizeof(uint64_t), end - dest};
Review Comment:
Type inconsistency: this returns sizeof(uint64_t) but should return
sizeof(int64_t) for consistency with the type used on lines 9055 and 9061.
```suggestion
return {sizeof(int64_t), end - dest};
```
##########
example/plugins/c-api/custom_logfield/custom_logfield.cc:
##########
@@ -0,0 +1,230 @@
+/** @file
+
+ This plugin counts the number of times every header has appeared.
+ Maintains separate counts for client and origin headers.
Review Comment:
The file header comment is incorrect. It states "This plugin counts the
number of times every header has appeared" which does not describe what this
plugin actually does. The plugin demonstrates custom log field registration and
usage, not header counting.
```suggestion
This plugin demonstrates custom log field registration and usage.
It populates custom log fields from per-transaction user arguments.
```
##########
src/traffic_server/traffic_server.cc:
##########
@@ -2228,6 +2228,16 @@ main(int /* argc ATS_UNUSED */, const char **argv)
pluginInitCheck.notify_one();
}
+ // Give plugins a chance to customize log fields
+ APIHook *hook = g_lifecycle_hooks->get(TS_LIFECYCLE_LOG_INITIAZLIED_HOOK);
+ while (hook) {
+ hook->invoke(TS_EVENT_LIFECYCLE_LOG_INITIAZLIED, nullptr);
Review Comment:
The hook constant contains a spelling error: "INITIAZLIED" should be
"INITIALIZED". This should be corrected to TS_LIFECYCLE_LOG_INITIALIZED_HOOK to
match the documentation.
```suggestion
APIHook *hook =
g_lifecycle_hooks->get(TS_LIFECYCLE_LOG_INITIALIZED_HOOK);
while (hook) {
hook->invoke(TS_EVENT_LIFECYCLE_LOG_INITIALIZED, nullptr);
```
##########
doc/developer-guide/api/functions/TSLogFieldRegister.en.rst:
##########
@@ -0,0 +1,99 @@
+.. 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
+
+.. default-domain:: cpp
+
+TSLogFieldRegister
+******************
+
+Registers a custom log field, or modifies an existing log field with a new
definition.
+
+Synopsis
+========
+
+.. code-block:: cpp
+
+ #include <ts/ts.h>
+
+.. function:: TSReturnCode TSLogFieldRegister(std::string_view name,
std::string_view symbol, TSLogType type, TSLogMarshalCallback marshal_cb,
TSLogUnmarshalCallback unmarshal_cb, bool replace = false);
+
+.. enum:: TSLogType
+
+ Specify the type of a log field
+
+ .. enumerator:: TS_LOG_TYPE_INT
+
+ Integer field.
+
+ .. enumerator:: TS_LOG_TYPE_STRING
+
+ String field.
+
+ .. enumerator:: TS_LOG_TYPE_ADDR
+
+ Address field. It supports IPv4 address, IPv6 address, and Unix Domain
Socket address (path).
+
+.. type:: int (*TSLogMarshalCallback)(TSHttpTxn, char *);
+
+ Callback sginature for functions to marshal log fields.
+
+.. type:: std::tuple<int, int> (*TSLogUnmarshalCallback)(char **, char *, int);
+
+ Callback sginature for functions to unmarshal log fields.
Review Comment:
Spelling error: "sginature" should be "signature".
```suggestion
Callback signature for functions to marshal log fields.
.. type:: std::tuple<int, int> (*TSLogUnmarshalCallback)(char **, char *,
int);
Callback signature for functions to unmarshal log fields.
```
##########
src/api/InkAPI.cc:
##########
@@ -8996,3 +8997,143 @@ TSConnectionLimitExemptListClear()
{
ConnectionTracker::clear_client_exempt_list();
}
+
+TSReturnCode
+TSLogFieldRegister(std::string_view name, std::string_view symbol, TSLogType
type, TSLogMarshalCallback marshal_cb,
+ TSLogUnmarshalCallback unmarshal_cb, bool replace)
+{
+ if (auto ite = Log::field_symbol_hash.find(symbol.data()); ite !=
Log::field_symbol_hash.end()) {
+ if (replace) {
+ // Symbol is registered and the plugin wants to replace it.
+ // Need to unregister the existing entry first.
+ Log::global_field_list.remove(ite->second);
+ Log::field_symbol_hash.erase(ite);
+ } else {
+ // Symbol conflict.
+ return TS_ERROR;
+ }
+ }
+
+ LogField *field = new LogField(name.data(), symbol.data(),
static_cast<LogField::Type>(type),
+
reinterpret_cast<LogField::CustomMarshalFunc>(marshal_cb), unmarshal_cb);
+ Log::global_field_list.add(field, false);
+ Log::field_symbol_hash.emplace(symbol.data(), field);
+
+ return TS_SUCCESS;
+}
+
+int
+TSLogStringMarshal(char *buf, std::string_view str)
+{
+ if (buf) {
+ ink_strlcpy(buf, str.data(), str.length() + 1);
+ }
+ return str.length() + 1;
+}
+
+std::tuple<int, int>
+TSLogStringUnmarshal(char **buf, char *dest, int len)
+{
+ // We cannot use LogAccess::unmarshal_str, etc. here because those internal
+ // functions take care of log buffer alignment. This function needs to be
+ // implemented as if it's a piece of code in plugin code, which is unaware
+ // of the alignment.
+ if (int l = strlen(*buf); l < len) {
+ memcpy(dest, *buf, l);
+ return {l, l};
+ } else {
+ return {-1, -1};
+ }
+}
+
+int
+TSLogIntMarshal(char *buf, int64_t value)
+{
+ if (buf) {
+ *(reinterpret_cast<int64_t *>(buf)) = value;
+ }
+ return sizeof(int64_t);
+}
+
+std::tuple<int, int>
+TSLogIntUnmarshal(char **buf, char *dest, int len)
+{
+ int64_t val = *(reinterpret_cast<int64_t *>(*buf));
+ auto [end, err] = std::to_chars(dest, dest + len, val);
+ if (err == std::errc()) {
+ *end = '\0';
+ return {sizeof(uint64_t), end - dest};
+ }
+
+ return {-1, -1};
+}
+
+int
+TSLogAddrMarshal(char *buf, sockaddr *addr)
+{
+ LogFieldIpStorage data;
+ int len = sizeof(data._ip);
+
+ if (nullptr == addr) {
+ data._ip._family = AF_UNSPEC;
+ } else if (ats_is_ip4(addr)) {
+ if (buf) {
+ data._ip4._family = AF_INET;
+ data._ip4._addr = ats_ip4_addr_cast(addr);
+ }
+ len = sizeof(data._ip4);
+ } else if (ats_is_ip6(addr)) {
+ if (buf) {
+ data._ip6._family = AF_INET6;
+ data._ip6._addr = ats_ip6_addr_cast(addr);
+ }
+ len = sizeof(data._ip6);
+ } else if (ats_is_unix(addr)) {
+ if (buf) {
+ data._un._family = AF_UNIX;
+ strncpy(data._un._path, ats_unix_cast(addr)->sun_path, TS_UNIX_SIZE);
+ }
+ len = sizeof(data._un);
+ } else {
+ data._ip._family = AF_UNSPEC;
+ }
+
+ if (buf) {
+ memcpy(buf, &data, len);
+ }
+ return len;
+}
+
+std::tuple<int, int>
+TSLogAddrUnmarshal(char **buf, char *dest, int len)
+{
+ IpEndpoint endpoint;
+ int read_len = sizeof(LogFieldIp);
+
+ LogFieldIp *raw = reinterpret_cast<LogFieldIp *>(*buf);
+ if (AF_INET == raw->_family) {
+ LogFieldIp4 *ip4 = static_cast<LogFieldIp4 *>(raw);
+ ats_ip4_set(&endpoint, ip4->_addr);
+ read_len = sizeof(*ip4);
+ } else if (AF_INET6 == raw->_family) {
+ LogFieldIp6 *ip6 = static_cast<LogFieldIp6 *>(raw);
+ ats_ip6_set(&endpoint, ip6->_addr);
+ read_len = sizeof(*ip6);
+ } else if (AF_UNIX == raw->_family) {
+ LogFieldUn *un = static_cast<LogFieldUn *>(raw);
+ ats_unix_set(&endpoint, un->_path, TS_UNIX_SIZE);
+ read_len = sizeof(*un);
+ } else {
+ ats_ip_invalidate(&endpoint);
+ }
+
+ if (!ats_is_ip(&endpoint) && !ats_is_unix(&endpoint)) {
+ *dest = '0';
Review Comment:
The first assignment on line 9131 sets *dest to '0', but it's immediately
overwritten on line 9132 with '\0', making the first assignment pointless. If
the intent was to write "0" as a string, the code should be: dest[0] = '0';
dest[1] = '\0'; Otherwise, just remove line 9131.
```suggestion
```
##########
example/plugins/c-api/custom_logfield/custom_logfield.cc:
##########
@@ -0,0 +1,230 @@
+/** @file
+
+ This plugin counts the number of times every header has appeared.
+ Maintains separate counts for client and origin headers.
+
+ @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 <inttypes.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+
+#include <ts/ts.h>
+#include <ts/remap.h>
+
+DbgCtl dbg_ctl{"custom_logfield"};
+
+char PLUGIN_NAME[] = "custom_logfield";
+char VENDOR_NAME[] = "Apache Software Foundation";
+char SUPPORT_EMAIL[] = "[email protected]";
+char USER_ARG_CSTM[] = "cstm_field";
+char USER_ARG_CSTMI[] = "cstmi_field";
+char USER_ARG_CSSN[] = "cssn_field";
+
+int
+write_text_from_user_arg(TSHttpTxn txnp, char *buf, const char *user_arg_name)
+{
+ int len = 0;
+ int index;
+
+ if (TSUserArgIndexNameLookup(TS_USER_ARGS_TXN, user_arg_name, &index,
nullptr) == TS_SUCCESS) {
+ Dbg(dbg_ctl, "User Arg Index: %d", index);
+ if (char *value = static_cast<char *>(TSUserArgGet(txnp, index)); value) {
+ Dbg(dbg_ctl, "Value: %s", value);
+ len = strlen(value);
+ if (buf) {
+ TSstrlcpy(buf, value, len + 1);
+ }
+ }
+ }
+ return len + 1;
+}
+
+int
+marshal_function_cstm(TSHttpTxn txnp, char *buf)
+{
+ if (buf) {
+ Dbg(dbg_ctl, "Marshaling a custom field cstm");
+ } else {
+ Dbg(dbg_ctl, "Marshaling a custom field cstm for size calculation");
+ }
+ return write_text_from_user_arg(txnp, buf, USER_ARG_CSTM);
+}
+
+int
+marshal_function_cssn(TSHttpTxn txnp, char *buf)
+{
+ if (buf) {
+ Dbg(dbg_ctl, "Marshaling a built-in field cssn");
+ } else {
+ Dbg(dbg_ctl, "Marshaling a built-in field cssn for size calculation");
+ }
+ return write_text_from_user_arg(txnp, buf, USER_ARG_CSSN);
+}
+
+int
+marshal_function_cstmi(TSHttpTxn txnp, char *buf)
+{
+ // This implementation is just to demonstrate marshaling an integer value.
+ // Predefined marshal function, TSLogIntMarshal, works for simple integer
values
+
+ int index;
+
+ if (buf) {
+ Dbg(dbg_ctl, "Marshaling a custom field cstmi");
+ } else {
+ Dbg(dbg_ctl, "Marshaling a custom field cstmi for size calculation");
+ }
+
+ if (buf) {
+ if (TSUserArgIndexNameLookup(TS_USER_ARGS_TXN, USER_ARG_CSTMI, &index,
nullptr) == TS_SUCCESS) {
+ Dbg(dbg_ctl, "User Arg Index: %d", index);
+ if (int64_t value = reinterpret_cast<int64_t>(TSUserArgGet(txnp,
index)); value) {
+ Dbg(dbg_ctl, "Value: %" PRId64, value);
+ *(reinterpret_cast<int64_t *>(buf)) = value;
+ }
+ }
+ }
+ return sizeof(int64_t);
+}
+
+std::tuple<int, int>
+unmarshal_function_string(char **buf, char *dest, int len)
+{
+ Dbg(dbg_ctl, "Unmarshaling a string field");
+
+ // This implementation is just to demonstrate unmarshaling a string value.
+ // Predefined unmarshal function, TSLogStringUnmarshal, works for simple
string values
+
+ int l = strlen(*buf);
+ Dbg(dbg_ctl, "Dest buf size: %d", len);
+ Dbg(dbg_ctl, "Unmarshaled value length: %d", l);
+ if (l < len) {
+ memcpy(dest, *buf, l);
+ Dbg(dbg_ctl, "Unmarshaled value: %.*s", l, dest);
+ return {
+ l, // The length of data read from buf
+ l // The length of data written to dest
+ };
+ } else {
+ return {-1, -1};
+ }
+}
+
+int
+lifecycle_event_handler(TSCont /* contp ATS_UNUSED */, TSEvent event, void *
/* edata ATS_UNUSED */)
+{
+ TSAssert(event == TS_EVENT_LIFECYCLE_LOG_INITIAZLIED);
+
+ // This registers a custom log field "cstm".
+ Dbg(dbg_ctl, "Registering cstm log field");
+ TSLogFieldRegister("custom log field", "cstm", TS_LOG_TYPE_STRING,
marshal_function_cstm, unmarshal_function_string);
+
+ // This replaces marshaling and unmarshaling functions for a built-in log
field "cssn".
+ Dbg(dbg_ctl, "Overriding cssn log field");
+ TSLogFieldRegister("modified cssn", "cssn", TS_LOG_TYPE_STRING,
marshal_function_cssn, TSLogStringUnmarshal, true);
+
+ // This registers a custom log field "cstmi"
+ Dbg(dbg_ctl, "Registering cstmi log field");
+ TSLogFieldRegister("custom integer log field", "cstmi", TS_LOG_TYPE_INT,
marshal_function_cstmi, TSLogIntUnmarshal);
+
+ // This replaces marshaling and unmarshaling functions for a built-in log
field "chi".
+ Dbg(dbg_ctl, "Overriding chi log field");
+ TSLogFieldRegister(
+ "modified cssn", "chi", TS_LOG_TYPE_ADDR,
+ [](TSHttpTxn /* txnp */, char *buf) -> int {
+ sockaddr_in addr;
+ addr.sin_family = AF_INET;
+ addr.sin_port = htons(80);
+ addr.sin_addr.s_addr = inet_addr("192.168.0.1");
+ return TSLogAddrMarshal(buf, reinterpret_cast<sockaddr *>(&addr));
+ },
+ TSLogAddrUnmarshal, true);
+
+ return TS_SUCCESS;
+}
+
+void
+TSPluginInit(int /* argc ATS_UNUSED */, const char ** /* argv ATS_UNUSED */)
+{
+ Dbg(dbg_ctl, "Initializing plugin");
+
+ TSPluginRegistrationInfo info = {PLUGIN_NAME, VENDOR_NAME, SUPPORT_EMAIL};
+ if (TSPluginRegister(&info) != TS_SUCCESS) {
+ TSError("[%s](%s) Plugin registration failed. \n", PLUGIN_NAME,
__FUNCTION__);
+ }
+
+ TSCont cont = TSContCreate(lifecycle_event_handler, nullptr);
+ TSLifecycleHookAdd(TS_LIFECYCLE_LOG_INITIAZLIED_HOOK, cont);
Review Comment:
The hook constant contains a spelling error: "INITIAZLIED" should be
"INITIALIZED". This should be corrected to TS_LIFECYCLE_LOG_INITIALIZED_HOOK.
##########
src/api/InkAPI.cc:
##########
@@ -8996,3 +8997,143 @@ TSConnectionLimitExemptListClear()
{
ConnectionTracker::clear_client_exempt_list();
}
+
+TSReturnCode
+TSLogFieldRegister(std::string_view name, std::string_view symbol, TSLogType
type, TSLogMarshalCallback marshal_cb,
+ TSLogUnmarshalCallback unmarshal_cb, bool replace)
+{
+ if (auto ite = Log::field_symbol_hash.find(symbol.data()); ite !=
Log::field_symbol_hash.end()) {
+ if (replace) {
+ // Symbol is registered and the plugin wants to replace it.
+ // Need to unregister the existing entry first.
+ Log::global_field_list.remove(ite->second);
+ Log::field_symbol_hash.erase(ite);
+ } else {
+ // Symbol conflict.
+ return TS_ERROR;
+ }
+ }
+
+ LogField *field = new LogField(name.data(), symbol.data(),
static_cast<LogField::Type>(type),
+
reinterpret_cast<LogField::CustomMarshalFunc>(marshal_cb), unmarshal_cb);
+ Log::global_field_list.add(field, false);
+ Log::field_symbol_hash.emplace(symbol.data(), field);
+
+ return TS_SUCCESS;
+}
+
+int
+TSLogStringMarshal(char *buf, std::string_view str)
+{
+ if (buf) {
+ ink_strlcpy(buf, str.data(), str.length() + 1);
+ }
+ return str.length() + 1;
+}
+
+std::tuple<int, int>
+TSLogStringUnmarshal(char **buf, char *dest, int len)
+{
+ // We cannot use LogAccess::unmarshal_str, etc. here because those internal
+ // functions take care of log buffer alignment. This function needs to be
+ // implemented as if it's a piece of code in plugin code, which is unaware
+ // of the alignment.
+ if (int l = strlen(*buf); l < len) {
+ memcpy(dest, *buf, l);
Review Comment:
The string copied to dest is not null-terminated. After memcpy, a null
terminator should be added to ensure dest is a valid C string. This could lead
to buffer overruns or undefined behavior when dest is used as a string.
```suggestion
// functions take care of log buffer alignment. This function needs to be
// functions take care of log buffer alignment. This function needs to be
// implemented as if it's a piece of code in plugin code, which is unaware
// of the alignment.
if (int l = strlen(*buf); l < len) {
memcpy(dest, *buf, l);
dest[l] = '\0';
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
