You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/05/04 23:34:46 UTC

[GitHub] [trafficserver] elsloo opened a new pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

elsloo opened a new pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784


   * Adds new X-Cache-Info header to the xdebug plugin
   * Injects the `X-Cache-Info` header with value of `path=/path/to/disk/from/storage/config; volume=n` where `n` us the volume number
   * Fixes a small bug inside `HttpCacheSM.h` in `get_volume_number()` that leads to `-1` being returned on cache miss
     * NOTE: this will change behavior of any plugin that relies on `TSHttpTxnInfoIntGet` with a key of `TS_TXN_INFO_CACHE_VOLUME`


-- 
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.

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



[GitHub] [trafficserver] elsloo commented on pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

Posted by GitBox <gi...@apache.org>.
elsloo commented on pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784#issuecomment-838566350


   > Looks like the autest is failing because the gold file includes the sandbox path in the output.
   
   Actually, that is not the case here. The string in the gold file, `out.gold` in this commit is:
   
   `X-Cache-Info: path=``/x_cache_info/ts/storage/cache.db; volume=0`; note the double ticks, which are used in a replacement algorithm seen here:
   
   https://bitbucket.org/autestsuite/reusable-gold-testing-system/src/e01363d98951863904e784b0a5a3b8b735c41c61/src/autest/testers/gold_file.py#lines-143
   
   This issue does not occur when I run this test locally, so I'm still trying to determine if there's something I can do with the header in `out.gold` in this commit, if it's something in the sandbox and testing process, or if there's something that needs to be adjusted in the Autest framework itself in the algorithm linked above.
   
   Any input or workarounds for this would be appreciated.


-- 
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.

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



[GitHub] [trafficserver] randall commented on a change in pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

Posted by GitBox <gi...@apache.org>.
randall commented on a change in pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784#discussion_r629709080



##########
File path: tests/gold_tests/pluginTest/xdebug/x_cache_info/x_cache_info.test.py
##########
@@ -0,0 +1,77 @@
+#  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.
+
+Test.Summary = '''
+Test xdebug plugin X-Cache-Info header
+'''
+
+server = Test.MakeOriginServer("server")
+
+request_header = {
+    "headers": "GET /argh HTTP/1.1\r\nHost: doesnotmatter\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+server.addResponse("sessionlog.json", request_header, response_header)
+
+ts = Test.MakeATSProcess("ts")
+
+ts.Disk.records_config.update({
+    'proxy.config.url_remap.remap_required': 0,
+    'proxy.config.diags.debug.enabled': 0,
+    'proxy.config.diags.debug.tags': 'http'
+})
+
+ts.Disk.plugin_config.AddLine('xdebug.so')
+
+ts.Disk.remap_config.AddLine(
+    "map http://one http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+ts.Disk.remap_config.AddLine(
+    "map http://two http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+ts.Disk.remap_config.AddLine(
+    "regex_map http://three[0-9]+ http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+
+tr = Test.AddTestRun()
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.Processes.Default.StartBefore(Test.Processes.server)
+tr.Processes.Default.Command = "cp {}/tcp_client.py {}/tcp_client.py".format(
+    Test.Variables.AtsTestToolsDir, Test.RunDirectory)
+tr.Processes.Default.ReturnCode = 0
+
+
+def sendMsg(msgFile):
+
+    tr = Test.AddTestRun()
+    tr.Processes.Default.Command = (
+        "( python {}/tcp_client.py 127.0.0.1 {} {}/{}.in".format(

Review comment:
       does this need to be python3? (like x_remap.test.py)?\




-- 
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.

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



[GitHub] [trafficserver] randall merged pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

Posted by GitBox <gi...@apache.org>.
randall merged pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784


   


-- 
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.

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



[GitHub] [trafficserver] elsloo edited a comment on pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

Posted by GitBox <gi...@apache.org>.
elsloo edited a comment on pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784#issuecomment-833718170


   > Could you add an Au test or add to x_remap.test.py?
   
   Done. I added a separate `x_cache_info` Au test that's mostly a copy of `x_remap`.


-- 
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.

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



[GitHub] [trafficserver] shinrich commented on pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784#issuecomment-837474874


   Looks like the autest is failing because the gold file includes the sandbox path in the output.


-- 
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.

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



[GitHub] [trafficserver] elsloo commented on pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

Posted by GitBox <gi...@apache.org>.
elsloo commented on pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784#issuecomment-837204024


   [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.

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



[GitHub] [trafficserver] dragon512 commented on a change in pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

Posted by GitBox <gi...@apache.org>.
dragon512 commented on a change in pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784#discussion_r630496702



##########
File path: tests/gold_tests/pluginTest/xdebug/x_cache_info/x_cache_info.test.py
##########
@@ -0,0 +1,77 @@
+#  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.
+
+Test.Summary = '''
+Test xdebug plugin X-Cache-Info header
+'''
+
+server = Test.MakeOriginServer("server")
+
+request_header = {
+    "headers": "GET /argh HTTP/1.1\r\nHost: doesnotmatter\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+server.addResponse("sessionlog.json", request_header, response_header)
+
+ts = Test.MakeATSProcess("ts")
+
+ts.Disk.records_config.update({
+    'proxy.config.url_remap.remap_required': 0,
+    'proxy.config.diags.debug.enabled': 0,
+    'proxy.config.diags.debug.tags': 'http'
+})
+
+ts.Disk.plugin_config.AddLine('xdebug.so')
+
+ts.Disk.remap_config.AddLine(
+    "map http://one http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+ts.Disk.remap_config.AddLine(
+    "map http://two http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+ts.Disk.remap_config.AddLine(
+    "regex_map http://three[0-9]+ http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+
+tr = Test.AddTestRun()
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.Processes.Default.StartBefore(Test.Processes.server)
+tr.Processes.Default.Command = "cp {}/tcp_client.py {}/tcp_client.py".format(
+    Test.Variables.AtsTestToolsDir, Test.RunDirectory)
+tr.Processes.Default.ReturnCode = 0
+
+
+def sendMsg(msgFile):
+
+    tr = Test.AddTestRun()
+    tr.Processes.Default.Command = (
+        "( python {}/tcp_client.py 127.0.0.1 {} {}/{}.in".format(
+            Test.RunDirectory, ts.Variables.port, Test.TestDirectory, msgFile) +
+        " ; echo '======' ) | sed 's/:{}/:SERVER_PORT/' >>  {}/out.log 2>&1 ".format(

Review comment:
       while I would rather avoid stuff like sed... you can clean this up a little be saying this:
   `
   f"sed 's/:{server.Variables.Port}/:SERVER_PORT/' >>  {Test.RunDirectory}/out.log 2>&1 "
   `
   
   
   

##########
File path: tests/gold_tests/pluginTest/xdebug/x_cache_info/x_cache_info.test.py
##########
@@ -0,0 +1,77 @@
+#  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.
+
+Test.Summary = '''
+Test xdebug plugin X-Cache-Info header
+'''
+
+server = Test.MakeOriginServer("server")
+
+request_header = {
+    "headers": "GET /argh HTTP/1.1\r\nHost: doesnotmatter\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+server.addResponse("sessionlog.json", request_header, response_header)
+
+ts = Test.MakeATSProcess("ts")
+
+ts.Disk.records_config.update({
+    'proxy.config.url_remap.remap_required': 0,
+    'proxy.config.diags.debug.enabled': 0,
+    'proxy.config.diags.debug.tags': 'http'
+})
+
+ts.Disk.plugin_config.AddLine('xdebug.so')
+
+ts.Disk.remap_config.AddLine(
+    "map http://one http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+ts.Disk.remap_config.AddLine(
+    "map http://two http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+ts.Disk.remap_config.AddLine(
+    "regex_map http://three[0-9]+ http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+
+tr = Test.AddTestRun()
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.Processes.Default.StartBefore(Test.Processes.server)
+tr.Processes.Default.Command = "cp {}/tcp_client.py {}/tcp_client.py".format(
+    Test.Variables.AtsTestToolsDir, Test.RunDirectory)
+tr.Processes.Default.ReturnCode = 0
+
+
+def sendMsg(msgFile):
+
+    tr = Test.AddTestRun()
+    tr.Processes.Default.Command = (
+        "( python {}/tcp_client.py 127.0.0.1 {} {}/{}.in".format(

Review comment:
       We should use python3 as the target just to be a little safer.




-- 
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.

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



[GitHub] [trafficserver] elsloo commented on pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

Posted by GitBox <gi...@apache.org>.
elsloo commented on pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784#issuecomment-833718170


   > Could you add an Au test or add to x_remap.test.py?
   
   Done. I added a separate `x_cach_info` Au test that's mostly a copy of `x_remap`.


-- 
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.

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



[GitHub] [trafficserver] bryancall commented on a change in pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

Posted by GitBox <gi...@apache.org>.
bryancall commented on a change in pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784#discussion_r629740578



##########
File path: tests/gold_tests/pluginTest/xdebug/x_cache_info/x_cache_info.test.py
##########
@@ -0,0 +1,77 @@
+#  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.
+
+Test.Summary = '''
+Test xdebug plugin X-Cache-Info header
+'''
+
+server = Test.MakeOriginServer("server")
+
+request_header = {
+    "headers": "GET /argh HTTP/1.1\r\nHost: doesnotmatter\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+server.addResponse("sessionlog.json", request_header, response_header)
+
+ts = Test.MakeATSProcess("ts")
+
+ts.Disk.records_config.update({
+    'proxy.config.url_remap.remap_required': 0,
+    'proxy.config.diags.debug.enabled': 0,
+    'proxy.config.diags.debug.tags': 'http'
+})
+
+ts.Disk.plugin_config.AddLine('xdebug.so')
+
+ts.Disk.remap_config.AddLine(
+    "map http://one http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+ts.Disk.remap_config.AddLine(
+    "map http://two http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+ts.Disk.remap_config.AddLine(
+    "regex_map http://three[0-9]+ http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+
+tr = Test.AddTestRun()
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.Processes.Default.StartBefore(Test.Processes.server)
+tr.Processes.Default.Command = "cp {}/tcp_client.py {}/tcp_client.py".format(
+    Test.Variables.AtsTestToolsDir, Test.RunDirectory)
+tr.Processes.Default.ReturnCode = 0
+
+
+def sendMsg(msgFile):
+
+    tr = Test.AddTestRun()
+    tr.Processes.Default.Command = (
+        "( python {}/tcp_client.py 127.0.0.1 {} {}/{}.in".format(

Review comment:
       Yes, we should use python3.




-- 
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.

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



[GitHub] [trafficserver] randall commented on a change in pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

Posted by GitBox <gi...@apache.org>.
randall commented on a change in pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784#discussion_r629709080



##########
File path: tests/gold_tests/pluginTest/xdebug/x_cache_info/x_cache_info.test.py
##########
@@ -0,0 +1,77 @@
+#  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.
+
+Test.Summary = '''
+Test xdebug plugin X-Cache-Info header
+'''
+
+server = Test.MakeOriginServer("server")
+
+request_header = {
+    "headers": "GET /argh HTTP/1.1\r\nHost: doesnotmatter\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+server.addResponse("sessionlog.json", request_header, response_header)
+
+ts = Test.MakeATSProcess("ts")
+
+ts.Disk.records_config.update({
+    'proxy.config.url_remap.remap_required': 0,
+    'proxy.config.diags.debug.enabled': 0,
+    'proxy.config.diags.debug.tags': 'http'
+})
+
+ts.Disk.plugin_config.AddLine('xdebug.so')
+
+ts.Disk.remap_config.AddLine(
+    "map http://one http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+ts.Disk.remap_config.AddLine(
+    "map http://two http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+ts.Disk.remap_config.AddLine(
+    "regex_map http://three[0-9]+ http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+
+tr = Test.AddTestRun()
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.Processes.Default.StartBefore(Test.Processes.server)
+tr.Processes.Default.Command = "cp {}/tcp_client.py {}/tcp_client.py".format(
+    Test.Variables.AtsTestToolsDir, Test.RunDirectory)
+tr.Processes.Default.ReturnCode = 0
+
+
+def sendMsg(msgFile):
+
+    tr = Test.AddTestRun()
+    tr.Processes.Default.Command = (
+        "( python {}/tcp_client.py 127.0.0.1 {} {}/{}.in".format(

Review comment:
       does this need to be python3? (like x_remap.test.py)?




-- 
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.

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



[GitHub] [trafficserver] ywkaras commented on pull request #7784: Adds new X-Cache-Info header to the xdebug plugin

Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #7784:
URL: https://github.com/apache/trafficserver/pull/7784#issuecomment-832717577


   Could you add an Au test or add to x_remap.test.py?


-- 
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.

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