membphis commented on code in PR #12686: URL: https://github.com/apache/apisix/pull/12686#discussion_r2467601253
########## apisix/utils/span.lua: ########## @@ -0,0 +1,97 @@ +-- +-- 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. +-- +local tablepool = require("tablepool") +local util = require("opentelemetry.util") +local span_status = require("opentelemetry.trace.span_status") +local setmetatable = setmetatable +local table = table +local ipairs = ipairs +local pool_name = "opentelemetry_span" + +local _M = {} + + +local mt = { + __index = _M +} + + +function _M.new(name, kind) + local self = tablepool.fetch(pool_name, 0, 8) + self.name = name + self.start_time = util.time_nano() + self.end_time = 0 + self.kind = kind + self.attributes = self.attributes or {} + self.children = self.children or {} Review Comment: the table `attributes` and `children`, they can be reused we can run the flame graph, to confirm if we need this `optimize` ########## apisix/utils/span.lua: ########## @@ -0,0 +1,97 @@ +-- +-- 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. +-- +local tablepool = require("tablepool") Review Comment: current way is fine ########## apisix/utils/span.lua: ########## @@ -0,0 +1,97 @@ +-- +-- 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. +-- +local tablepool = require("tablepool") +local util = require("opentelemetry.util") +local span_status = require("opentelemetry.trace.span_status") +local setmetatable = setmetatable +local table = table +local ipairs = ipairs +local pool_name = "opentelemetry_span" + +local _M = {} + + +local mt = { + __index = _M +} + + +function _M.new(name, kind) + local self = tablepool.fetch(pool_name, 0, 8) + self.name = name + self.start_time = util.time_nano() + self.end_time = 0 + self.kind = kind + self.attributes = self.attributes or {} + self.children = self.children or {} + self.status = nil + return setmetatable(self, mt) +end + + +function _M.append_child(self, span) + table.insert(self.children, span) +end + + +function _M.set_status(self, code, message) + code = span_status.validate(code) + local status = { + code = code, + message = "" + } + if code == span_status.ERROR then + status.message = message + end + + self.status = status +end + + +function _M.set_attributes(self, ...) + for _, attr in ipairs({ ... }) do Review Comment: current way is slow, make a try with new way: ```lua for ... select('#' ) ########## apisix/utils/span.lua: ########## @@ -0,0 +1,97 @@ +-- +-- 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. +-- +local tablepool = require("tablepool") +local util = require("opentelemetry.util") +local span_status = require("opentelemetry.trace.span_status") +local setmetatable = setmetatable +local table = table +local ipairs = ipairs +local pool_name = "opentelemetry_span" + +local _M = {} + + +local mt = { + __index = _M +} + + +function _M.new(name, kind) + local self = tablepool.fetch(pool_name, 0, 8) + self.name = name + self.start_time = util.time_nano() + self.end_time = 0 + self.kind = kind + self.attributes = self.attributes or {} + self.children = self.children or {} + self.status = nil + return setmetatable(self, mt) +end + + +function _M.append_child(self, span) + table.insert(self.children, span) +end + + +function _M.set_status(self, code, message) + code = span_status.validate(code) + local status = { + code = code, + message = "" + } + if code == span_status.ERROR then + status.message = message + end + + self.status = status +end + + +function _M.set_attributes(self, ...) + for _, attr in ipairs({ ... }) do + table.insert(self.attributes, attr) + end +end + + +function _M.finish(self) + self.end_time = util.time_nano() +end + +function _M.release(self) Review Comment: the same as current, the table pool will call `table.clear` auto ```suggestion function _M.release(self) tablepool.release(pool_name, self) end ``` ########## apisix/utils/tracer.lua: ########## @@ -0,0 +1,78 @@ +-- +-- 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. +-- +local ngx = ngx +local stack = require("apisix.utils.stack") +local span = require("apisix.utils.span") +local span_kind = require("opentelemetry.trace.span_kind") +local span_status = require("opentelemetry.trace.span_status") +local table = table +local pairs = pairs + +local _M = { + kind = span_kind, + status = span_status, +} + + +function _M.new_span(name, kind) + local ctx = ngx.ctx + if not ctx._apisix_spans then + ctx._apisix_spans = {} + end + if not ctx._apisix_span_stack then + ctx._apisix_span_stack = stack.new() + end + local sp = span.new(name, kind) + if ctx._apisix_skip_tracing then + return sp + end + if ctx._apisix_span_stack:is_empty() then + table.insert(ctx._apisix_spans, sp) + else + local parent_span = ctx._apisix_span_stack:peek() + parent_span:append_child(sp) + end + ctx._apisix_span_stack:push(sp) + return sp +end + + +function _M.finish_current_span(code, message) + if not ngx.ctx._apisix_span_stack then + return + end + local sp = ngx.ctx._apisix_span_stack:pop() + if code then + sp:set_status(code, message) + end + sp:finish() +end + +function _M.finish_all_spans(code, message) + if not ngx.ctx._apisix_spans then Review Comment: ```lua local apisix_spans = ngx.ctx._apisix_spans if not apisix_spans then return end for _, sp in pairs(apisix_spans) do if code then sp:set_status(code, message) end sp:finish() end ########## apisix/utils/span.lua: ########## @@ -0,0 +1,97 @@ +-- +-- 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. +-- +local tablepool = require("tablepool") +local util = require("opentelemetry.util") +local span_status = require("opentelemetry.trace.span_status") +local setmetatable = setmetatable +local table = table +local ipairs = ipairs +local pool_name = "opentelemetry_span" + +local _M = {} + + +local mt = { + __index = _M +} + + +function _M.new(name, kind) + local self = tablepool.fetch(pool_name, 0, 8) + self.name = name + self.start_time = util.time_nano() + self.end_time = 0 + self.kind = kind + self.attributes = self.attributes or {} + self.children = self.children or {} + self.status = nil + return setmetatable(self, mt) +end + + +function _M.append_child(self, span) + table.insert(self.children, span) +end + + +function _M.set_status(self, code, message) + code = span_status.validate(code) + local status = { Review Comment: ```lua local status = self.status if not status then status = { code = code, message = "" } self.status = status else status.code = code end if code == span_status.ERROR then status.message = message end ########## apisix/utils/stack.lua: ########## @@ -0,0 +1,76 @@ +-- +-- 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. +-- +local _M = {} +local mt = { __index = _M } +local setmetatable = setmetatable + +function _M.new() + local self = { + _data = {}, + _n = 0, + } + return setmetatable(self, mt) +end + + +function _M.push(self, value) + self._n = self._n + 1 + self._data[self._n] = value +end + + +function _M.pop(self) + if self._n == 0 then + return nil + end + + local value = self._data[self._n] + self._data[self._n] = nil + self._n = self._n - 1 + return value +end + + +function _M.peek(self) + if self._n == 0 then + return nil + end + + return self._data[self._n] +end + + +function _M.is_empty(self) + return self._n == 0 +end + + +function _M.size(self) + return self._n +end + + +function _M.clear(self) + for i = 1, self._n do Review Comment: we can call `table.clear(self._data)`, which is much easier -- 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]
