You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/10/29 04:01:19 UTC

[GitHub] [apisix] spacewander commented on a change in pull request #2389: feat: upgrade skywalking plugin to support skywalking 8.0 .

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:\
-![](../images/plugin/skywalking-1.png)\
+![ ](../images/plugin/skywalking-1.png)\
 Then add skywalking plugin:\
-![](../images/plugin/skywalking-2.png)
+![ ](../images/plugin/skywalking-2.png)
+
+## 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:\
-![](../images/plugin/skywalking-1.png)\
+![ ](../images/plugin/skywalking-1.png)\
 Then add skywalking plugin:\
-![](../images/plugin/skywalking-2.png)
+![ ](../images/plugin/skywalking-2.png)
+
+## 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:
users@infra.apache.org