Copilot commented on code in PR #13232:
URL: https://github.com/apache/apisix/pull/13232#discussion_r3151143517
##########
t/plugin/opentelemetry6.t:
##########
@@ -248,3 +241,56 @@ opentracing
end
}
}
+
+
+
+=== TEST 7: clear file
+--- exec
+echo '' > ci/pod/otelcol-contrib/data-otlp.json
+--- response_body eval
+qr//
+
+
+
+=== TEST 8: trigger two concurrent HTTP/2 requests on the same TLS connection
+--- init_by_lua_block
+ require "resty.core"
+ apisix = require("apisix")
+ core = require("apisix.core")
+ apisix.http_init()
+
+ local utils = require("apisix.core.utils")
+ utils.dns_parse = function (domain)
+ if domain == "test1.com" then
+ return {address = "127.0.0.2"}
+ end
+ error("unknown domain: " .. domain)
+ end
+--- exec
+curl -sk --http2 --parallel --resolve "test.com:1994:127.0.0.1"
https://test.com:1994/opentracing https://test.com:1994/opentracing
Review Comment:
Using `curl --parallel` makes the test nondeterministic: (1) responses
written to stdout can be interleaved/concatenated in ways that vary by curl
version/platform, and (2) curl may open more than one HTTP/2 connection under
parallel scheduling, undermining the goal of 'same TLS connection' validation.
For a stable regression, prefer a single curl invocation without `--parallel`
(still reuses the same HTTP/2 connection sequentially) or write outputs to
separate files (`--output`) and assert their contents independently.
##########
t/node/tracer.t:
##########
@@ -0,0 +1,105 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+log_level('debug');
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->extra_yaml_config) {
+ my $extra_yaml_config = <<_EOC_;
+apisix:
+ tracing: true
+_EOC_
+ $block->set_value("extra_yaml_config", $extra_yaml_config);
+ }
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!defined $block->response_body) {
+ $block->set_value("response_body", "passed\n");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set SSL cert for test.com
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin")
+ local ssl_cert = t.read_file("t/certs/apisix.crt")
+ local ssl_key = t.read_file("t/certs/apisix.key")
+ local core = require("apisix.core")
+ local data = {cert = ssl_cert, key = ssl_key, sni = "test.com"}
+ local code, body = t.test('/apisix/admin/ssls/1',
+ ngx.HTTP_PUT,
+ core.json.encode(data),
+ [[{
+ "value": {
+ "sni": "test.com"
+ },
+ "key": "/apisix/ssls/1"
+ }]]
+ )
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 2: set route
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/opentracing"
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 3: consecutive HTTPS keepalive requests do not crash when tracing is
enabled
+--- exec
+curl -s -k https://test.com:1994/opentracing https://test.com:1994/opentracing
Review Comment:
This test relies on `test.com` resolving in the test environment. Many CI
environments won’t have DNS/hosts entries for `test.com`, making the test
flaky. Use `--resolve \"test.com:1994:127.0.0.1\"` (or the project-standard
mechanism used in other tests) to ensure the request targets the local APISIX
instance deterministically.
```suggestion
curl -s -k --resolve "test.com:1994:127.0.0.1"
https://test.com:1994/opentracing https://test.com:1994/opentracing
```
##########
t/lib/test_otel.lua:
##########
@@ -132,4 +132,50 @@ function _M.verify_tree(filepath, expected_tree)
end
+function _M.verify_isolated_traces(filepath, root_name, count)
+ local spans_by_id, err = parse_spans(filepath)
+ if not spans_by_id then
+ return false, err
+ end
+
+ local traces = {}
+ for _, span in pairs(spans_by_id) do
+ if not traces[span.traceId] then
+ traces[span.traceId] = {}
+ end
+ table.insert(traces[span.traceId], span.name)
+ end
+
+ local matching = {}
+ for trace_id, names in pairs(traces) do
+ for _, name in ipairs(names) do
+ if name == root_name then
+ table.insert(matching, { id = trace_id, names = names })
+ break
+ end
+ end
+ end
+
+ if #matching ~= count then
+ return false, string.format(
+ "expected %d traces with span '%s', got %d",
+ count, root_name, #matching)
+ end
+
+ for _, trace in ipairs(matching) do
+ local seen = {}
+ for _, name in ipairs(trace.names) do
+ if seen[name] then
+ return false, string.format(
+ "trace %s has duplicate span '%s': cross-stream
contamination detected",
+ trace.id, name)
+ end
+ seen[name] = true
+ end
+ end
Review Comment:
This asserts that span names are unique within a trace. That’s not a safe
invariant for tracing systems in general (a single request/trace can
legitimately contain multiple spans with the same name due to retries, repeated
phases, or multiple identical sub-operations). This can create false negatives
unrelated to cross-stream contamination. A more robust check would validate
isolation using trace structure/relationships (e.g., exactly one root span per
trace and all descendants linked to it) and/or assert expected span sets/counts
per trace rather than enforcing global name uniqueness.
--
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]
