Copilot commented on code in PR #13265:
URL: https://github.com/apache/apisix/pull/13265#discussion_r3114687731
##########
apisix/plugins/opentelemetry.lua:
##########
@@ -478,6 +465,26 @@ function _M.log(conf, api_ctx)
inject_core_spans(ctx, api_ctx, conf)
span:set_attributes(attr.int("http.status_code", status_code),
attr.int("http.response.status_code", status_code))
+
+
+ local attributes = {}
+ if conf.additional_attributes then
+ inject_attributes(attributes, conf.additional_attributes,
api_ctx.var, false)
+ end
+
+ if conf.additional_header_prefix_attributes then
+ inject_attributes(
+ attributes,
+ conf.additional_header_prefix_attributes,
+ core.request.headers(api_ctx),
+ true
+ )
+ end
Review Comment:
This PR also changes when `additional_header_prefix_attributes` are injected
(rewrite -> log), but the test suite here only covers `additional_attributes`
(`consumer_name`). Add a test that asserts a header captured via
`additional_header_prefix_attributes` is present in the exported span, ideally
using a lower-priority plugin that adds/modifies a request header after
opentelemetry runs in rewrite/access, to prevent regressions.
##########
apisix/plugins/opentelemetry.lua:
##########
@@ -478,6 +465,26 @@ function _M.log(conf, api_ctx)
inject_core_spans(ctx, api_ctx, conf)
span:set_attributes(attr.int("http.status_code", status_code),
attr.int("http.response.status_code", status_code))
+
+
+ local attributes = {}
+ if conf.additional_attributes then
+ inject_attributes(attributes, conf.additional_attributes,
api_ctx.var, false)
+ end
+
+ if conf.additional_header_prefix_attributes then
+ inject_attributes(
+ attributes,
+ conf.additional_header_prefix_attributes,
+ core.request.headers(api_ctx),
+ true
+ )
+ end
+
+ if #attributes > 0 then
+ span:set_attributes(unpack(attributes))
Review Comment:
`span:set_attributes(unpack(attributes))` expands the attribute list into
varargs. With wildcard `additional_header_prefix_attributes` this can become a
large number of arguments (all matching headers), which is both slower than
passing a table and can hit Lua/LuaJIT argument/stack limits in extreme cases.
Consider iterating and calling `span:set_attributes` in a loop (or batching) to
avoid `unpack` vararg expansion.
```suggestion
for i = 1, #attributes do
span:set_attributes(attributes[i])
end
```
--
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]
