Copilot commented on code in PR #13633:
URL: https://github.com/apache/apisix/pull/13633#discussion_r3503048720
##########
t/plugin/opentelemetry.t:
##########
@@ -653,37 +653,66 @@ qr/.*x-injected-by-plugin.*test-value.*/
content_by_lua_block {
local t = require("lib.test_admin").test
local plugin = require("apisix.plugin")
- t('/apisix/admin/plugin_metadata/opentelemetry', ngx.HTTP_PUT,
+ local code, body =
t('/apisix/admin/plugin_metadata/opentelemetry', ngx.HTTP_PUT,
[[{"batch_span_processor":{"max_export_batch_size":1,"inactive_timeout":0.5},"collector":{"address":"127.0.0.1:4318"},"resource":{"service.name":"otel-meta-change-first"}}]])
- t('/apisix/admin/routes/1', ngx.HTTP_PUT,
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+ code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT,
[[{"plugins":{"opentelemetry":{"sampler":{"name":"always_on"}}},"upstream":{"nodes":{"127.0.0.1:1980":1},"type":"roundrobin"},"uri":"/opentracing"}]])
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
Review Comment:
Same issue as above: if `t()` returns `nil, err` (request failure), the
numeric comparison will throw and the test will abort without reporting the
root cause. Handle `code == nil` and set a fallback status.
##########
t/plugin/opentelemetry.t:
##########
@@ -653,37 +653,66 @@ qr/.*x-injected-by-plugin.*test-value.*/
content_by_lua_block {
local t = require("lib.test_admin").test
local plugin = require("apisix.plugin")
- t('/apisix/admin/plugin_metadata/opentelemetry', ngx.HTTP_PUT,
+ local code, body =
t('/apisix/admin/plugin_metadata/opentelemetry', ngx.HTTP_PUT,
[[{"batch_span_processor":{"max_export_batch_size":1,"inactive_timeout":0.5},"collector":{"address":"127.0.0.1:4318"},"resource":{"service.name":"otel-meta-change-first"}}]])
- t('/apisix/admin/routes/1', ngx.HTTP_PUT,
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
Review Comment:
`lib.test_admin.test()` returns `nil, err` when the admin request itself
fails (e.g. connection error). In that case `if code >= 300` will raise a Lua
error (`attempt to compare nil with number`), and `ngx.status = code` would
also be invalid. Guard for `code == nil` and fall back to a 500 status so the
test fails cleanly with the underlying error message.
##########
t/plugin/opentelemetry.t:
##########
@@ -653,37 +653,66 @@ qr/.*x-injected-by-plugin.*test-value.*/
content_by_lua_block {
local t = require("lib.test_admin").test
local plugin = require("apisix.plugin")
- t('/apisix/admin/plugin_metadata/opentelemetry', ngx.HTTP_PUT,
+ local code, body =
t('/apisix/admin/plugin_metadata/opentelemetry', ngx.HTTP_PUT,
[[{"batch_span_processor":{"max_export_batch_size":1,"inactive_timeout":0.5},"collector":{"address":"127.0.0.1:4318"},"resource":{"service.name":"otel-meta-change-first"}}]])
- t('/apisix/admin/routes/1', ngx.HTTP_PUT,
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+ code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT,
[[{"plugins":{"opentelemetry":{"sampler":{"name":"always_on"}}},"upstream":{"nodes":{"127.0.0.1:1980":1},"type":"roundrobin"},"uri":"/opentracing"}]])
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
-- wait until this worker sees the metadata so the warm-up span
uses it
+ local seen
for _ = 1, 50 do
local m = plugin.plugin_metadata("opentelemetry")
if m and m.value and m.value.resource
and m.value.resource["service.name"] ==
"otel-meta-change-first" then
+ seen = true
break
end
ngx.sleep(0.1)
end
+ if not seen then
+ ngx.status = 500
+ ngx.say("metadata did not propagate")
+ return
+ end
ngx.say("ok")
}
}
location /setup_second {
content_by_lua_block {
local t = require("lib.test_admin").test
local plugin = require("apisix.plugin")
- t('/apisix/admin/plugin_metadata/opentelemetry', ngx.HTTP_PUT,
+ local code, body =
t('/apisix/admin/plugin_metadata/opentelemetry', ngx.HTTP_PUT,
[[{"batch_span_processor":{"max_export_batch_size":1,"inactive_timeout":0.5},"collector":{"address":"127.0.0.1:4318"},"resource":{"service.name":"otel-meta-change-second"}}]])
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
Review Comment:
`lib.test_admin.test()` can return `nil, err` when the request fails. This
block assumes `code` is always a number, so `code >= 300` can throw and mask
the failure reason. Add a nil guard and default the status to 500.
--
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]
