bug: zipkin plugin throw exception(span already finished) while use plugin

junhao69535 commented 3 months ago

Current Behavior

zipkin plugin throw exception(span already finished) while using zipkin plugin in route and global rule at the same time.

It is known from the documentation: When the same plugin is configured both globally in a global rule and locally in an object (e.g. a route), both plugin instances are executed sequentially.

Therefore, I think it is caused by both the global plugin and the routing plugin calling span.finish.

following is zipkin plugin code, comment explain the cause :

function _M.header_filter(conf, ctx)
    if not ctx.opentracing_sample then
        return
    end

    local opentracing = ctx.opentracing
    local end_time = opentracing.tracer:time()

    if conf.span_version == ZIPKIN_SPAN_VER_1 then
        if  opentracing.proxy_span then
            opentracing.body_filter_span = opentracing.proxy_span:start_child_span(
                "apisix.body_filter", end_time)
        end
    else
        -- call by route plugin and global plugin rule
        opentracing.proxy_span:finish(end_time)
        opentracing.response_span = opentracing.request_span:start_child_span(
            "apisix.response_span", end_time)
    end
end

Expected Behavior

plugin runs normally while using zipkin plugin in route and global rule at the same time

Error Logs

[error] 2289#2289: 34273328 failed to run log_by_lua: /usr/local/apisix//deps/share/lua/5.1/opentracing/span.lua:59: span already finished

Steps to Reproduce

1、configure zipkin plugin in route. 2、configure zipkin plugin in global rule. 3、send a request

Environment

APISIX version (run apisix version):2.14.1
Operating system (run uname -a):Linux apisix-b7d9c6fd4-5tn2r 4.19.0-10-amd64 SMP Debian 4.19.132-1 (2020-07-24) x86_64 GNU/Linux
OpenResty / Nginx version (run openresty -V or nginx -V):openresty/1.21.4.2
etcd version, if relevant (run curl http://127.0.0.1:9090/v1/server_info):3.4.0
APISIX Dashboard version, if relevant:3.0.1
Plugin runner version, for issues related to plugin runners:
LuaRocks version, for installation issues (run luarocks --version):

shreemaan-abhishek commented 3 months ago

to solve this we should disallow executing the plugin second time?

junhao69535 commented 3 months ago

to solve this we should disallow executing the plugin second time? @shreemaan-abhishek

yes, but that is a common rule we can't change.

I think we can add context in ctx.zipkin to avoid call second time.

apache / apisix