subject:"Re\: \[PR\] feat\: add more spans to opentelemetry plugin \[apisix\]"

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-02-07 Thread via GitHub



nic-6443 merged PR #12686:
URL: https://github.com/apache/apisix/pull/12686


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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-02-04 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2767337686


##
docs/zh/latest/plugins/opentelemetry.md:
##
@@ -152,37 +167,152 @@ curl "http://127.0.0.1:9080/anything";
 
 ```text
 2024-02-18T17:14:03.825Z info ResourceSpans #0
-Resource SchemaURL:
-Resource attributes:
--> telemetry.sdk.language: Str(lua)
--> telemetry.sdk.name: Str(opentelemetry-lua)
--> telemetry.sdk.version: Str(0.1.1)
--> hostname: Str(e34673e24631)
--> service.name: Str(APISIX)
 ScopeSpans #0
 ScopeSpans SchemaURL:
 InstrumentationScope opentelemetry-lua
 Span #0
-Trace ID   : fbd0a38d4ea4a128ff1a688197bc58b0
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 5a3835b61110d942
+Name   : http_router_match
+Kind   : Internal
+Start time : 2025-10-24 06:58:04.430430976 + UTC
+End time   : 2025-10-24 06:58:04.431542016 + UTC
+Status code: Unset
+Status message :
+Span #1
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 4ab25e2b92f394e1
+Name   : resolve_dns
+Kind   : Internal
+Start time : 2025-10-24 06:58:04.432521984 + UTC
+End time   : 2025-10-24 06:58:04.44903296 + UTC
+Status code: Unset
+Status message :
+Span #2
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 3620c0f05dd2be4f
+Name   : apisix.phase.header_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960481024 + UTC
+End time   : 2025-10-24 06:58:06.960510976 + UTC
+Status code: Unset
+Status message :
+Span #3
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 4c5f3476f62a7e8a
+ID : a9bfad7bb6986e41
+Name   : apisix.phase.body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960579072 + UTC
+End time   : 2025-10-24 06:58:06.96059008 + UTC
+Status code: Unset
+Status message :
+Span #4
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : b2994675df6baa83
+ID : 26705f9c47584a5b
+Name   : apisix.phase.delayed_body_filter.opentelemetry
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960613888 + UTC
+End time   : 2025-10-24 06:58:06.960687104 + UTC
+Status code: Unset
+Status message :
+Span #5
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 4c5f3476f62a7e8a
+ID : b2994675df6baa83
+Name   : apisix.phase.delayed_body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.96059904 + UTC
+End time   : 2025-10-24 06:58:06.960692992 + UTC
+Status code: Unset
+Status message :
+Span #6
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 4c5f3476f62a7e8a
+Name   : apisix.phase.body_filter
+Kind   : Server
+Start time : 2025-10-24 06:58:06.96056704 + UTC
+End time   : 2025-10-24 06:58:06.960698112 + UTC
+Status code: Unset
+Status message :
+Span #7
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 2024d73d32cbd81b
+ID : 223c64fb691a24e8
+Name   : apisix.phase.body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961624064 + UTC
+End time   : 2025-10-24 06:58:06.961635072 + UTC
+Status code: Unset
+Status message :
+Span #8
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : fd193dd24c618f60
+ID : 8729ad6e0d94a23b
+Name   : apisix.phase.delayed_body_filter.opentelemetry
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961648896 + UTC
+End time   : 1970-01-01 00:00:00 + UTC
+Status code: Unset
+Status message :
+Span #9
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 2024d73d32cbd81b
+ID : fd193dd24c618f60
+Name   : apisix.phase.delayed_body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961641984 + UTC
+End time   : 1970-01-01 00:00:00 + UTC
+Status code: Unset
+Status message :
+Span #10
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 2024d73d32cbd81b
+Name   : apisix.phase.body_filter
+Kind   : Server
+Start time : 2025-10-24 06:58:06.960980992 + UTC
+End time   : 1970-01-01 00:00:00 + UTC
+Status code: Unset
+

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-02-04 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2767246471


##
apisix/secret.lua:
##
@@ -148,16 +150,19 @@ local function fetch_by_uri_secret(secret_uri)
 return nil, "no secret conf, secret_uri: " .. secret_uri
 end
 
+local span = tracer.start(ngx.ctx, "fetch_secret", tracer.kind.client)
 local ok, sm = pcall(require, "apisix.secret." .. opts.manager)
 if not ok then
 return nil, "no secret manager: " .. opts.manager
 end
 
 local value, err = sm.get(conf, opts.key)
 if err then
+tracer.finish(ngx.ctx, span, tracer.status.ERROR, err)
 return nil, err

Review Comment:
   Invalid comments.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-02-04 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2767233967


##
apisix/tracer.lua:
##
@@ -0,0 +1,131 @@
+--
+-- 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 table = require("apisix.core.table")
+local tablepool = require("tablepool")
+local span = require("apisix.utils.span")
+local span_kind = require("opentelemetry.trace.span_kind")
+local span_status = require("opentelemetry.trace.span_status")
+local local_conf = require("apisix.core.config_local").local_conf()
+local ipairs = ipairs
+local ngx = ngx
+
+local enable_tracing = false
+if ngx.config.subsystem == "http" and type(local_conf.apisix.tracing) == 
"boolean" then
+enable_tracing = local_conf.apisix.tracing
+end
+
+local _M = {
+kind = span_kind,
+status = span_status,
+span_state = {},
+}
+
+function _M.start(ctx, name, kind)
+if not enable_tracing then
+return
+end
+
+local tracing = ctx and ctx.tracing
+if not tracing then
+local root_span = span.new()
+tracing = tablepool.fetch("tracing", 0, 8)
+tracing.spans = tablepool.fetch("tracing_spans", 20, 0)
+tracing.root_span = root_span
+tracing.current_span = root_span
+table.insert(tracing.spans, root_span)
+root_span.id = 1
+ctx.tracing = tracing
+end
+if tracing.skip then
+return
+end
+
+local spans = tracing.spans
+local sp = span.new(name, kind)
+
+table.insert(spans, sp)
+local id = #spans
+sp.id = id
+local parent = tracing.current_span
+if parent then
+sp:set_parent(parent.id)
+parent:append_child(id)
+end
+tracing.current_span = sp
+return sp
+end
+
+
+local function finish_span(spans, sp, code, message)
+if not sp or sp.end_time then
+return
+end
+for _, id in ipairs(sp.child_ids or {}) do
+finish_span(spans, spans[id])
+end
+if code then
+sp:set_status(code, message)
+end
+sp:finish()
+end
+
+
+function _M.finish(ctx, sp, code, message)
+local tracing = ctx and ctx.tracing
+if not tracing then
+return
+end
+
+sp = sp or tracing.current_span
+if not sp then
+return
+end
+
+finish_span(tracing.spans, sp, code, message)
+tracing.current_span = tracing.spans[sp.parent_id]
+end

Review Comment:
   fixed.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-02-04 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2762839350


##
apisix/utils/span.lua:
##
@@ -0,0 +1,101 @@
+--
+-- 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 select = select
+local pool_name = "opentelemetry_span"
+local update_time = ngx.update_time
+
+local _M = {}
+
+
+local mt = {
+__index = _M
+}
+
+local function get_time()
+update_time()
+return util.time_nano()
+end
+
+
+function _M.new(name, kind)
+local self = tablepool.fetch(pool_name, 0, 16)
+self.start_time = get_time()
+self.name = name
+self.kind = kind
+self.status = nil
+self.dead = false
+return setmetatable(self, mt)
+end
+
+
+function _M.append_child(self, child_id)
+if not self.child_ids then
+self.child_ids = table.new(10, 0)
+end
+table.insert(self.child_ids, child_id)
+end
+
+
+function _M.set_parent(self, parent_id)
+self.parent_id = parent_id
+end
+
+
+function _M.release(self)
+tablepool.release(pool_name, self)
+end
+
+
+function _M.set_status(self, code, message)
+code = span_status.validate(code)
+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
+end
+
+
+function _M.set_attributes(self, ...)
+local count = select('#', ...)
+for i = 1, count do
+local attr = select(i, ...)
+table.insert(self.attributes, attr)
+end
+end

Review Comment:
   fixed.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-02-04 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2762763708


##
apisix/plugins/opentelemetry.lua:
##
@@ -376,48 +384,96 @@ function _M.rewrite(conf, api_ctx)
   ngx_var.opentelemetry_span_id = span_context.span_id
 end
 
+if not ctx:span():is_recording() and ngx.ctx.tracing then
+ngx.ctx.tracing.skip = true
+end
+
 api_ctx.otel_context_token = ctx:attach()
 
 -- inject trace context into the headers of upstream HTTP request
 trace_context_propagator:inject(ctx, ngx.req)
 end
 
 
-function _M.delayed_body_filter(conf, api_ctx)
-if api_ctx.otel_context_token and ngx.arg[2] then
-local ctx = context:current()
-ctx:detach(api_ctx.otel_context_token)
-api_ctx.otel_context_token = nil
+local function create_child_span(tracer, parent_span_ctx, spans, span)
+if not span or span.finished then
+return
+end
+span.finished = true
+local new_span_ctx, new_span = tracer:start(parent_span_ctx, span.name,
+{
+kind = span.kind,
+attributes = span.attributes,
+})
+new_span.start_time = span.start_time
+
+for _, idx in ipairs(span.child_ids or {}) do
+create_child_span(tracer, new_span_ctx, spans, spans[idx])
+end
+if span.status then
+new_span:set_status(span.status.code, span.status.message)
+end
+new_span:finish(span.end_time)
+end

Review Comment:
   When end_time is nil, it will automatically call finish to update the time.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-02-04 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2762763111


##
apisix/plugins/opentelemetry.lua:
##
@@ -376,48 +384,96 @@ function _M.rewrite(conf, api_ctx)
   ngx_var.opentelemetry_span_id = span_context.span_id
 end
 
+if not ctx:span():is_recording() and ngx.ctx.tracing then
+ngx.ctx.tracing.skip = true
+end
+
 api_ctx.otel_context_token = ctx:attach()
 
 -- inject trace context into the headers of upstream HTTP request
 trace_context_propagator:inject(ctx, ngx.req)
 end
 
 
-function _M.delayed_body_filter(conf, api_ctx)
-if api_ctx.otel_context_token and ngx.arg[2] then
-local ctx = context:current()
-ctx:detach(api_ctx.otel_context_token)
-api_ctx.otel_context_token = nil
+local function create_child_span(tracer, parent_span_ctx, spans, span)
+if not span or span.finished then
+return
+end
+span.finished = true
+local new_span_ctx, new_span = tracer:start(parent_span_ctx, span.name,
+{
+kind = span.kind,
+attributes = span.attributes,
+})
+new_span.start_time = span.start_time
+
+for _, idx in ipairs(span.child_ids or {}) do
+create_child_span(tracer, new_span_ctx, spans, spans[idx])
+end
+if span.status then
+new_span:set_status(span.status.code, span.status.message)
+end
+new_span:finish(span.end_time)
+end
 
--- get span from current context
-local span = ctx:span()
-local upstream_status = core.response.get_upstream_status(api_ctx)
-if upstream_status and upstream_status >= 500 then
-span:set_status(span_status.ERROR,
-"upstream response status: " .. upstream_status)
-end
 
-span:set_attributes(attr.int("http.status_code", upstream_status))
+local function inject_core_spans(root_span_ctx, api_ctx, conf)
+local tracing = api_ctx.ngx_ctx.tracing
+if not tracing then
+return
+end
 
-span:finish()
+local span = root_span_ctx:span()
+
+local metadata = plugin.plugin_metadata(plugin_name)
+local plugin_info = metadata.value
+if span and not span:is_recording() then
+return
+end
+local inject_conf = {
+sampler = {
+name = "always_on",
+options = conf.sampler.options
+},
+additional_attributes = conf.additional_attributes,
+additional_header_prefix_attributes = 
conf.additional_header_prefix_attributes
+}
+local tracer, err = core.lrucache.plugin_ctx(lrucache, api_ctx, nil,
+create_tracer_obj, 
inject_conf, plugin_info)
+if not tracer then
+core.log.error("failed to fetch tracer object: ", err)
+return
+end
+
+if #tracing.spans == 0 then
+return
+end
+span.start_time = tracing.spans[1].start_time
+local root_span = tracing.root_span
+local spans = tracing.spans
+for _, idx in ipairs(root_span.child_ids or {}) do
+create_child_span(tracer, root_span_ctx, spans, spans[idx])
 end
 end
 
 
--- body_filter maybe not called because of empty http body response
--- so we need to check if the span has finished in log phase
 function _M.log(conf, api_ctx)
 if api_ctx.otel_context_token then
 -- ctx:detach() is not necessary, because of ctx is stored in ngx.ctx
 local upstream_status = core.response.get_upstream_status(api_ctx)
 
 -- get span from current context
-local span = context:current():span()
+local ctx = context:current()
+local span = ctx:span()
 if upstream_status and upstream_status >= 500 then
 span:set_status(span_status.ERROR,
 "upstream response status: " .. upstream_status)
 end
 
+inject_core_spans(ctx, api_ctx, conf)
+span:set_attributes(attr.int("http.status_code", upstream_status),
+attr.int("http.response.status_code", 
upstream_status))
+update_time()

Review Comment:
   When end_time is nil, it will automatically call finish to update the time.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-02-03 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2762656120


##
apisix/init.lua:
##
@@ -901,6 +916,9 @@ function _M.http_header_filter_phase()
 end
 core.response.set_header("Apisix-Plugins", 
core.table.concat(deduplicate, ", "))
 end
+tracer.finish(ngx_ctx, span)
+
+tracer.start(ngx_ctx, "apisix.phase.body_filter", tracer.kind.server)
 end

Review Comment:
   In the log phase, all spans will be finished.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-02-03 Thread via GitHub



Copilot commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2762593711


##
apisix/utils/span.lua:
##
@@ -0,0 +1,101 @@
+--
+-- 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 select = select
+local pool_name = "opentelemetry_span"
+local update_time = ngx.update_time
+
+local _M = {}
+
+
+local mt = {
+__index = _M
+}
+
+local function get_time()
+update_time()
+return util.time_nano()
+end
+
+
+function _M.new(name, kind)
+local self = tablepool.fetch(pool_name, 0, 16)
+self.start_time = get_time()
+self.name = name
+self.kind = kind
+self.status = nil
+self.dead = false
+return setmetatable(self, mt)
+end
+
+
+function _M.append_child(self, child_id)
+if not self.child_ids then
+self.child_ids = table.new(10, 0)
+end
+table.insert(self.child_ids, child_id)
+end
+
+
+function _M.set_parent(self, parent_id)
+self.parent_id = parent_id
+end
+
+
+function _M.release(self)
+tablepool.release(pool_name, self)
+end
+
+
+function _M.set_status(self, code, message)
+code = span_status.validate(code)
+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
+end
+
+
+function _M.set_attributes(self, ...)
+local count = select('#', ...)
+for i = 1, count do
+local attr = select(i, ...)
+table.insert(self.attributes, attr)
+end
+end

Review Comment:
   `set_attributes` appends into `self.attributes`, but `attributes` is never 
initialized in `new()`. This will throw on first use. Initialize 
`self.attributes` (and clear it on reuse) before inserting.



##
apisix/plugins/opentelemetry.lua:
##
@@ -376,48 +384,96 @@ function _M.rewrite(conf, api_ctx)
   ngx_var.opentelemetry_span_id = span_context.span_id
 end
 
+if not ctx:span():is_recording() and ngx.ctx.tracing then
+ngx.ctx.tracing.skip = true
+end
+
 api_ctx.otel_context_token = ctx:attach()
 
 -- inject trace context into the headers of upstream HTTP request
 trace_context_propagator:inject(ctx, ngx.req)
 end
 
 
-function _M.delayed_body_filter(conf, api_ctx)
-if api_ctx.otel_context_token and ngx.arg[2] then
-local ctx = context:current()
-ctx:detach(api_ctx.otel_context_token)
-api_ctx.otel_context_token = nil
+local function create_child_span(tracer, parent_span_ctx, spans, span)
+if not span or span.finished then
+return
+end
+span.finished = true
+local new_span_ctx, new_span = tracer:start(parent_span_ctx, span.name,
+{
+kind = span.kind,
+attributes = span.attributes,
+})
+new_span.start_time = span.start_time
+
+for _, idx in ipairs(span.child_ids or {}) do
+create_child_span(tracer, new_span_ctx, spans, spans[idx])
+end
+if span.status then
+new_span:set_status(span.status.code, span.status.message)
+end
+new_span:finish(span.end_time)
+end
 
--- get span from current context
-local span = ctx:span()
-local upstream_status = core.response.get_upstream_status(api_ctx)
-if upstream_status and upstream_status >= 500 then
-span:set_status(span_status.ERROR,
-"upstream response status: " .. upstream_status)
-end
 
-span:set_attributes(attr.int("http.status_code", upstream_status))
+local function inject_core_spans(root_span_ctx, api_ctx, conf)
+local tracing = api_ctx.ngx_ctx.tracing
+if not tracing then
+return
+end
 
-span:finish()
+local span = root_span_ctx:span()
+
+local metadata = plugin.pl

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-02-03 Thread via GitHub



Copilot commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2762500654


##
apisix/secret.lua:
##
@@ -148,16 +150,19 @@ local function fetch_by_uri_secret(secret_uri)
 return nil, "no secret conf, secret_uri: " .. secret_uri
 end
 
+local span = tracer.start(ngx.ctx, "fetch_secret", tracer.kind.client)
 local ok, sm = pcall(require, "apisix.secret." .. opts.manager)
 if not ok then
 return nil, "no secret manager: " .. opts.manager
 end
 
 local value, err = sm.get(conf, opts.key)
 if err then
+tracer.finish(ngx.ctx, span, tracer.status.ERROR, err)
 return nil, err

Review Comment:
   `tracer.finish` is called with the wrong argument order on the error path. 
It should be `tracer.finish(ngx.ctx, span, tracer.status.ERROR, err)` (or 
equivalent) so it doesn't attempt to treat the status enum as a span object.



##
apisix/core/response.lua:
##
@@ -86,6 +87,9 @@ function resp_exit(code, ...)
 end
 
 if code then
+if code >= 400 then
+tracer.finish_all(ngx.ctx, tracer.status.ERROR, "response code " 
.. code)

Review Comment:
   This call uses `tracer.finish(ngx.ctx, tracer.status.ERROR, ...)` but 
`finish` expects `(ctx, sp, code, message)`. As written it will crash for any 
`code >= 400` response when tracing is enabled. If the intent is “finish 
current span with error”, pass an explicit `nil` span (`finish(ctx, nil, code, 
msg)`) or introduce a dedicated `finish_current(ctx, code, msg)` helper.
   ```suggestion
   tracer.finish_all(ngx.ctx, nil, code, "response code " .. code)
   ```



##
apisix/tracer.lua:
##
@@ -0,0 +1,131 @@
+--
+-- 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 table = require("apisix.core.table")
+local tablepool = require("tablepool")
+local span = require("apisix.utils.span")
+local span_kind = require("opentelemetry.trace.span_kind")
+local span_status = require("opentelemetry.trace.span_status")
+local local_conf = require("apisix.core.config_local").local_conf()
+local ipairs = ipairs
+local ngx = ngx
+
+local enable_tracing = false
+if ngx.config.subsystem == "http" and type(local_conf.apisix.tracing) == 
"boolean" then
+enable_tracing = local_conf.apisix.tracing
+end
+
+local _M = {
+kind = span_kind,
+status = span_status,
+span_state = {},
+}
+
+function _M.start(ctx, name, kind)
+if not enable_tracing then
+return
+end
+
+local tracing = ctx and ctx.tracing
+if not tracing then
+local root_span = span.new()
+tracing = tablepool.fetch("tracing", 0, 8)
+tracing.spans = tablepool.fetch("tracing_spans", 20, 0)
+tracing.root_span = root_span
+tracing.current_span = root_span
+table.insert(tracing.spans, root_span)
+root_span.id = 1
+ctx.tracing = tracing
+end
+if tracing.skip then
+return
+end
+
+local spans = tracing.spans
+local sp = span.new(name, kind)
+
+table.insert(spans, sp)
+local id = #spans
+sp.id = id
+local parent = tracing.current_span
+if parent then
+sp:set_parent(parent.id)
+parent:append_child(id)
+end
+tracing.current_span = sp
+return sp
+end
+
+
+local function finish_span(spans, sp, code, message)
+if not sp or sp.end_time then
+return
+end
+for _, id in ipairs(sp.child_ids or {}) do
+finish_span(spans, spans[id])
+end
+if code then
+sp:set_status(code, message)
+end
+sp:finish()
+end
+
+
+function _M.finish(ctx, sp, code, message)
+local tracing = ctx and ctx.tracing
+if not tracing then
+return
+end
+
+sp = sp or tracing.current_span
+if not sp then
+return
+end
+
+finish_span(tracing.spans, sp, code, message)
+tracing.current_span = tracing.spans[sp.parent_id]
+end

Review Comment:
   `finish()` defaults to `tracing.current_span` when `sp` is nil. Combined 
with `start()` returning nil when `tracing.skip` is true, common call sites 
(`local sp = start(...); finish(ctx, sp)`) will incorrectly finish the 
*current* span even though no new s

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-02-03 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2762444852


##
apisix/ssl/router/radixtree_sni.lua:
##
@@ -177,9 +179,10 @@ function _M.match_and_set(api_ctx, match_only, alt_sni)
 -- with it sometimes
 core.log.error("failed to find any SSL certificate by SNI: ", sni)
 end
+tracer.finish(api_ctx.ngx_ctx, tracer.status.ERROR, "failed match SNI")
 return false
 end
-
+tracer.finish(api_ctx.ngx_ctx)

Review Comment:
   
   In APISIX, explicitly managing span hierarchies is not practical, especially 
across multiple modules. The management cost is high and error-prone: if a 
child span misses a `finish` call, all subsequent spans may be attached to the 
wrong parent.
   
   To address this, the model is adjusted to be centered around `current_span`.
   `tracer.start` only updates the current execution point (`current_span`), 
while `tracer.finish(span)` recursively finishes all child spans of the given 
span. This avoids hierarchy corruption caused by missing `finish` calls on 
child spans.
   
   Example behavior:
   
   ```lua
   -- current_span --> parent
   local parent = tracer.start("parent")
   -- current_span --> child
   local child = tracer.start("child")
   
   -- child missing finish
   
   -- finishing parent will automatically finish child
   tracer.finish(parent)
   ```
   
   This approach keeps the span hierarchy consistent without increasing the 
complexity for callers.
   
   And it is pluggable, so when detailed tracing is not enabled, there is no 
need to make additional redundant judgments when calling start/minimize.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-29 Thread via GitHub



bzp2010 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2742044413


##
apisix/ssl/router/radixtree_sni.lua:
##
@@ -177,9 +179,10 @@ function _M.match_and_set(api_ctx, match_only, alt_sni)
 -- with it sometimes
 core.log.error("failed to find any SSL certificate by SNI: ", sni)
 end
+tracer.finish(api_ctx.ngx_ctx, tracer.status.ERROR, "failed match SNI")
 return false
 end
-
+tracer.finish(api_ctx.ngx_ctx)

Review Comment:
   Strictly, this API design is still strange, with many internal details 
hidden within the stack data structure. For developers, it presents an opaque 
data structure.
   If any span of code begins with `tracer.start` but fails to conclude with 
`tracer.finish`, the entire tracing process will encounter an error. This 
unclosed span will encompass all subsequent spans, which is not the intended 
behavior.
   
   If we examine OTEL implementations in other major programming languages, we 
find that many of them follow the following pattern:
   
   Use the tracer's method to create a span, then obtain an instance of that 
span. After internal operations complete, use the span instance's methods to 
actively terminate the specified span. The ctx will be used to track the 
current span, ensuring that parent-child relationships are correctly maintained 
when creating child spans.
   
   ```lua
   -- context-based
   local parent_span = tracer.start(ctx, "parent")
   
   local child_span = tracer.start(ctx, "child")
   
   child_span:end()
   
   parent_span:end()
   ```
   
   ref: 
https://opentelemetry.io/docs/languages/go/instrumentation/#create-nested-spans
   ref: 
https://opentelemetry.io/docs/languages/php/instrumentation/#create-nested-spans
   ref: 
https://opentelemetry.io/docs/languages/js/instrumentation/#create-nested-spans
   ref: https://opentelemetry.io/docs/languages/java/api/#span
   ref: 
https://opentelemetry.io/docs/languages/cpp/instrumentation/#create-nested-spans
   
   Alternatively, explicitly pass the parent span instance to the child span to 
achieve precise hierarchical control. 
   
   ```lua
   -- pass span directly
   local parent_span = tracer.start("parent")
   
   local child_span = tracer.start("child", parent_span)
   
   child_span:end()
   
   parent_span:end()
   ```
   
   Note that the following Rust code typically terminates spans automatically 
via drop calls when the span instance's variable scopes end.
   
   ref: 
https://docs.rs/tracing/latest/tracing/span/index.html#span-relationships
   
   None of the OTEL libraries for these languages organize and manage the 
entire span stack using only global state. Each one terminates spans by 
returning span instances and requiring developers to explicitly call 
`span.end()` when operations conclude.
   
   Our current approach requires any developer writing trace code to be 
familiar with the stack mechanism we've created and to have complete clarity 
about where their spans appear in the stack, whether parent spans exist, 
whether they've closed as expected, or any other state. Otherwise, they cannot 
write correct code. Therefore, I disagree with the current API design.
   
   It's all too easy to make mistakes like the one below.
   
   ```lua
   tracer.start("parent")
   
   tracer.start("child")
   
   tracer.finish()
   
   -- missing last tracer.finish
   
   ```
   
   The result is an output issue, because regardless of what you do, calling 
`finish_all` will terminate the span. However, the parent-child relationships 
and timestamps for all subsequent spans will be incorrect. This won't trigger 
any obvious error messages, making it extremely difficult for users to debug.
   
   ---
   
   I have already pointed this out in 
https://github.com/apache/apisix/pull/12686#discussion_r2471659658, and 
@membphis has clearly reiterated this point through images.
   
   
![](https://private-user-images.githubusercontent.com/6814606/507373141-8a75f5fe-e0a4-4375-8e97-84c80445be5a.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Njk2OTY1NDIsIm5iZiI6MTc2OTY5NjI0MiwicGF0aCI6Ii82ODE0NjA2LzUwNzM3MzE0MS04YTc1ZjVmZS1lMGE0LTQzNzUtOGU5Ny04NGM4MDQ0NWJlNWEucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI2MDEyOSUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNjAxMjlUMTQxNzIyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MGQ2MmNhM2RkZmUzMTkxZDJmMjVmYWU3OWE1ZDFiYWI2OTRjZjFiMzA5YTE5Y2Y4ZmExMmFjOTZkOGQzZmVjMSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.YV-yZUolOcov273ML-vqQfSEmcNLL74SOB_7Cl5rDCw)



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-29 Thread via GitHub



bzp2010 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2741867364


##
docs/en/latest/plugins/opentelemetry.md:
##
@@ -153,37 +168,152 @@ In OpenTelemetry collector's log, you should see 
information similar to the foll
 
 ```text
 2024-02-18T17:14:03.825Z info ResourceSpans #0
-Resource SchemaURL:
-Resource attributes:
- -> telemetry.sdk.language: Str(lua)
- -> telemetry.sdk.name: Str(opentelemetry-lua)
- -> telemetry.sdk.version: Str(0.1.1)
- -> hostname: Str(e34673e24631)
- -> service.name: Str(APISIX)
 ScopeSpans #0
 ScopeSpans SchemaURL:
 InstrumentationScope opentelemetry-lua
 Span #0
-Trace ID   : fbd0a38d4ea4a128ff1a688197bc58b0
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 5a3835b61110d942
+Name   : http_router_match
+Kind   : Internal
+Start time : 2025-10-24 06:58:04.430430976 + UTC
+End time   : 2025-10-24 06:58:04.431542016 + UTC
+Status code: Unset
+Status message :
+Span #1
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 4ab25e2b92f394e1
+Name   : resolve_dns
+Kind   : Internal
+Start time : 2025-10-24 06:58:04.432521984 + UTC
+End time   : 2025-10-24 06:58:04.44903296 + UTC
+Status code: Unset
+Status message :
+Span #2
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 3620c0f05dd2be4f
+Name   : apisix.phase.header_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960481024 + UTC
+End time   : 2025-10-24 06:58:06.960510976 + UTC
+Status code: Unset
+Status message :
+Span #3
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 4c5f3476f62a7e8a
+ID : a9bfad7bb6986e41
+Name   : apisix.phase.body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960579072 + UTC
+End time   : 2025-10-24 06:58:06.96059008 + UTC
+Status code: Unset
+Status message :
+Span #4
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : b2994675df6baa83
+ID : 26705f9c47584a5b
+Name   : apisix.phase.delayed_body_filter.opentelemetry
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960613888 + UTC
+End time   : 2025-10-24 06:58:06.960687104 + UTC
+Status code: Unset
+Status message :
+Span #5
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 4c5f3476f62a7e8a
+ID : b2994675df6baa83
+Name   : apisix.phase.delayed_body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.96059904 + UTC
+End time   : 2025-10-24 06:58:06.960692992 + UTC
+Status code: Unset
+Status message :
+Span #6
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 4c5f3476f62a7e8a
+Name   : apisix.phase.body_filter
+Kind   : Server
+Start time : 2025-10-24 06:58:06.96056704 + UTC
+End time   : 2025-10-24 06:58:06.960698112 + UTC
+Status code: Unset
+Status message :
+Span #7
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 2024d73d32cbd81b
+ID : 223c64fb691a24e8
+Name   : apisix.phase.body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961624064 + UTC
+End time   : 2025-10-24 06:58:06.961635072 + UTC
+Status code: Unset
+Status message :
+Span #8
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : fd193dd24c618f60
+ID : 8729ad6e0d94a23b
+Name   : apisix.phase.delayed_body_filter.opentelemetry
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961648896 + UTC
+End time   : 1970-01-01 00:00:00 + UTC
+Status code: Unset
+Status message :
+Span #9
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 2024d73d32cbd81b
+ID : fd193dd24c618f60
+Name   : apisix.phase.delayed_body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961641984 + UTC
+End time   : 1970-01-01 00:00:00 + UTC
+Status code: Unset
+Status message :
+Span #10
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 2024d73d32cbd81b
+Name   : apisix.phase.body_filter
+Kind   : Server
+Start time : 2025-10-24 06:58:06.960980992 + UTC
+End time   : 1970-01-01 0

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-29 Thread via GitHub



bzp2010 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2742044413


##
apisix/ssl/router/radixtree_sni.lua:
##
@@ -177,9 +179,10 @@ function _M.match_and_set(api_ctx, match_only, alt_sni)
 -- with it sometimes
 core.log.error("failed to find any SSL certificate by SNI: ", sni)
 end
+tracer.finish(api_ctx.ngx_ctx, tracer.status.ERROR, "failed match SNI")
 return false
 end
-
+tracer.finish(api_ctx.ngx_ctx)

Review Comment:
   Strictly, this API design is still strange, with many internal details 
hidden within the stack data structure. For developers, it presents an opaque 
data structure.
   If any span of code begins with `tracer.start` but fails to conclude with 
`tracer.finish`, the entire tracing process will encounter an error. This 
unclosed span will encompass all subsequent spans, which is not the intended 
behavior.
   
   If we examine OTEL implementations in other major programming languages, we 
find that many of them follow the following pattern:
   
   Use the tracer's method to create a span, then obtain an instance of that 
span. After internal operations complete, use the span instance's methods to 
actively terminate the specified span. The ctx will be used to track the 
current span, ensuring that parent-child relationships are correctly maintained 
when creating child spans.
   
   ```lua
   -- context-based
   local parent_span = tracer.start(ctx, "parent")
   
   local child_span = tracer.start(ctx, "child")
   
   child_span:end()
   
   parent_span:end()
   ```
   
   ref: 
https://opentelemetry.io/docs/languages/go/instrumentation/#create-nested-spans
   ref: 
https://opentelemetry.io/docs/languages/php/instrumentation/#create-nested-spans
   ref: 
https://opentelemetry.io/docs/languages/js/instrumentation/#create-nested-spans
   ref: https://opentelemetry.io/docs/languages/java/api/#span
   ref: 
https://opentelemetry.io/docs/languages/cpp/instrumentation/#create-nested-spans
   
   Alternatively, explicitly pass the parent span instance to the child span to 
achieve precise hierarchical control. 
   
   ```lua
   -- pass span directly
   local parent_span = tracer.start("parent")
   
   local child_span = tracer.start("child", parent_span)
   
   child_span:end()
   
   parent_span:end()
   ```
   
   Note that the following Rust code typically terminates spans automatically 
via drop calls when the span instance's variable scopes end.
   
   ref: 
https://docs.rs/tracing/latest/tracing/span/index.html#span-relationships
   
   None of the OTEL libraries for these languages organize and manage the 
entire span stack using only global state. Each one terminates spans by 
returning span instances and requiring developers to explicitly call 
`span.end()` when operations conclude.
   
   Our current approach requires any developer writing trace code to be 
familiar with the stack mechanism we've created and to have complete clarity 
about where their spans appear in the stack, whether parent spans exist, 
whether they've closed as expected, or any other state. Otherwise, they cannot 
write correct code. Therefore, I disagree with the current API design.
   
   I have already pointed this out in 
https://github.com/apache/apisix/pull/12686#discussion_r2471659658, and 
@membphis has clearly reiterated this point through images.
   
   
![](https://private-user-images.githubusercontent.com/6814606/507373141-8a75f5fe-e0a4-4375-8e97-84c80445be5a.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Njk2OTY1NDIsIm5iZiI6MTc2OTY5NjI0MiwicGF0aCI6Ii82ODE0NjA2LzUwNzM3MzE0MS04YTc1ZjVmZS1lMGE0LTQzNzUtOGU5Ny04NGM4MDQ0NWJlNWEucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI2MDEyOSUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNjAxMjlUMTQxNzIyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MGQ2MmNhM2RkZmUzMTkxZDJmMjVmYWU3OWE1ZDFiYWI2OTRjZjFiMzA5YTE5Y2Y4ZmExMmFjOTZkOGQzZmVjMSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.YV-yZUolOcov273ML-vqQfSEmcNLL74SOB_7Cl5rDCw)



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-29 Thread via GitHub



bzp2010 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2741840072


##
docs/zh/latest/plugins/opentelemetry.md:
##
@@ -152,37 +167,152 @@ curl "http://127.0.0.1:9080/anything";
 
 ```text
 2024-02-18T17:14:03.825Z info ResourceSpans #0
-Resource SchemaURL:
-Resource attributes:
--> telemetry.sdk.language: Str(lua)
--> telemetry.sdk.name: Str(opentelemetry-lua)
--> telemetry.sdk.version: Str(0.1.1)
--> hostname: Str(e34673e24631)
--> service.name: Str(APISIX)
 ScopeSpans #0
 ScopeSpans SchemaURL:
 InstrumentationScope opentelemetry-lua
 Span #0
-Trace ID   : fbd0a38d4ea4a128ff1a688197bc58b0
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 5a3835b61110d942
+Name   : http_router_match
+Kind   : Internal
+Start time : 2025-10-24 06:58:04.430430976 + UTC
+End time   : 2025-10-24 06:58:04.431542016 + UTC
+Status code: Unset
+Status message :
+Span #1
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 4ab25e2b92f394e1
+Name   : resolve_dns
+Kind   : Internal
+Start time : 2025-10-24 06:58:04.432521984 + UTC
+End time   : 2025-10-24 06:58:04.44903296 + UTC
+Status code: Unset
+Status message :
+Span #2
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 3620c0f05dd2be4f
+Name   : apisix.phase.header_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960481024 + UTC
+End time   : 2025-10-24 06:58:06.960510976 + UTC
+Status code: Unset
+Status message :
+Span #3
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 4c5f3476f62a7e8a
+ID : a9bfad7bb6986e41
+Name   : apisix.phase.body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960579072 + UTC
+End time   : 2025-10-24 06:58:06.96059008 + UTC
+Status code: Unset
+Status message :
+Span #4
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : b2994675df6baa83
+ID : 26705f9c47584a5b
+Name   : apisix.phase.delayed_body_filter.opentelemetry
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960613888 + UTC
+End time   : 2025-10-24 06:58:06.960687104 + UTC
+Status code: Unset
+Status message :
+Span #5
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 4c5f3476f62a7e8a
+ID : b2994675df6baa83
+Name   : apisix.phase.delayed_body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.96059904 + UTC
+End time   : 2025-10-24 06:58:06.960692992 + UTC
+Status code: Unset
+Status message :
+Span #6
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 4c5f3476f62a7e8a
+Name   : apisix.phase.body_filter
+Kind   : Server
+Start time : 2025-10-24 06:58:06.96056704 + UTC
+End time   : 2025-10-24 06:58:06.960698112 + UTC
+Status code: Unset
+Status message :
+Span #7
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 2024d73d32cbd81b
+ID : 223c64fb691a24e8
+Name   : apisix.phase.body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961624064 + UTC
+End time   : 2025-10-24 06:58:06.961635072 + UTC
+Status code: Unset
+Status message :
+Span #8
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : fd193dd24c618f60
+ID : 8729ad6e0d94a23b
+Name   : apisix.phase.delayed_body_filter.opentelemetry
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961648896 + UTC
+End time   : 1970-01-01 00:00:00 + UTC
+Status code: Unset
+Status message :
+Span #9
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 2024d73d32cbd81b
+ID : fd193dd24c618f60
+Name   : apisix.phase.delayed_body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961641984 + UTC
+End time   : 1970-01-01 00:00:00 + UTC
+Status code: Unset
+Status message :
+Span #10
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 2024d73d32cbd81b
+Name   : apisix.phase.body_filter
+Kind   : Server
+Start time : 2025-10-24 06:58:06.960980992 + UTC
+End time   : 1970-01-01 00:00:00 + UTC
+Status code: Unset
+

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-29 Thread via GitHub



bzp2010 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2741788327


##
apisix/plugins/opentelemetry.lua:
##
@@ -376,49 +377,101 @@ function _M.rewrite(conf, api_ctx)
   ngx_var.opentelemetry_span_id = span_context.span_id
 end
 
+if not ctx:span():is_recording() and ngx.ctx.tracing then
+ngx.ctx.tracing.skip = true
+end
+
 api_ctx.otel_context_token = ctx:attach()
 
 -- inject trace context into the headers of upstream HTTP request
 trace_context_propagator:inject(ctx, ngx.req)
 end
 
 
-function _M.delayed_body_filter(conf, api_ctx)
-if api_ctx.otel_context_token and ngx.arg[2] then
-local ctx = context:current()
-ctx:detach(api_ctx.otel_context_token)
-api_ctx.otel_context_token = nil
+local function create_child_span(tracer, parent_span_ctx, spans, span_idx)
+local span = spans[span_idx]
+if not span or span.finished then
+return
+end
+span.finished = true
+local new_span_ctx, new_span = tracer:start(parent_span_ctx, span.name,
+{
+kind = span.kind,
+attributes = span.attributes,
+})
+new_span.start_time = span.start_time
+
+for _, idx in ipairs(span.child_ids or {}) do
+create_child_span(tracer, new_span_ctx, spans, idx)
+end
+if span.status then
+new_span:set_status(span.status.code, span.status.message)
+end
+new_span:finish(span.end_time)
+end
 
--- get span from current context
-local span = ctx:span()
-local upstream_status = core.response.get_upstream_status(api_ctx)
-if upstream_status and upstream_status >= 500 then
-span:set_status(span_status.ERROR,
-"upstream response status: " .. upstream_status)
-end
 
-span:set_attributes(attr.int("http.status_code", upstream_status))
+local function inject_core_spans(root_span_ctx, api_ctx, conf)
+local tracing = api_ctx.ngx_ctx.tracing
+if not tracing then
+return
+end
 
-span:finish()
+local span = root_span_ctx:span()
+
+local metadata = plugin.plugin_metadata(plugin_name)
+local plugin_info = metadata.value
+if span and not span:is_recording() then
+return
+end
+local inject_conf = {
+sampler = {
+name = "always_on",
+options = conf.sampler.options
+},
+additional_attributes = conf.additional_attributes,
+additional_header_prefix_attributes = 
conf.additional_header_prefix_attributes
+}
+local tracer, err = core.lrucache.plugin_ctx(lrucache, api_ctx, nil,
+create_tracer_obj, 
inject_conf, plugin_info)
+if not tracer then
+core.log.error("failed to fetch tracer object: ", err)
+return
+end
+
+if #tracing.spans == 0 then
+return
+end
+span.start_time = tracing.spans[1].start_time
+for i, _ in ipairs(tracing.spans or {}) do
+create_child_span(tracer, root_span_ctx, tracing.spans, i)
 end
 end
 
 
--- body_filter maybe not called because of empty http body response
--- so we need to check if the span has finished in log phase
 function _M.log(conf, api_ctx)
 if api_ctx.otel_context_token then
 -- ctx:detach() is not necessary, because of ctx is stored in ngx.ctx
 local upstream_status = core.response.get_upstream_status(api_ctx)
 
 -- get span from current context
-local span = context:current():span()
+local ctx = context:current()
+local span = ctx:span()
 if upstream_status and upstream_status >= 500 then
 span:set_status(span_status.ERROR,
 "upstream response status: " .. upstream_status)
 end
 
+inject_core_spans(ctx, api_ctx, conf)
+span:set_attributes(attr.int("http.status_code", upstream_status))

Review Comment:
   Pls use `http.response.status_code` to comply with the OTEL semantic 
conventions.
   
   https://opentelemetry.io/docs/specs/semconv/registry/attributes/http/



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-29 Thread via GitHub



bzp2010 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2741867364


##
docs/en/latest/plugins/opentelemetry.md:
##
@@ -153,37 +168,152 @@ In OpenTelemetry collector's log, you should see 
information similar to the foll
 
 ```text
 2024-02-18T17:14:03.825Z info ResourceSpans #0
-Resource SchemaURL:
-Resource attributes:
- -> telemetry.sdk.language: Str(lua)
- -> telemetry.sdk.name: Str(opentelemetry-lua)
- -> telemetry.sdk.version: Str(0.1.1)
- -> hostname: Str(e34673e24631)
- -> service.name: Str(APISIX)
 ScopeSpans #0
 ScopeSpans SchemaURL:
 InstrumentationScope opentelemetry-lua
 Span #0
-Trace ID   : fbd0a38d4ea4a128ff1a688197bc58b0
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 5a3835b61110d942
+Name   : http_router_match
+Kind   : Internal
+Start time : 2025-10-24 06:58:04.430430976 + UTC
+End time   : 2025-10-24 06:58:04.431542016 + UTC
+Status code: Unset
+Status message :
+Span #1
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 4ab25e2b92f394e1
+Name   : resolve_dns
+Kind   : Internal
+Start time : 2025-10-24 06:58:04.432521984 + UTC
+End time   : 2025-10-24 06:58:04.44903296 + UTC
+Status code: Unset
+Status message :
+Span #2
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 3620c0f05dd2be4f
+Name   : apisix.phase.header_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960481024 + UTC
+End time   : 2025-10-24 06:58:06.960510976 + UTC
+Status code: Unset
+Status message :
+Span #3
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 4c5f3476f62a7e8a
+ID : a9bfad7bb6986e41
+Name   : apisix.phase.body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960579072 + UTC
+End time   : 2025-10-24 06:58:06.96059008 + UTC
+Status code: Unset
+Status message :
+Span #4
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : b2994675df6baa83
+ID : 26705f9c47584a5b
+Name   : apisix.phase.delayed_body_filter.opentelemetry
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.960613888 + UTC
+End time   : 2025-10-24 06:58:06.960687104 + UTC
+Status code: Unset
+Status message :
+Span #5
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 4c5f3476f62a7e8a
+ID : b2994675df6baa83
+Name   : apisix.phase.delayed_body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.96059904 + UTC
+End time   : 2025-10-24 06:58:06.960692992 + UTC
+Status code: Unset
+Status message :
+Span #6
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 4c5f3476f62a7e8a
+Name   : apisix.phase.body_filter
+Kind   : Server
+Start time : 2025-10-24 06:58:06.96056704 + UTC
+End time   : 2025-10-24 06:58:06.960698112 + UTC
+Status code: Unset
+Status message :
+Span #7
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 2024d73d32cbd81b
+ID : 223c64fb691a24e8
+Name   : apisix.phase.body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961624064 + UTC
+End time   : 2025-10-24 06:58:06.961635072 + UTC
+Status code: Unset
+Status message :
+Span #8
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : fd193dd24c618f60
+ID : 8729ad6e0d94a23b
+Name   : apisix.phase.delayed_body_filter.opentelemetry
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961648896 + UTC
+End time   : 1970-01-01 00:00:00 + UTC
+Status code: Unset
+Status message :
+Span #9
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 2024d73d32cbd81b
+ID : fd193dd24c618f60
+Name   : apisix.phase.delayed_body_filter
+Kind   : Internal
+Start time : 2025-10-24 06:58:06.961641984 + UTC
+End time   : 1970-01-01 00:00:00 + UTC
+Status code: Unset
+Status message :
+Span #10
+Trace ID   : 95a1644afaaf65e1f0193b1f193b990a
+Parent ID  : 905f850f13e32bfb
+ID : 2024d73d32cbd81b
+Name   : apisix.phase.body_filter
+Kind   : Server
+Start time : 2025-10-24 06:58:06.960980992 + UTC
+End time   : 1970-01-01 0

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-28 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2740097712


##
apisix/ssl/router/radixtree_sni.lua:
##
@@ -149,11 +150,15 @@ function _M.match_and_set(api_ctx, match_only, alt_sni)
 local err
 if not radixtree_router or
radixtree_router_ver ~= ssl_certificates.conf_version then
+local span = tracer.new_span("create_router", tracer.kind.internal)
 radixtree_router, err = create_router(ssl_certificates.values)
 if not radixtree_router then
+span:set_status(tracer.status.ERROR, "failed create router")
+tracer.finish_current_span()
 return false, "failed to create radixtree router: " .. err
 end
 radixtree_router_ver = ssl_certificates.conf_version
+tracer.finish_current_span()

Review Comment:
   fixed.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-28 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2740080946


##
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:
   refactor



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-28 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2740080044


##
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:
   fixed.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-28 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2740077904


##
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:
   Cannot be reused, its lifecycle is bound to requests, and it cannot be used 
by multiple requests simultaneously.



##
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:
   Cannot be reused, its lifecycle is bound to requests, and it cannot be used 
by multiple requests simultaneously.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-28 Thread via GitHub



AlinsRan commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2740073756


##
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:
   fixed.



##
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:
   fixed.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-26 Thread via GitHub



github-actions[bot] commented on PR #12686:
URL: https://github.com/apache/apisix/pull/12686#issuecomment-3798950504

   This pull request/issue has been closed due to lack of activity. If you 
think that is incorrect, or the pull request requires review, you can revive 
the PR at any time.


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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2026-01-26 Thread via GitHub



github-actions[bot] closed pull request #12686: feat: add more spans to 
opentelemetry plugin
URL: https://github.com/apache/apisix/pull/12686


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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-12-29 Thread via GitHub



github-actions[bot] commented on PR #12686:
URL: https://github.com/apache/apisix/pull/12686#issuecomment-3696057152

   This pull request has been marked as stale due to 60 days of inactivity. It 
will be closed in 4 weeks if no further activity occurs. If you think that's 
incorrect or this pull request should instead be reviewed, please simply write 
any comment. Even if closed, you can still revive the PR at any time or discuss 
it on the [email protected] list. Thank you for your contributions.


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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-29 Thread via GitHub



membphis commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2476153552


##
apisix/ssl/router/radixtree_sni.lua:
##
@@ -149,11 +150,15 @@ function _M.match_and_set(api_ctx, match_only, alt_sni)
 local err
 if not radixtree_router or
radixtree_router_ver ~= ssl_certificates.conf_version then
+local span = tracer.new_span("create_router", tracer.kind.internal)
 radixtree_router, err = create_router(ssl_certificates.values)
 if not radixtree_router then
+span:set_status(tracer.status.ERROR, "failed create router")
+tracer.finish_current_span()
 return false, "failed to create radixtree router: " .. err
 end
 radixtree_router_ver = ssl_certificates.conf_version
+tracer.finish_current_span()

Review Comment:
   I have the same issue when I review this code first time
   
   APISIX will encounter an error if there are concurrent requests.
   
   https://github.com/user-attachments/assets/e1ef5df2-f7ad-46c3-8673-751aa877d674";
 />
   
   New way(should same as @bzp2010)
   
   https://github.com/user-attachments/assets/8a75f5fe-e0a4-4375-8e97-84c80445be5a";
 />



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-28 Thread via GitHub



bzp2010 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2471659658


##
apisix/ssl/router/radixtree_sni.lua:
##
@@ -149,11 +150,15 @@ function _M.match_and_set(api_ctx, match_only, alt_sni)
 local err
 if not radixtree_router or
radixtree_router_ver ~= ssl_certificates.conf_version then
+local span = tracer.new_span("create_router", tracer.kind.internal)
 radixtree_router, err = create_router(ssl_certificates.values)
 if not radixtree_router then
+span:set_status(tracer.status.ERROR, "failed create router")
+tracer.finish_current_span()
 return false, "failed to create radixtree router: " .. err
 end
 radixtree_router_ver = ssl_certificates.conf_version
+tracer.finish_current_span()

Review Comment:
   Frankly, this API is confusing. The call to end a span should be something 
like `span:finish()`. Currently, using `tracer.finish_current_span()` implies 
that spans are managed by a context record within the tracer. This creates 
confusion for me—could it lead to misuse or conflicts when handling multiple 
requests in parallel or when a single request contains yield operations? 
Especially since we might optimize it in the future to use some kind of table 
pooling mechanism.
   
   This is a concern. While I initially suspect it might not be an issue in 
single-threaded Nginx, we should avoid this confusion in the API design. It 
requires programmers to be thoroughly familiar with the OpenResty programming 
model, understanding the extent to which data is shared and how conflicts might 
arise. This imposes an additional explanation burden on us.
   
   This is from a DX perspective. Technically, we may need to rethink how the 
stack is used to properly connect all spans.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-27 Thread via GitHub



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 dist

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-27 Thread via GitHub



Revolyssup commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2464849801


##
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:
   cannot use core.tablepool due to circular dependency so directly using 
tablepool



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-27 Thread via GitHub



nic-6443 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2464711981


##
apisix/plugins/opentelemetry.lua:
##
@@ -376,13 +376,57 @@ function _M.rewrite(conf, api_ctx)
   ngx_var.opentelemetry_span_id = span_context.span_id
 end
 
+if not ctx:span():is_recording() then
+ngx.ctx._apisix_skip_tracing = true
+end
+
 api_ctx.otel_context_token = ctx:attach()
 
 -- inject trace context into the headers of upstream HTTP request
 trace_context_propagator:inject(ctx, ngx.req)
 end
 
 
+local function create_child_span(tracer, parent_span_ctx, span)
+local new_span_ctx, new_span = tracer:start(parent_span_ctx, span.name,
+{
+kind = span.kind,
+attributes = span.attributes,
+})
+new_span.start_time = span.start_time
+
+for _, child in ipairs(span.children or {}) do
+create_child_span(tracer, new_span_ctx, child)
+end
+if span.status then
+new_span:set_status(span.status.code, span.status.message)
+end
+new_span:finish(span.end_time)
+end
+
+
+local function inject_core_spans(root_span_ctx, api_ctx, conf)
+local metadata = plugin.plugin_metadata(plugin_name)
+local plugin_info = metadata.value
+if root_span_ctx.span and not root_span_ctx:span():is_recording() then
+return
+end
+local conf = core.table.deepcopy(conf)
+conf.sampler.name = "always_on"

Review Comment:
   we should reuse this deepcopy result



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-27 Thread via GitHub



Revolyssup commented on PR #12686:
URL: https://github.com/apache/apisix/pull/12686#issuecomment-3449799354

   # Benchmark
   
   1. Add route
   
   ```jsx
apisix git:(nic/opentelemetry) ✗ curl 
"" -X PUT \\
 -H "X-API-KEY: ${admin_key}" \\
 -d '{
   "id": "otel-tracing-route",
   "uri": "/headers",
   "plugins": {
 "opentelemetry": {
   "sampler": {
 "name": "always_on"
   }
 }
   },
   "upstream": {
 "type": "roundrobin",
 "nodes": {
   "127.0.0.1:8080": 1
 }
   }
 }'
   
   ```
   
   ## Without instrumentation
   
   - Run wrk
   
   ```jsx
wrk -t4 -c100 -d30s http://localhost:9080/headers
   
   Running 30s test @ http://localhost:9080/headers
 4 threads and 100 connections
 Thread Stats   Avg  Stdev Max   +/- Stdev
   Latency 8.26ms9.89ms 224.12ms   93.95%
   Req/Sec 3.63k   793.25 6.19k74.06%
 434324 requests in 30.09s, 142.90MB read
   Requests/sec:  14432.04
   Transfer/sec:  4.75MB
   ```
   
   ## With instrumentation
   
   - Run wrk
   
   ```jsx
   apisix git:(nic/opentelemetry) ✗ wrk -t4 -c100 -d30s 
http://localhost:9080/headers
   
   Running 30s test @ http://localhost:9080/headers
 4 threads and 100 connections
 Thread Stats   Avg  Stdev Max   +/- Stdev
   Latency25.08ms   23.26ms 375.33ms   88.01%
   Req/Sec 1.16k   334.41 2.13k67.03%
 138659 requests in 30.08s, 45.62MB read
   Requests/sec:   4609.57
   Transfer/sec:  1.52MB
   ```
   
   - Check traces
   
   
![screenshot-2025-10-27_12-29-30.png](attachment:3abee5d7-6847-4abb-8400-50d65fd9b22a:screenshot-2025-10-27_12-29-30.png)
   
   📊 Summary:
   After adding OpenTelemetry instrumentation, the request throughput dropped 
by about 68%, and average latency increased roughly threefold.


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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-26 Thread via GitHub



Revolyssup commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2464554153


##
apisix/plugin.lua:
##
@@ -1221,19 +1222,21 @@ function _M.run_plugin(phase, plugins, api_ctx)
 end
 return api_ctx, plugin_run
 end
-
+tracer.new_span("apisix.phase." .. phase)

Review Comment:
   okay i changed the name



##
apisix/core/response.lua:
##
@@ -86,6 +89,9 @@ function resp_exit(code, ...)
 end
 
 if code then
+if code >= 400 then
+tracer.finish_current_span(tracer.status.ERROR, message or 
("response code " .. code))

Review Comment:
   done



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-26 Thread via GitHub



Revolyssup commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2464541680


##
apisix/plugin.lua:
##
@@ -1221,19 +1222,21 @@ function _M.run_plugin(phase, plugins, api_ctx)
 end
 return api_ctx, plugin_run
 end
-
+tracer.new_span("apisix.phase." .. phase)

Review Comment:
   This one only tracks the combined plugin execution time in each phase. The 
manually added ones are more general, maybe we can name them different.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-26 Thread via GitHub



nic-6443 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2464486398


##
apisix/core/response.lua:
##
@@ -86,6 +89,9 @@ function resp_exit(code, ...)
 end
 
 if code then
+if code >= 400 then
+tracer.finish_current_span(tracer.status.ERROR, message or 
("response code " .. code))

Review Comment:
   I think we should finish all pending spans in here, not only current span, 
we should provide another util function in `tracer` for this.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-26 Thread via GitHub



nic-6443 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2464484697


##
apisix/plugin.lua:
##
@@ -1221,19 +1222,21 @@ function _M.run_plugin(phase, plugins, api_ctx)
 end
 return api_ctx, plugin_run
 end
-
+tracer.new_span("apisix.phase." .. phase)

Review Comment:
   is this span duplicate with spans that we start maually like 
`apisix.phase.access` and `apisix.phase.body_filter`?



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-21 Thread via GitHub



nic-6443 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2450511782


##
apisix/utils/span.lua:
##
@@ -0,0 +1,73 @@
+--
+-- 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 util = require("opentelemetry.util")
+local span_status = require("opentelemetry.trace.span_status")
+
+
+local _M = {}
+
+
+local mt = {
+__index = _M
+}
+
+
+function _M.new(name, kind)

Review Comment:
   TODO: reuse span table if there has any proformance issue.



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

Re: [PR] feat: add more spans to opentelemetry plugin [apisix]

2025-10-19 Thread via GitHub



nic-6443 commented on code in PR #12686:
URL: https://github.com/apache/apisix/pull/12686#discussion_r2443621238


##
apisix/core/response.lua:
##
@@ -86,6 +89,9 @@ function resp_exit(code, ...)
 end
 
 if code then
+if code >= 400 then

Review Comment:
   Without this, we would need to add a `tracer.finish_current_span` call in 
every `core.response.exit()` code, which is very troublesome.



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

37 matches

Mail list logo