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/11 11:49:10 UTC
[GitHub] [apisix] membphis opened a new pull request #2389: feat: upgrade skywalking plugin to support skywalking 8.0 .
membphis opened a new pull request #2389:
URL: https://github.com/apache/apisix/pull/2389
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
fix #2072
### Pre-submission checklist:
* [ ] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [ ] Is this PR backward compatible?
----------------------------------------------------------------
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
[GitHub] [apisix] gxthrj commented on pull request #2389: [WIP] feat: upgrade skywalking plugin to support skywalking 8.0 .
Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #2389:
URL: https://github.com/apache/apisix/pull/2389#issuecomment-717840113
There are some files conflict
----------------------------------------------------------------
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
[GitHub] [apisix] membphis commented on a change in pull request #2389: feat: upgrade skywalking plugin to support skywalking 8.0 .
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2389:
URL: https://github.com/apache/apisix/pull/2389#discussion_r514005987
##########
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:
This code may be changes the table.
https://github.com/apache/apisix/pull/2389/files/1d2c965beb680ce9c48df286b55104005a1d6ea6#diff-8ae4d69a56407342d1196c01486b54a5d5021c70d955d9ab7793ff870e206063R129
eg: it will set the default value.
##########
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:
we can do this in a new separate PR.
##########
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:
related issue: https://github.com/apache/apisix/issues/2557
----------------------------------------------------------------
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
[GitHub] [apisix] moonming merged pull request #2389: feat: upgrade skywalking plugin to support skywalking 8.0 .
Posted by GitBox <gi...@apache.org>.
moonming merged pull request #2389:
URL: https://github.com/apache/apisix/pull/2389
----------------------------------------------------------------
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
[GitHub] [apisix] membphis commented on pull request #2389: feat: upgrade skywalking plugin to support skywalking 8.0 .
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2389:
URL: https://github.com/apache/apisix/pull/2389#issuecomment-717896940
@gxthrj @moonming fixed conflicts, please take a look when you have time.
----------------------------------------------------------------
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
[GitHub] [apisix] spacewander commented on a change in pull request #2389: feat: upgrade skywalking plugin to support skywalking 8.0 .
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org