spacewander commented on a change in pull request #2389: URL: https://github.com/apache/apisix/pull/2389#discussion_r513896783
########## File path: doc/plugins/skywalking.md ########## @@ -67,59 +64,97 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f1 } }' ``` + You can open dashboard with a browser:`http://127.0.0.1:9080/apisix/dashboard/`，to complete the above operation through the web interface, first add a route:\ -\ +\ Then add skywalking plugin:\ - + + +## How to set endpoint + +We can set the endpoint by specified the configuration in `conf/config.yaml`. + +| Name | Type | Default | Description | +| ------------ | ------ | -------- | -------------------------------------------------------------------- | +| service_name | string | "APISIX" | service name for skywalking reporter | +|service_instance_name|string|"APISIX Instance Name" | service instance name for skywalking reporter | +| endpoint | string | "http://127.0.0.1:12800"; | the http endpoint of Skywalking, for example: http://127.0.0.1:12800 | + +Here is an example: + +```yaml +plugin_attr: + skywalking: + service_name: APISIX + service_instance_name: "APISIX Instance Name" + endpoint_addr: http://127.0.0.1:12800 +``` + ## Test Plugin ### Run-Skywalking-Example #### e.g. + 1. Run Skywalking Server: - - By default,use H2 storage , start skywalking directly - ``` + - By default, use H2 storage , start skywalking directly + + ```shell sudo docker run --name skywalking -d -p 1234:1234 -p 11800:11800 -p 12800:12800 --restart always apache/skywalking-oap-server ``` - - Of Course,you can use elasticsearch storage + - Of Course,you can use elasticsearch storage Review comment: A space is need after the comma ########## File path: doc/plugins/skywalking.md ########## @@ -67,59 +64,97 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f1 } }' ``` + You can open dashboard with a browser:`http://127.0.0.1:9080/apisix/dashboard/`，to complete the above operation through the web interface, first add a route:\ -\ +\ Then add skywalking plugin:\ - + + +## How to set endpoint + +We can set the endpoint by specified the configuration in `conf/config.yaml`. + +| Name | Type | Default | Description | +| ------------ | ------ | -------- | -------------------------------------------------------------------- | +| service_name | string | "APISIX" | service name for skywalking reporter | +|service_instance_name|string|"APISIX Instance Name" | service instance name for skywalking reporter | +| endpoint | string | "http://127.0.0.1:12800"; | the http endpoint of Skywalking, for example: http://127.0.0.1:12800 | + +Here is an example: + +```yaml +plugin_attr: + skywalking: + service_name: APISIX + service_instance_name: "APISIX Instance Name" + endpoint_addr: http://127.0.0.1:12800 +``` + ## Test Plugin ### Run-Skywalking-Example #### e.g. + 1. Run Skywalking Server: - - By default,use H2 storage , start skywalking directly - ``` + - By default, use H2 storage , start skywalking directly Review comment: Better to use English comma ',' ########## File path: apisix/plugins/skywalking.lua ########## @@ -55,26 +76,74 @@ end function _M.rewrite(conf, ctx) core.log.debug("rewrite phase of skywalking plugin") ctx.skywalking_sample = false - if conf.sample_ratio == 1 or math.random() < conf.sample_ratio then + if conf.sample_ratio == 1 or math.random() <= conf.sample_ratio then ctx.skywalking_sample = true - sw_client.heartbeat(conf) - -- Currently, we can not have the upstream real network address - sw_tracer.start(ctx, conf.endpoint, "upstream service") + sw_tracer:start("upstream service") + core.log.info("tracer start") + return end + + core.log.info("miss sampling, ignore") end function _M.body_filter(conf, ctx) if ctx.skywalking_sample and ngx.arg[2] then - sw_tracer.finish(ctx) + sw_tracer:finish() + core.log.info("tracer finish") end end function _M.log(conf, ctx) if ctx.skywalking_sample then - sw_tracer.prepareForReport(ctx, conf.endpoint) + sw_tracer:prepareForReport() + core.log.info("tracer prepare for report") end end + +local function try_read_attr(t, ...) + local count = select('#', ...) + for i = 1, count do + local attr = select(i, ...) + if type(t) ~= "table" then + return nil + end + t = t[attr] + end + + return t +end + + +function _M.init() + if process.type() ~= "worker" and process.type() ~= "single" then + return + end + + local local_conf = core.config.local_conf() + local local_plugin_info = try_read_attr(local_conf, "plugin_attr", + plugin_name) or {} + local_plugin_info = core.table.clone(local_plugin_info) Review comment: Do we need to create a copy of `local_plugin_info`? We don't modify this table below. ########## File path: apisix/plugins/skywalking.lua ########## @@ -55,26 +76,74 @@ end function _M.rewrite(conf, ctx) core.log.debug("rewrite phase of skywalking plugin") ctx.skywalking_sample = false - if conf.sample_ratio == 1 or math.random() < conf.sample_ratio then + if conf.sample_ratio == 1 or math.random() <= conf.sample_ratio then ctx.skywalking_sample = true - sw_client.heartbeat(conf) - -- Currently, we can not have the upstream real network address - sw_tracer.start(ctx, conf.endpoint, "upstream service") + sw_tracer:start("upstream service") + core.log.info("tracer start") + return end + + core.log.info("miss sampling, ignore") end function _M.body_filter(conf, ctx) if ctx.skywalking_sample and ngx.arg[2] then - sw_tracer.finish(ctx) + sw_tracer:finish() + core.log.info("tracer finish") end end function _M.log(conf, ctx) if ctx.skywalking_sample then - sw_tracer.prepareForReport(ctx, conf.endpoint) + sw_tracer:prepareForReport() + core.log.info("tracer prepare for report") end end + +local function try_read_attr(t, ...) Review comment: Better to merge `try_read_attr` and `try_attr` (which can be found in log-rotate and other plugins) into one function. Anyway, we can do it in separate PR later. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
