feature: supported zipkin plugin. #304
Conversation
lua/apisix/core/request.lua
Outdated
| @@ -7,7 +7,8 @@ local get_headers = ngx.req.get_headers | |||
| local _M = {version = 0.1} | |||
|
|
|||
|
|
|||
| local function _headers(ctx) | |||
| local function _headers() | |||
| local ctx = ngx.ctx.api_ctx | |||
There was a problem hiding this comment.
bad change, we should keep the advantages of the old code.
| local plugin_name = "zipkin" | ||
|
|
||
|
|
||
| -- You can follow this document to write schema: |
There was a problem hiding this comment.
how about we remove this comment?
lua/apisix/plugins/zipkin.lua
Outdated
|
|
||
|
|
||
| local function create_tracer(conf) | ||
| local tracer = new_tracer(new_reporter(conf), new_random_sampler(conf)) |
There was a problem hiding this comment.
bad indentation, and pls fix other points like this.
lua/apisix/plugins/zipkin.lua
Outdated
| function _M.log(conf, ctx) | ||
| local tracer = ctx.opentracing.tracer | ||
| local reporter = tracer.reporter | ||
| local ok, err = ngx.timer.at(0, report2endpoint, reporter) |
There was a problem hiding this comment.
that a bad way to create a timer for each request.
we can add a todo comment first, will fix it later.
|
You still need some time to complete this PR @moonming |
|
it is a Draft now. |
moonming
force-pushed
the
zipkin
branch
3 times, most recently
from
b0fe670
to
989d941
Compare
lua/apisix/plugins/zipkin.lua
Outdated
| properties = { | ||
| endpoint = {type = "string"}, | ||
| sample_ratio = {type = "number", | ||
| default = 0.001, minimum = 0.00001, maximum = 1} |
There was a problem hiding this comment.
The default value here may be invalid, it is best to add a use case to cover this case.
There was a problem hiding this comment.
json schema in rapidjson not support default value :(
| --- config | ||
| location /t { | ||
| content_by_lua_block { | ||
| local t = require("lib.test_admin").test |
There was a problem hiding this comment.
the size of code block is more than 1k, it may failed. we need to make it smaller.
There was a problem hiding this comment.
need to fix other similar issues.
todo:
copyright
doc