-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat: add more spans to opentelemetry plugin #12686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 3 commits
b5197a1
f6414fd
0317cf5
7944e9f
ec7adef
bb28639
1b29310
57cac44
f8d5974
0c38fc7
ce4277c
1d6c4e2
1f0cedb
e6c31c0
234f9f7
c3f9ae9
03f3906
12d2513
f6e92b8
0577ab2
b1aaba2
c3f37eb
cab6620
9d195e5
b505630
9fc46b0
c5be5f9
05eda81
956935a
d2bd719
119f9d3
91e37be
fe16d9e
14b4101
079484d
c310ade
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ local apisix_ssl = require("apisix.ssl") | |
| local secret = require("apisix.secret") | ||
| local ngx_ssl = require("ngx.ssl") | ||
| local config_util = require("apisix.core.config_util") | ||
| local tracer = require("apisix.utils.tracer") | ||
| local ngx = ngx | ||
| local ipairs = ipairs | ||
| local type = type | ||
|
|
@@ -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() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Frankly, this API is confusing. The call to end a span should be something like 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have the same issue when I review this code first time APISIX will encounter an error if there are concurrent requests.
New way(should same as @bzp2010)
|
||
| end | ||
|
|
||
| local sni = alt_sni | ||
|
|
@@ -170,15 +175,19 @@ function _M.match_and_set(api_ctx, match_only, alt_sni) | |
| core.log.debug("sni: ", sni) | ||
|
|
||
| local sni_rev = sni:reverse() | ||
| local span = tracer.new_span("sni_radixtree_match", tracer.kind.internal) | ||
| local ok = radixtree_router:dispatch(sni_rev, nil, api_ctx) | ||
| if not ok then | ||
| if not alt_sni then | ||
| -- it is expected that alternative SNI doesn't have a SSL certificate associated | ||
| -- with it sometimes | ||
| core.log.error("failed to find any SSL certificate by SNI: ", sni) | ||
| end | ||
| span:set_status(tracer.status.ERROR, "failed match SNI") | ||
| tracer.finish_current_span() | ||
| return false | ||
| end | ||
| tracer.finish_current_span() | ||
|
|
||
|
|
||
| if type(api_ctx.matched_sni) == "table" then | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
nic-6443 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| local self = { | ||
| name = name, | ||
| start_time = util.time_nano(), | ||
| end_time = 0, | ||
| kind = kind, | ||
| attributes = {}, | ||
| children = {}, | ||
| } | ||
| 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 | ||
|
|
||
|
|
||
| return _M | ||


Uh oh!
There was an error while loading. Please reload this page.