You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bc...@apache.org on 2022/06/15 13:27:29 UTC
[trafficserver] 04/04: remap_stats: convert to using TSHttpTxnPristineUrlGet and TSHttpTxnClientReqGet for hostname, remove pre remap continuation (#8362)
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a commit to branch 9.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 5945f96617bad43ba48228868facca9b26ecb205
Author: Brian Olsen <bn...@gmail.com>
AuthorDate: Wed Nov 10 06:47:41 2021 -0700
remap_stats: convert to using TSHttpTxnPristineUrlGet and TSHttpTxnClientReqGet for hostname, remove pre remap continuation (#8362)
also fixes and adds an autest for --post-remap-host
(cherry picked from commit 0e59150cce09ff1bb1e9714fa7238db2e0f1c475)
---
plugins/experimental/remap_stats/remap_stats.cc | 230 +++++++++------------
.../pluginTest/remap_stats/gold/metrics.gold | 2 +
.../pluginTest/remap_stats/gold/metrics_post.gold | 2 +
tests/gold_tests/pluginTest/remap_stats/metrics.sh | 33 +++
.../pluginTest/remap_stats/metrics_post.sh | 33 +++
.../pluginTest/remap_stats/remap_stats.test.py | 13 +-
...emap_stats.test.py => remap_stats_post.test.py} | 15 +-
7 files changed, 182 insertions(+), 146 deletions(-)
diff --git a/plugins/experimental/remap_stats/remap_stats.cc b/plugins/experimental/remap_stats/remap_stats.cc
index 727c752a4..39db77acc 100644
--- a/plugins/experimental/remap_stats/remap_stats.cc
+++ b/plugins/experimental/remap_stats/remap_stats.cc
@@ -34,11 +34,16 @@
#define MAX_STAT_LENGTH (1 << 8)
+enum UriType {
+ REMAP,
+ PRISTINE,
+};
+
struct config_t {
- bool post_remap_host;
- int txn_slot;
- TSStatPersistence persist_type;
- TSMutex stat_creation_mutex;
+ TSMutex stat_creation_mutex{nullptr};
+ UriType uri_type{PRISTINE};
+ TSStatPersistence persist_type{TS_STAT_NON_PERSISTENT};
+ int txn_slot{-1};
};
// From "core".... sigh, but we need it for now at least.
@@ -82,70 +87,61 @@ stat_add(const char *name, TSMgmtInt amount, TSStatPersistence persist_type, TSM
}
}
-char *
-get_effective_host(TSHttpTxn txn)
+std::string
+get_hostname(TSHttpTxn txnp, UriType uriType)
{
- char *effective_url, *tmp;
- const char *host;
- int len;
- TSMBuffer buf;
- TSMLoc url_loc;
-
- buf = TSMBufferCreate();
- if (TS_SUCCESS != TSUrlCreate(buf, &url_loc)) {
- TSDebug(DEBUG_TAG, "unable to create url");
- TSMBufferDestroy(buf);
- return nullptr;
+ std::string hostname;
+
+ switch (uriType) {
+ case PRISTINE: {
+ TSMBuffer hbuf;
+ TSMLoc hloc;
+ if (TS_SUCCESS == TSHttpTxnPristineUrlGet(txnp, &hbuf, &hloc)) {
+ int tlen = 0;
+ char const *const thost = TSUrlHostGet(hbuf, hloc, &tlen);
+ if (nullptr != thost && 0 < tlen) {
+ hostname.assign(thost, tlen);
+ }
+ TSHandleMLocRelease(hbuf, TS_NULL_MLOC, hloc);
+ }
+ } break;
+ case REMAP: {
+ TSMBuffer hbuf;
+ TSMLoc hloc;
+ if (TS_SUCCESS == TSHttpTxnClientReqGet(txnp, &hbuf, &hloc)) {
+ TSMLoc url_loc;
+ if (TS_SUCCESS == TSHttpHdrUrlGet(hbuf, hloc, &url_loc)) {
+ int tlen = 0;
+ char const *const thost = TSUrlHostGet(hbuf, url_loc, &tlen);
+ if (nullptr != thost && 0 < tlen) {
+ hostname.assign(thost, tlen);
+ }
+ TSHandleMLocRelease(hbuf, hloc, url_loc);
+ }
+ TSHandleMLocRelease(hbuf, TS_NULL_MLOC, hloc);
+ }
+ } break;
+ default:
+ break;
}
- tmp = effective_url = TSHttpTxnEffectiveUrlStringGet(txn, &len);
- TSUrlParse(buf, url_loc, (const char **)(&tmp), (const char *)(effective_url + len));
- TSfree(effective_url);
- host = TSUrlHostGet(buf, url_loc, &len);
- tmp = TSstrndup(host, len);
- TSHandleMLocRelease(buf, TS_NULL_MLOC, url_loc);
- TSMBufferDestroy(buf);
- return tmp;
-}
-
-int
-handle_read_req_hdr(TSCont cont, TSEvent event ATS_UNUSED, void *edata)
-{
- TSHttpTxn txn = static_cast<TSHttpTxn>(edata);
- config_t *config;
- void *txnd;
-
- config = static_cast<config_t *>(TSContDataGet(cont));
- txnd = (void *)get_effective_host(txn); // low bit left 0 because we do not know that remap succeeded yet
- TSUserArgSet(txn, config->txn_slot, txnd);
- TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE);
- TSDebug(DEBUG_TAG, "Read Req Handler Finished");
- return 0;
+ return hostname;
}
int
handle_post_remap(TSCont cont, TSEvent event ATS_UNUSED, void *edata)
{
- TSHttpTxn txn = static_cast<TSHttpTxn>(edata);
- config_t *config;
- void *txnd = (void *)0x01; // low bit 1 because we are post remap and thus success
-
- config = static_cast<config_t *>(TSContDataGet(cont));
-
- if (config->post_remap_host) {
- TSUserArgSet(txn, config->txn_slot, txnd);
- } else {
- txnd = (void *)((uintptr_t)txnd | (uintptr_t)TSUserArgGet(txn, config->txn_slot)); // We need the hostname pre-remap
- TSUserArgSet(txn, config->txn_slot, txnd);
- }
-
+ TSHttpTxn txn = static_cast<TSHttpTxn>(edata);
+ config_t *const config = static_cast<config_t *>(TSContDataGet(cont));
+ void *const txnd = reinterpret_cast<void *>(0x01);
+ TSUserArgSet(txn, config->txn_slot, txnd);
TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE);
TSDebug(DEBUG_TAG, "Post Remap Handler Finished");
return 0;
}
void
-create_stat_name(ts::FixedBufferWriter &stat_name, std::string_view h, std::string_view b)
+create_stat_name(ts::FixedBufferWriter &stat_name, std::string_view const h, std::string_view const b)
{
stat_name.reset().clip(1);
stat_name.print("plugin.{}.{}.{}", PLUGIN_NAME, h, b);
@@ -155,78 +151,64 @@ create_stat_name(ts::FixedBufferWriter &stat_name, std::string_view h, std::stri
int
handle_txn_close(TSCont cont, TSEvent event ATS_UNUSED, void *edata)
{
- TSHttpTxn const txn = static_cast<TSHttpTxn>(edata);
- char const *remap = nullptr;
- char *hostname = nullptr;
- char *effective_hostname = nullptr;
-
- static char const *const unknown = "unknown";
-
+ TSHttpTxn const txn = static_cast<TSHttpTxn>(edata);
config_t const *const config = static_cast<config_t *>(TSContDataGet(cont));
void const *const txnd = TSUserArgGet(txn, config->txn_slot);
- hostname = reinterpret_cast<char *>(reinterpret_cast<uintptr_t>(txnd) & ~0x01); // Get hostname
+ static std::string_view const unknown = "unknown";
+ // check remap successful
if (nullptr != txnd) {
- if (reinterpret_cast<uintptr_t>(txnd) & 0x01) // remap succeeded?
- {
- if (!config->post_remap_host) {
- remap = hostname;
- } else {
- effective_hostname = get_effective_host(txn);
- remap = effective_hostname;
- }
+ std::string const hostname = get_hostname(txn, config->uri_type);
- if (nullptr == remap) {
- remap = unknown;
- }
+ std::string_view hostsv;
+ if (!hostname.empty()) {
+ hostsv = hostname;
+ } else {
+ hostsv = unknown;
+ }
- uint64_t in_bytes = TSHttpTxnClientReqHdrBytesGet(txn);
- in_bytes += TSHttpTxnClientReqBodyBytesGet(txn);
-
- ts::LocalBufferWriter<MAX_STAT_LENGTH> stat_name;
-
- create_stat_name(stat_name, remap, "in_bytes");
- stat_add(stat_name.data(), static_cast<TSMgmtInt>(in_bytes), config->persist_type, config->stat_creation_mutex);
-
- uint64_t out_bytes = TSHttpTxnClientRespHdrBytesGet(txn);
- out_bytes += TSHttpTxnClientRespBodyBytesGet(txn);
-
- create_stat_name(stat_name, remap, "out_bytes");
- stat_add(stat_name.data(), static_cast<TSMgmtInt>(out_bytes), config->persist_type, config->stat_creation_mutex);
-
- TSMBuffer buf = nullptr;
- TSMLoc hdr_loc = nullptr;
- if (TSHttpTxnClientRespGet(txn, &buf, &hdr_loc) == TS_SUCCESS) {
- int const status_code = static_cast<int>(TSHttpHdrStatusGet(buf, hdr_loc));
- TSHandleMLocRelease(buf, TS_NULL_MLOC, hdr_loc);
-
- if (status_code < 200) {
- create_stat_name(stat_name, remap, "status_other");
- } else if (status_code <= 299) {
- create_stat_name(stat_name, remap, "status_2xx");
- } else if (status_code <= 399) {
- create_stat_name(stat_name, remap, "status_3xx");
- } else if (status_code <= 499) {
- create_stat_name(stat_name, remap, "status_4xx");
- } else if (status_code <= 599) {
- create_stat_name(stat_name, remap, "status_5xx");
- } else {
- create_stat_name(stat_name, remap, "status_other");
- }
+ uint64_t in_bytes = TSHttpTxnClientReqHdrBytesGet(txn);
+ in_bytes += TSHttpTxnClientReqBodyBytesGet(txn);
+
+ ts::LocalBufferWriter<MAX_STAT_LENGTH> stat_name;
+
+ create_stat_name(stat_name, hostsv, "in_bytes");
+ stat_add(stat_name.data(), static_cast<TSMgmtInt>(in_bytes), config->persist_type, config->stat_creation_mutex);
- stat_add(stat_name.data(), 1, config->persist_type, config->stat_creation_mutex);
+ uint64_t out_bytes = TSHttpTxnClientRespHdrBytesGet(txn);
+ out_bytes += TSHttpTxnClientRespBodyBytesGet(txn);
+
+ create_stat_name(stat_name, hostsv, "out_bytes");
+ stat_add(stat_name.data(), static_cast<TSMgmtInt>(out_bytes), config->persist_type, config->stat_creation_mutex);
+
+ TSMBuffer buf = nullptr;
+ TSMLoc hdr_loc = nullptr;
+ if (TSHttpTxnClientRespGet(txn, &buf, &hdr_loc) == TS_SUCCESS) {
+ int const status_code = static_cast<int>(TSHttpHdrStatusGet(buf, hdr_loc));
+ TSHandleMLocRelease(buf, TS_NULL_MLOC, hdr_loc);
+
+ if (status_code < 200) {
+ create_stat_name(stat_name, hostsv, "status_other");
+ } else if (status_code <= 299) {
+ create_stat_name(stat_name, hostsv, "status_2xx");
+ } else if (status_code <= 399) {
+ create_stat_name(stat_name, hostsv, "status_3xx");
+ } else if (status_code <= 499) {
+ create_stat_name(stat_name, hostsv, "status_4xx");
+ } else if (status_code <= 599) {
+ create_stat_name(stat_name, hostsv, "status_5xx");
} else {
- create_stat_name(stat_name, remap, "status_unknown");
- stat_add(stat_name.data(), 1, config->persist_type, config->stat_creation_mutex);
+ create_stat_name(stat_name, hostsv, "status_other");
}
- if (nullptr != effective_hostname) {
- TSfree(effective_hostname);
- }
- } else if (nullptr != hostname) {
- TSfree(hostname);
+ stat_add(stat_name.data(), 1, config->persist_type, config->stat_creation_mutex);
+ } else {
+ create_stat_name(stat_name, hostsv, "status_unknown");
+ stat_add(stat_name.data(), 1, config->persist_type, config->stat_creation_mutex);
}
+ } else {
+ TSDebug(DEBUG_TAG, "skipping unsuccessfully remapped transaction");
}
TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE);
@@ -240,8 +222,6 @@ void
TSPluginInit(int argc, const char *argv[])
{
TSPluginRegistrationInfo info;
- TSCont pre_remap_cont, post_remap_cont, global_cont;
-
info.plugin_name = PLUGIN_NAME;
info.vendor_name = "Apache Software Foundation";
info.support_email = "dev@trafficserver.apache.org";
@@ -255,16 +235,16 @@ TSPluginInit(int argc, const char *argv[])
}
auto config = new config_t;
- config->post_remap_host = false;
- config->persist_type = TS_STAT_NON_PERSISTENT;
config->stat_creation_mutex = TSMutexCreate();
+ config->uri_type = PRISTINE;
+ config->persist_type = TS_STAT_NON_PERSISTENT;
if (argc > 1) {
// Argument parser
for (int ii = 0; ii < argc; ++ii) {
std::string_view const arg(argv[ii]);
if (arg == "-P" || arg == "--post-remap-host") {
- config->post_remap_host = true;
+ config->uri_type = REMAP;
TSDebug(DEBUG_TAG, "Using post remap hostname");
} else if (arg == "-p" || arg == "--persistent") {
config->persist_type = TS_STAT_PERSISTENT;
@@ -275,17 +255,13 @@ TSPluginInit(int argc, const char *argv[])
TSUserArgIndexReserve(TS_USER_ARGS_TXN, PLUGIN_NAME, "txn data", &(config->txn_slot));
- if (!config->post_remap_host) {
- pre_remap_cont = TSContCreate(handle_read_req_hdr, nullptr);
- TSContDataSet(pre_remap_cont, static_cast<void *>(config));
- TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, pre_remap_cont);
- }
-
- post_remap_cont = TSContCreate(handle_post_remap, nullptr);
+ // this is to mark the transaction as successfully remapped
+ TSCont const post_remap_cont = TSContCreate(handle_post_remap, nullptr);
TSContDataSet(post_remap_cont, static_cast<void *>(config));
TSHttpHookAdd(TS_HTTP_POST_REMAP_HOOK, post_remap_cont);
- global_cont = TSContCreate(handle_txn_close, nullptr);
+ // collects stats for successful remaps
+ TSCont const global_cont = TSContCreate(handle_txn_close, nullptr);
TSContDataSet(global_cont, static_cast<void *>(config));
TSHttpHookAdd(TS_HTTP_TXN_CLOSE_HOOK, global_cont);
diff --git a/tests/gold_tests/pluginTest/remap_stats/gold/metrics.gold b/tests/gold_tests/pluginTest/remap_stats/gold/metrics.gold
new file mode 100644
index 000000000..95796745d
--- /dev/null
+++ b/tests/gold_tests/pluginTest/remap_stats/gold/metrics.gold
@@ -0,0 +1,2 @@
+plugin.remap_stats.one.status_2xx 1
+plugin.remap_stats.two.status_4xx 1
diff --git a/tests/gold_tests/pluginTest/remap_stats/gold/metrics_post.gold b/tests/gold_tests/pluginTest/remap_stats/gold/metrics_post.gold
new file mode 100644
index 000000000..fcbdfdadc
--- /dev/null
+++ b/tests/gold_tests/pluginTest/remap_stats/gold/metrics_post.gold
@@ -0,0 +1,2 @@
+plugin.remap_stats.127.0.0.1.status_2xx 1
+plugin.remap_stats.127.0.0.1.status_4xx 1
diff --git a/tests/gold_tests/pluginTest/remap_stats/metrics.sh b/tests/gold_tests/pluginTest/remap_stats/metrics.sh
new file mode 100755
index 000000000..f0faa2277
--- /dev/null
+++ b/tests/gold_tests/pluginTest/remap_stats/metrics.sh
@@ -0,0 +1,33 @@
+# 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.
+
+N=60
+while (( N > 0 ))
+do
+ rm -f metrics.out
+ traffic_ctl metric match \.remap_stats\. \
+ | grep status \
+ | sort \
+ > metrics.out
+ sleep 1
+ if diff metrics.out "${AUTEST_TEST_DIR}/gold/metrics.gold"
+ then
+ exit 0
+ fi
+ let N=N-1
+done
+echo TIMEOUT
+exit 1
diff --git a/tests/gold_tests/pluginTest/remap_stats/metrics_post.sh b/tests/gold_tests/pluginTest/remap_stats/metrics_post.sh
new file mode 100755
index 000000000..46f993ba9
--- /dev/null
+++ b/tests/gold_tests/pluginTest/remap_stats/metrics_post.sh
@@ -0,0 +1,33 @@
+# 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.
+
+N=60
+while (( N > 0 ))
+do
+ rm -f metrics.out
+ traffic_ctl metric match \.remap_stats\. \
+ | grep status \
+ | sort \
+ > metrics.out
+ sleep 1
+ if diff metrics.out "${AUTEST_TEST_DIR}/gold/metrics_post.gold"
+ then
+ exit 0
+ fi
+ let N=N-1
+done
+echo TIMEOUT
+exit 1
diff --git a/tests/gold_tests/pluginTest/remap_stats/remap_stats.test.py b/tests/gold_tests/pluginTest/remap_stats/remap_stats.test.py
index 727ab9abe..d55d17b1a 100644
--- a/tests/gold_tests/pluginTest/remap_stats/remap_stats.test.py
+++ b/tests/gold_tests/pluginTest/remap_stats/remap_stats.test.py
@@ -19,10 +19,11 @@ Test remap_stats plugin
'''
# Skip if plugins not present.
Test.SkipUnless(Condition.PluginExists('remap_stats.so'))
-Test.SkipIf(Condition.true("Test cannot deterministically wait until the stats appear"))
server = Test.MakeOriginServer("server")
+Test.Setup.Copy("metrics.sh")
+
request_header = {
"headers": "GET /argh HTTP/1.1\r\nHost: one\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n",
@@ -64,13 +65,7 @@ tr.StillRunningAfter = server
# 2 Test - Gather output
tr = Test.AddTestRun("analyze stats")
-tr.Processes.Default.Command = r'traffic_ctl metric match \.\*remap_stats\*'
+tr.Processes.Default.Command = 'bash -c ./metrics.sh'
tr.Processes.Default.Env = ts.Env
tr.Processes.Default.ReturnCode = 0
-tr.Processes.Default.TimeOut = 5
-tr.DelayStart = 15
-tr.TimeOut = 5
-tr.Processes.Default.Streams.stdout = Testers.ContainsExpression(
- "plugin.remap_stats.one.status_2xx 1", "expected 2xx on first remap")
-tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
- "plugin.remap_stats.two.status_4xx 1", "expected 4xx on second remap")
+tr.StillRunningAfter = ts
diff --git a/tests/gold_tests/pluginTest/remap_stats/remap_stats.test.py b/tests/gold_tests/pluginTest/remap_stats/remap_stats_post.test.py
similarity index 82%
copy from tests/gold_tests/pluginTest/remap_stats/remap_stats.test.py
copy to tests/gold_tests/pluginTest/remap_stats/remap_stats_post.test.py
index 727ab9abe..93e9e996e 100644
--- a/tests/gold_tests/pluginTest/remap_stats/remap_stats.test.py
+++ b/tests/gold_tests/pluginTest/remap_stats/remap_stats_post.test.py
@@ -19,10 +19,11 @@ Test remap_stats plugin
'''
# Skip if plugins not present.
Test.SkipUnless(Condition.PluginExists('remap_stats.so'))
-Test.SkipIf(Condition.true("Test cannot deterministically wait until the stats appear"))
server = Test.MakeOriginServer("server")
+Test.Setup.Copy("metrics_post.sh")
+
request_header = {
"headers": "GET /argh HTTP/1.1\r\nHost: one\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n",
@@ -31,7 +32,7 @@ server.addResponse("sessionlog.json", request_header, response_header)
ts = Test.MakeATSProcess("ts", command="traffic_manager", select_ports=True)
-ts.Disk.plugin_config.AddLine('remap_stats.so')
+ts.Disk.plugin_config.AddLine('remap_stats.so --post-remap-host')
ts.Disk.remap_config.AddLine(
"map http://one http://127.0.0.1:{0}".format(server.Variables.Port)
@@ -64,13 +65,7 @@ tr.StillRunningAfter = server
# 2 Test - Gather output
tr = Test.AddTestRun("analyze stats")
-tr.Processes.Default.Command = r'traffic_ctl metric match \.\*remap_stats\*'
+tr.Processes.Default.Command = 'bash -c ./metrics_post.sh'
tr.Processes.Default.Env = ts.Env
tr.Processes.Default.ReturnCode = 0
-tr.Processes.Default.TimeOut = 5
-tr.DelayStart = 15
-tr.TimeOut = 5
-tr.Processes.Default.Streams.stdout = Testers.ContainsExpression(
- "plugin.remap_stats.one.status_2xx 1", "expected 2xx on first remap")
-tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
- "plugin.remap_stats.two.status_4xx 1", "expected 4xx on second remap")
+tr.StillRunningAfter = ts