You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/04/04 07:44:01 UTC
[GitHub] [skywalking-nginx-lua] mrproliu opened a new pull request #30:
Adapt the new v3 protocol
mrproliu opened a new pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30
Follow the [new v3 protocol](https://github.com/apache/skywalking/pull/4599#issuecomment-607540654), I rewrite the Nginx agent.
Currently, I'm still waiting for the backend to finish, and then I can carry out the E2E test.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] mrproliu commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404669345
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
--- Format a trace/segment id into an array.
--- An official ID should have three parts separated by '.' and each part of it is a number
-function _M.formatID(str)
- local regex = '.'
- if _M.is_ngx_lua then
- regex = [[\.]]
- end
- local parts = split(str, regex)
- if #parts ~= 3 then
- return nil
- end
+math.randomseed(random_seed())
- return parts
+-- for Nginx Lua
+local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
Review comment:
Got it.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] mrproliu commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404658856
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
--- Format a trace/segment id into an array.
--- An official ID should have three parts separated by '.' and each part of it is a number
-function _M.formatID(str)
- local regex = '.'
- if _M.is_ngx_lua then
- regex = [[\.]]
- end
- local parts = split(str, regex)
- if #parts ~= 3 then
- return nil
- end
+math.randomseed(random_seed())
- return parts
+-- for Nginx Lua
+local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
+if ok then
+ uuid.seed()
+ newID = function()
+ return uuid.generate_v4()
+ end
Review comment:
When the unit test, or the nginx not running on the LuaJIT environment.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404077464
##########
File path: README.md
##########
@@ -82,20 +82,20 @@ By using the `/examples/nginx.conf`, you could start the Nginx with LUA module o
Then you could
1. See the `register logs` happens on the console log.
```
-2020/02/27 09:22:06 [debug] 20676#11799235: *4 [lua] content_by_lua(nginx.conf:118):4: Service register request = {"services":[{"type":"normal","serviceName":"User Service Name"}]}
-2020/02/27 09:22:06 [debug] 20676#11799235: *2 [lua] client.lua:99: registerService(): Service register response = [{"key":"User Service Name","value":1}]
+2020/04/04 15:15:37 [debug] 12089#1446111: *4 [lua] content_by_lua(nginx.conf:175):4: Instance report request = {"service":"User Service Name","serviceInstance":"User Service Instance Name","properties":[{"language":"Lua"}]}
Review comment:
This isn't register logs, please change the title, this is instance properties update logs.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] membphis commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404509963
##########
File path: lib/skywalking/client.lua
##########
@@ -72,110 +60,54 @@ function Client:startBackendTimer(backend_http_uri)
end
end
--- Register service
-function Client:registerService(metadata_buffer, backend_http_uri)
+function Client:reportServiceInstance(metadata_buffer, backend_http_uri)
local log = ngx.log
local DEBUG = ngx.DEBUG
local ERR = ngx.ERR
local serviceName = metadata_buffer:get('serviceName')
+ local serviceInstanceName = metadata_buffer:get('serviceInstanceName')
local cjson = require('cjson')
- local serviceRegister = require("register").newServiceRegister(serviceName)
- local serviceRegisterParam = cjson.encode(serviceRegister)
+ local reportInstance = require("management").newReportInstanceProperties(serviceName, serviceInstanceName)
+ local reportInstanceParam = cjson.encode(reportInstance)
Review comment:
it may fail, we need to capture the error message.
`local data, err = json.encode(...)`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] membphis commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404529160
##########
File path: lib/skywalking/client.lua
##########
@@ -166,16 +102,19 @@ function Client:ping(metadata_buffer, backend_http_uri)
local DEBUG = ngx.DEBUG
local ERR = ngx.ERR
+ local serviceName = metadata_buffer:get('serviceName')
+ local serviceInstanceName = metadata_buffer:get('serviceInstanceName')
+
local cjson = require('cjson')
- local pingPkg = require("register").newServiceInstancePingPkg(
- metadata_buffer:get('serviceInstId'),
- metadata_buffer:get('serviceInstanceUUID'),
- ngx.now() * 1000)
- local pingPkgParam = cjson.encode(pingPkg)
+ local pingPkg = require("management").newServiceInstancePingPkg(serviceName, serviceInstanceName)
+ local pingPkgParam, err = cjson.encode(pingPkg)
+ if err ~= nil then
Review comment:
`if err then` this style is simpler
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] moonming commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404671219
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
--- Format a trace/segment id into an array.
--- An official ID should have three parts separated by '.' and each part of it is a number
-function _M.formatID(str)
- local regex = '.'
- if _M.is_ngx_lua then
- regex = [[\.]]
- end
- local parts = split(str, regex)
- if #parts ~= 3 then
- return nil
- end
+math.randomseed(random_seed())
- return parts
+-- for Nginx Lua
+local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
Review comment:
yes, you can add this lib in https://github.com/apache/skywalking-nginx-lua/blob/master/skywalking-nginx-lua-1.0-0.rockspec#L15
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] moonming commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404535944
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
--- Format a trace/segment id into an array.
--- An official ID should have three parts separated by '.' and each part of it is a number
-function _M.formatID(str)
- local regex = '.'
- if _M.is_ngx_lua then
- regex = [[\.]]
- end
- local parts = split(str, regex)
- if #parts ~= 3 then
- return nil
- end
+math.randomseed(random_seed())
- return parts
+-- for Nginx Lua
+local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
+if ok then
+ uuid.seed()
+ newID = function()
+ return uuid.generate_v4()
+ end
Review comment:
```
if not ok then
return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
```
is better.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404079378
##########
File path: lib/skywalking/client.lua
##########
@@ -17,8 +17,8 @@
local Client = {}
--- Tracing timer does the service and instance register
--- After register successfully, it sends traces and heart beat
+-- Tracing timer does the instance report and sends traces
Review comment:
```suggestion
-- Tracing timer reports instance properties report, keeps alive and sends traces
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] mrproliu commented on issue #30: Adapt the
new v3 protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on issue #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#issuecomment-610153529
@membphis Thanks for your review.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng merged pull request #30: Adapt the
new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r406022025
##########
File path: lib/skywalking/tracer.lua
##########
@@ -25,20 +25,15 @@ function Tracer:start(upstream_name)
local tracingContext
local serviceName = metadata_buffer:get("serviceName")
- local serviceInstId = metadata_buffer:get("serviceInstId")
- local serviceId = metadata_buffer:get('serviceId')
- if (serviceInstId ~= nil and serviceInstId ~= 0) then
- tracingContext = TC.new(serviceId, serviceInstId)
- else
- tracingContext = TC.newNoOP()
- end
+ local serviceInstanceName = metadata_buffer:get('serviceInstanceName')
+ tracingContext = TC.new(serviceName, serviceInstanceName)
-- Constant pre-defined in SkyWalking main repo
-- 84 represents Nginx
Review comment:
Send a separated PR for this, I am merging this one.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] mrproliu commented on issue #30: Adapt the
new v3 protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on issue #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#issuecomment-609684500
/run ci
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] mrproliu commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404661165
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
--- Format a trace/segment id into an array.
--- An official ID should have three parts separated by '.' and each part of it is a number
-function _M.formatID(str)
- local regex = '.'
- if _M.is_ngx_lua then
- regex = [[\.]]
- end
- local parts = split(str, regex)
- if #parts ~= 3 then
- return nil
- end
+math.randomseed(random_seed())
- return parts
+-- for Nginx Lua
+local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
Review comment:
Are you suggest we need to move `dependencies` files into the parent directory?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] moonming commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404534761
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
--- Format a trace/segment id into an array.
--- An official ID should have three parts separated by '.' and each part of it is a number
-function _M.formatID(str)
- local regex = '.'
- if _M.is_ngx_lua then
- regex = [[\.]]
- end
- local parts = split(str, regex)
- if #parts ~= 3 then
- return nil
- end
+math.randomseed(random_seed())
- return parts
+-- for Nginx Lua
+local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
Review comment:
we shoulde remove `dependencies` directory:
```
local ok, uuid = pcall(require, "lua-resty-jit-uuid")
```
Because most users will use `luarocks` to manage dependencies.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] moonming commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404535763
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
Review comment:
This function will always run, which is redundant in an OpenResty environment
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404080120
##########
File path: lib/skywalking/client.lua
##########
@@ -17,8 +17,8 @@
local Client = {}
--- Tracing timer does the service and instance register
--- After register successfully, it sends traces and heart beat
+-- Tracing timer does the instance report and sends traces
+-- After report successfully, it will heart beat
Review comment:
```suggestion
-- After report instance properties successfully, it sends keep alive packages.
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] membphis commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404529779
##########
File path: lib/skywalking/management.lua
##########
@@ -0,0 +1,46 @@
+--
+-- 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 _M = {}
+
+-- Return Services as service register parameter
+function _M.newReportInstanceProperties(serviceName, serviceInstance)
+ local instanceProperties = {
+ service = serviceName,
+ serviceInstance = serviceInstance,
+ properties = {}
+ }
+
+ local allProperties = {
+ language = "Lua"
+ }
+
+ instanceProperties.properties[#instanceProperties.properties + 1] = allProperties
+
+ return instanceProperties
+end
+
+function _M.newServiceInstancePingPkg(serviceName, serviceInstance)
+ local serviceInstancePingPkg = {
Review comment:
`return {...}` another style
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r406021461
##########
File path: lib/skywalking/tracer.lua
##########
@@ -25,20 +25,15 @@ function Tracer:start(upstream_name)
local tracingContext
local serviceName = metadata_buffer:get("serviceName")
- local serviceInstId = metadata_buffer:get("serviceInstId")
- local serviceId = metadata_buffer:get('serviceId')
- if (serviceInstId ~= nil and serviceInstId ~= 0) then
- tracingContext = TC.new(serviceId, serviceInstId)
- else
- tracingContext = TC.newNoOP()
- end
+ local serviceInstanceName = metadata_buffer:get('serviceInstanceName')
+ tracingContext = TC.new(serviceName, serviceInstanceName)
-- Constant pre-defined in SkyWalking main repo
-- 84 represents Nginx
Review comment:
This is a previous wrong comment, please fix this 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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404639410
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
--- Format a trace/segment id into an array.
--- An official ID should have three parts separated by '.' and each part of it is a number
-function _M.formatID(str)
- local regex = '.'
- if _M.is_ngx_lua then
- regex = [[\.]]
- end
- local parts = split(str, regex)
- if #parts ~= 3 then
- return nil
- end
+math.randomseed(random_seed())
- return parts
+-- for Nginx Lua
+local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
+if ok then
+ uuid.seed()
+ newID = function()
+ return uuid.generate_v4()
+ end
Review comment:
When does this go to wrong side?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on issue #30: Adapt the
new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#issuecomment-610144970
At https://github.com/apache/skywalking/pull/4599, all java plugin tests are passed. And we are just waiting for @kezhenxu94 updating the e2e test result.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] membphis commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404530386
##########
File path: lib/skywalking/segment_ref.lua
##########
@@ -31,131 +31,78 @@ local _M = {}
-- trace_id,
-- segment_id,
-- span_id,
--- network_address,
--- network_address_id = 0,
--- entry_service_instance_id = 0,
--- parent_service_instance_id = 0,
--- entry_endpoint_name,
--- entry_endpoint_id = 0,
--- parent_endpoint_name,
--- parent_endpoint_id = 0,
+-- parent_service,
+-- parent_service_instance,
+-- parent_endpoint,
+-- address_used_at_client,
-- }
function _M.new()
return {
type = 'CROSS_PROCESS',
- network_address_id = 0,
- entry_service_instance_id = 0,
- parent_service_instance_id = 0,
- entry_endpoint_id = 0,
- parent_endpoint_id = 0,
}
end
-- Deserialize value from the propagated context and initialize the SegmentRef
-function _M.fromSW6Value(value)
+function _M.fromSW8Value(value)
local ref = _M.new()
local parts = Util.split(value, '-')
- if #parts ~= 9 then
+ if #parts ~= 8 then
return nil
end
- ref.trace_id = Util.formatID(decode_base64(parts[2]))
- ref.segment_id = Util.formatID(decode_base64(parts[3]))
+ ref.trace_id = decode_base64(parts[2])
+ ref.segment_id = decode_base64(parts[3])
ref.span_id = tonumber(parts[4])
- ref.parent_service_instance_id = tonumber(parts[5])
- ref.entry_service_instance_id = tonumber(parts[6])
- local peerStr = decode_base64(parts[7])
- if string.sub(peerStr, 1, 1) == '#' then
- ref.network_address = string.sub(peerStr, 2)
- else
- ref.network_address_id = tonumber(peerStr)
- end
- local entryEndpointStr = decode_base64(parts[8])
- if string.sub(entryEndpointStr, 1, 1) == '#' then
- ref.entry_endpoint_name = string.sub(entryEndpointStr, 2)
- else
- ref.entry_endpoint_id = tonumber(entryEndpointStr)
- end
- local parentEndpointStr = decode_base64(parts[9])
- if string.sub(parentEndpointStr, 1, 1) == '#' then
- ref.parent_endpoint_name = string.sub(parentEndpointStr, 2)
- else
- ref.parent_endpoint_id = tonumber(parentEndpointStr)
- end
+ ref.parent_service = decode_base64(parts[5])
+ ref.parent_service_instance = decode_base64(parts[6])
+ ref.parent_endpoint = decode_base64(parts[7])
+ ref.address_used_at_client = decode_base64(parts[8])
return ref
end
-- Return string to represent this ref.
function _M.serialize(ref)
local encodedRef = '1'
- encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.trace_id))
- encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.segment_id))
+ encodedRef = encodedRef .. '-' .. encode_base64(ref.trace_id)
+ encodedRef = encodedRef .. '-' .. encode_base64(ref.segment_id)
encodedRef = encodedRef .. '-' .. ref.span_id
- encodedRef = encodedRef .. '-' .. ref.parent_service_instance_id
- encodedRef = encodedRef .. '-' .. ref.entry_service_instance_id
-
- local networkAddress
- if ref.network_address_id ~= 0 then
- networkAddress = ref.network_address_id .. ''
- else
- networkAddress = '#' .. ref.network_address
- end
- encodedRef = encodedRef .. '-' .. encode_base64(networkAddress)
-
- local entryEndpoint
- if ref.entry_endpoint_id ~= 0 then
- entryEndpoint = ref.entry_endpoint_id .. ''
- else
- entryEndpoint = '#' .. ref.entry_endpoint_name
- end
- encodedRef = encodedRef .. '-' .. encode_base64(entryEndpoint)
-
- local parentEndpoint
- if ref.parent_endpoint_id ~= 0 then
- parentEndpoint = ref.parent_endpoint_id .. ''
- else
- parentEndpoint = '#' .. ref.parent_endpoint_name
- end
- encodedRef = encodedRef .. '-' .. encode_base64(parentEndpoint)
+ encodedRef = encodedRef .. '-' .. encode_base64(ref.parent_service)
Review comment:
this style is better performance.
```lua
encodedRef = encodedRef .. '-' .. xxx
.. '-' .. xxx
.. '-' .. xxx
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] moonming commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404533915
##########
File path: lib/skywalking/segment_ref.lua
##########
@@ -31,131 +31,78 @@ local _M = {}
-- trace_id,
-- segment_id,
-- span_id,
--- network_address,
--- network_address_id = 0,
--- entry_service_instance_id = 0,
--- parent_service_instance_id = 0,
--- entry_endpoint_name,
--- entry_endpoint_id = 0,
--- parent_endpoint_name,
--- parent_endpoint_id = 0,
+-- parent_service,
+-- parent_service_instance,
+-- parent_endpoint,
+-- address_used_at_client,
-- }
function _M.new()
return {
type = 'CROSS_PROCESS',
- network_address_id = 0,
- entry_service_instance_id = 0,
- parent_service_instance_id = 0,
- entry_endpoint_id = 0,
- parent_endpoint_id = 0,
}
end
-- Deserialize value from the propagated context and initialize the SegmentRef
-function _M.fromSW6Value(value)
+function _M.fromSW8Value(value)
local ref = _M.new()
local parts = Util.split(value, '-')
- if #parts ~= 9 then
+ if #parts ~= 8 then
return nil
end
- ref.trace_id = Util.formatID(decode_base64(parts[2]))
- ref.segment_id = Util.formatID(decode_base64(parts[3]))
+ ref.trace_id = decode_base64(parts[2])
+ ref.segment_id = decode_base64(parts[3])
ref.span_id = tonumber(parts[4])
- ref.parent_service_instance_id = tonumber(parts[5])
- ref.entry_service_instance_id = tonumber(parts[6])
- local peerStr = decode_base64(parts[7])
- if string.sub(peerStr, 1, 1) == '#' then
- ref.network_address = string.sub(peerStr, 2)
- else
- ref.network_address_id = tonumber(peerStr)
- end
- local entryEndpointStr = decode_base64(parts[8])
- if string.sub(entryEndpointStr, 1, 1) == '#' then
- ref.entry_endpoint_name = string.sub(entryEndpointStr, 2)
- else
- ref.entry_endpoint_id = tonumber(entryEndpointStr)
- end
- local parentEndpointStr = decode_base64(parts[9])
- if string.sub(parentEndpointStr, 1, 1) == '#' then
- ref.parent_endpoint_name = string.sub(parentEndpointStr, 2)
- else
- ref.parent_endpoint_id = tonumber(parentEndpointStr)
- end
+ ref.parent_service = decode_base64(parts[5])
+ ref.parent_service_instance = decode_base64(parts[6])
+ ref.parent_endpoint = decode_base64(parts[7])
+ ref.address_used_at_client = decode_base64(parts[8])
return ref
end
-- Return string to represent this ref.
function _M.serialize(ref)
local encodedRef = '1'
- encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.trace_id))
- encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.segment_id))
+ encodedRef = encodedRef .. '-' .. encode_base64(ref.trace_id)
+ encodedRef = encodedRef .. '-' .. encode_base64(ref.segment_id)
encodedRef = encodedRef .. '-' .. ref.span_id
- encodedRef = encodedRef .. '-' .. ref.parent_service_instance_id
- encodedRef = encodedRef .. '-' .. ref.entry_service_instance_id
-
- local networkAddress
- if ref.network_address_id ~= 0 then
- networkAddress = ref.network_address_id .. ''
- else
- networkAddress = '#' .. ref.network_address
- end
- encodedRef = encodedRef .. '-' .. encode_base64(networkAddress)
-
- local entryEndpoint
- if ref.entry_endpoint_id ~= 0 then
- entryEndpoint = ref.entry_endpoint_id .. ''
- else
- entryEndpoint = '#' .. ref.entry_endpoint_name
- end
- encodedRef = encodedRef .. '-' .. encode_base64(entryEndpoint)
-
- local parentEndpoint
- if ref.parent_endpoint_id ~= 0 then
- parentEndpoint = ref.parent_endpoint_id .. ''
- else
- parentEndpoint = '#' .. ref.parent_endpoint_name
- end
- encodedRef = encodedRef .. '-' .. encode_base64(parentEndpoint)
+ encodedRef = encodedRef .. '-' .. encode_base64(ref.parent_service)
Review comment:
@membphis code styles are low priority things, and I think we should start with the structure to see if it fits with OpenResty and Apache APISIX users
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404667058
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
--- Format a trace/segment id into an array.
--- An official ID should have three parts separated by '.' and each part of it is a number
-function _M.formatID(str)
- local regex = '.'
- if _M.is_ngx_lua then
- regex = [[\.]]
- end
- local parts = split(str, regex)
- if #parts ~= 3 then
- return nil
- end
+math.randomseed(random_seed())
- return parts
+-- for Nginx Lua
+local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
Review comment:
I think @moonming 's point is, removing the copied files in the repo. As luarocks install the lib, we just need run some commands/add guides in the documentation.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] moonming commented on issue #30: Adapt the
new v3 protocol
Posted by GitBox <gi...@apache.org>.
moonming commented on issue #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#issuecomment-610175147
one more thing, is it works with the current version of skywalking? Or we should merge this PR after skywalking 8.0 released?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] membphis commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404535409
##########
File path: lib/skywalking/segment_ref.lua
##########
@@ -31,131 +31,78 @@ local _M = {}
-- trace_id,
-- segment_id,
-- span_id,
--- network_address,
--- network_address_id = 0,
--- entry_service_instance_id = 0,
--- parent_service_instance_id = 0,
--- entry_endpoint_name,
--- entry_endpoint_id = 0,
--- parent_endpoint_name,
--- parent_endpoint_id = 0,
+-- parent_service,
+-- parent_service_instance,
+-- parent_endpoint,
+-- address_used_at_client,
-- }
function _M.new()
return {
type = 'CROSS_PROCESS',
- network_address_id = 0,
- entry_service_instance_id = 0,
- parent_service_instance_id = 0,
- entry_endpoint_id = 0,
- parent_endpoint_id = 0,
}
end
-- Deserialize value from the propagated context and initialize the SegmentRef
-function _M.fromSW6Value(value)
+function _M.fromSW8Value(value)
local ref = _M.new()
local parts = Util.split(value, '-')
- if #parts ~= 9 then
+ if #parts ~= 8 then
return nil
end
- ref.trace_id = Util.formatID(decode_base64(parts[2]))
- ref.segment_id = Util.formatID(decode_base64(parts[3]))
+ ref.trace_id = decode_base64(parts[2])
+ ref.segment_id = decode_base64(parts[3])
ref.span_id = tonumber(parts[4])
- ref.parent_service_instance_id = tonumber(parts[5])
- ref.entry_service_instance_id = tonumber(parts[6])
- local peerStr = decode_base64(parts[7])
- if string.sub(peerStr, 1, 1) == '#' then
- ref.network_address = string.sub(peerStr, 2)
- else
- ref.network_address_id = tonumber(peerStr)
- end
- local entryEndpointStr = decode_base64(parts[8])
- if string.sub(entryEndpointStr, 1, 1) == '#' then
- ref.entry_endpoint_name = string.sub(entryEndpointStr, 2)
- else
- ref.entry_endpoint_id = tonumber(entryEndpointStr)
- end
- local parentEndpointStr = decode_base64(parts[9])
- if string.sub(parentEndpointStr, 1, 1) == '#' then
- ref.parent_endpoint_name = string.sub(parentEndpointStr, 2)
- else
- ref.parent_endpoint_id = tonumber(parentEndpointStr)
- end
+ ref.parent_service = decode_base64(parts[5])
+ ref.parent_service_instance = decode_base64(parts[6])
+ ref.parent_endpoint = decode_base64(parts[7])
+ ref.address_used_at_client = decode_base64(parts[8])
return ref
end
-- Return string to represent this ref.
function _M.serialize(ref)
local encodedRef = '1'
- encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.trace_id))
- encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.segment_id))
+ encodedRef = encodedRef .. '-' .. encode_base64(ref.trace_id)
+ encodedRef = encodedRef .. '-' .. encode_base64(ref.segment_id)
encodedRef = encodedRef .. '-' .. ref.span_id
- encodedRef = encodedRef .. '-' .. ref.parent_service_instance_id
- encodedRef = encodedRef .. '-' .. ref.entry_service_instance_id
-
- local networkAddress
- if ref.network_address_id ~= 0 then
- networkAddress = ref.network_address_id .. ''
- else
- networkAddress = '#' .. ref.network_address
- end
- encodedRef = encodedRef .. '-' .. encode_base64(networkAddress)
-
- local entryEndpoint
- if ref.entry_endpoint_id ~= 0 then
- entryEndpoint = ref.entry_endpoint_id .. ''
- else
- entryEndpoint = '#' .. ref.entry_endpoint_name
- end
- encodedRef = encodedRef .. '-' .. encode_base64(entryEndpoint)
-
- local parentEndpoint
- if ref.parent_endpoint_id ~= 0 then
- parentEndpoint = ref.parent_endpoint_id .. ''
- else
- parentEndpoint = '#' .. ref.parent_endpoint_name
- end
- encodedRef = encodedRef .. '-' .. encode_base64(parentEndpoint)
+ encodedRef = encodedRef .. '-' .. encode_base64(ref.parent_service)
Review comment:
I'm learning this project, give me more 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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] membphis commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404510152
##########
File path: lib/skywalking/client.lua
##########
@@ -72,110 +60,54 @@ function Client:startBackendTimer(backend_http_uri)
end
end
--- Register service
-function Client:registerService(metadata_buffer, backend_http_uri)
+function Client:reportServiceInstance(metadata_buffer, backend_http_uri)
local log = ngx.log
local DEBUG = ngx.DEBUG
local ERR = ngx.ERR
local serviceName = metadata_buffer:get('serviceName')
+ local serviceInstanceName = metadata_buffer:get('serviceInstanceName')
local cjson = require('cjson')
- local serviceRegister = require("register").newServiceRegister(serviceName)
- local serviceRegisterParam = cjson.encode(serviceRegister)
+ local reportInstance = require("management").newReportInstanceProperties(serviceName, serviceInstanceName)
+ local reportInstanceParam = cjson.encode(reportInstance)
local http = require('resty.http')
local httpc = http.new()
- local res, err = httpc:request_uri(backend_http_uri .. '/v2/service/register', {
+ local res, err = httpc:request_uri(backend_http_uri .. '/v3/management/reportProperties', {
method = "POST",
- body = serviceRegisterParam,
+ body = reportInstanceParam,
headers = {
["Content-Type"] = "application/json",
},
})
if not res then
- log(ERR, "Service register fails, " .. err)
+ log(ERR, "Instance report fails, " .. err)
Review comment:
better style: `log(ERR, "Instance report fails, ", err)`
it is a simple and better performance.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] moonming commented on issue #30: Adapt the
new v3 protocol
Posted by GitBox <gi...@apache.org>.
moonming commented on issue #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#issuecomment-610291976
yes, we should keep the content of 1.1.0.
Thanks,
Ming Wen
Twitter: _WenMing
mrproliu <no...@github.com> 于2020年4月7日周二 下午5:44写道：
> *@mrproliu* commented on this pull request.
> ------------------------------
>
> In lib/skywalking/util.lua
> <https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404677673>
> :
>
> >
> - return parts
> +-- for Nginx Lua
> +local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
>
> We have the file deleted in 2.0.0, lib/skywalking/register.lua.
> The skywalking-nginx-lua-1.0-0.rockspec need to keep the content?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404677673>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGJZBK5A2CY6DNOIIGUHEODRLLYXJANCNFSM4L5RSS2Q>
> .
>
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] mrproliu commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404677673
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
--- Format a trace/segment id into an array.
--- An official ID should have three parts separated by '.' and each part of it is a number
-function _M.formatID(str)
- local regex = '.'
- if _M.is_ngx_lua then
- regex = [[\.]]
- end
- local parts = split(str, regex)
- if #parts ~= 3 then
- return nil
- end
+math.randomseed(random_seed())
- return parts
+-- for Nginx Lua
+local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
Review comment:
We have the file deleted in 2.0.0, `lib/skywalking/register.lua`.
The `skywalking-nginx-lua-1.0-0.rockspec` need to keep the content?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404078204
##########
File path: README.md
##########
@@ -82,20 +82,20 @@ By using the `/examples/nginx.conf`, you could start the Nginx with LUA module o
Then you could
1. See the `register logs` happens on the console log.
```
-2020/02/27 09:22:06 [debug] 20676#11799235: *4 [lua] content_by_lua(nginx.conf:118):4: Service register request = {"services":[{"type":"normal","serviceName":"User Service Name"}]}
-2020/02/27 09:22:06 [debug] 20676#11799235: *2 [lua] client.lua:99: registerService(): Service register response = [{"key":"User Service Name","value":1}]
+2020/04/04 15:15:37 [debug] 12089#1446111: *4 [lua] content_by_lua(nginx.conf:175):4: Instance report request = {"service":"User Service Name","serviceInstance":"User Service Instance Name","properties":[{"language":"Lua"}]}
+2020/04/04 15:15:37 [debug] 12089#1446111: *2 [lua] client.lua:89: reportServiceInstance(): Instance report response = {}
+```
-2020/02/27 09:22:06 [debug] 20676#11799235: *2 [lua] client.lua:106: registerService(): Service registered, service id = 1
-2020/02/27 09:22:06 [debug] 20676#11799235: *4 [lua] content_by_lua(nginx.conf:134):3: Service instance register request = {"instances":[{"time":1582766526928,"instanceUUID":"name:User Service Instance Name","properties":[{"key":"language","value":"Lua"}],"serviceId":1}]}
-2020/02/27 09:22:06 [debug] 20676#11799235: *2 [lua] client.lua:143: registerServiceInstance(): Service Instance register response = [{"key":"name:User Service Instance Name","value":1}]
-2020/02/27 09:22:06 [debug] 20676#11799235: *2 [lua] client.lua:150: registerServiceInstance(): Service Instance registered, service instance id = 1
+1. See the `heartbeat logs` happens after the `register logs`
+```
+2020/04/04 15:15:40 [debug] 12089#1446111: *4 [lua] content_by_lua(nginx.conf:188):3: KeepAlive request = {"service":"User Service Name","serviceInstance":"User Service Instance Name"}
```
-2. Access the `http://127.0.0.1:8080/ingress` then you could see the tracing happens and reported spans in the logs.
+1. Access the `http://127.0.0.1:8080/ingress` then you could see the tracing happens and reported spans in the logs.
Review comment:
```suggestion
3. Access the `http://127.0.0.1:8080/ingress` then you could see the tracing happens and reported spans in the logs.
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] mrproliu commented on issue #30: Adapt the
new v3 protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on issue #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#issuecomment-610145550
> LGTM. Please test this with 8.0 core backend and java agent. They are ready to test.
Sure.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404078060
##########
File path: README.md
##########
@@ -82,20 +82,20 @@ By using the `/examples/nginx.conf`, you could start the Nginx with LUA module o
Then you could
1. See the `register logs` happens on the console log.
```
-2020/02/27 09:22:06 [debug] 20676#11799235: *4 [lua] content_by_lua(nginx.conf:118):4: Service register request = {"services":[{"type":"normal","serviceName":"User Service Name"}]}
-2020/02/27 09:22:06 [debug] 20676#11799235: *2 [lua] client.lua:99: registerService(): Service register response = [{"key":"User Service Name","value":1}]
+2020/04/04 15:15:37 [debug] 12089#1446111: *4 [lua] content_by_lua(nginx.conf:175):4: Instance report request = {"service":"User Service Name","serviceInstance":"User Service Instance Name","properties":[{"language":"Lua"}]}
+2020/04/04 15:15:37 [debug] 12089#1446111: *2 [lua] client.lua:89: reportServiceInstance(): Instance report response = {}
+```
-2020/02/27 09:22:06 [debug] 20676#11799235: *2 [lua] client.lua:106: registerService(): Service registered, service id = 1
-2020/02/27 09:22:06 [debug] 20676#11799235: *4 [lua] content_by_lua(nginx.conf:134):3: Service instance register request = {"instances":[{"time":1582766526928,"instanceUUID":"name:User Service Instance Name","properties":[{"key":"language","value":"Lua"}],"serviceId":1}]}
-2020/02/27 09:22:06 [debug] 20676#11799235: *2 [lua] client.lua:143: registerServiceInstance(): Service Instance register response = [{"key":"name:User Service Instance Name","value":1}]
-2020/02/27 09:22:06 [debug] 20676#11799235: *2 [lua] client.lua:150: registerServiceInstance(): Service Instance registered, service instance id = 1
+1. See the `heartbeat logs` happens after the `register logs`
Review comment:
```suggestion
2. See the `heartbeat logs` happens after the `register logs`
```
This can't be ordered automatically, as you injected logs.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] membphis commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404510199
##########
File path: lib/skywalking/client.lua
##########
@@ -72,110 +60,54 @@ function Client:startBackendTimer(backend_http_uri)
end
end
--- Register service
-function Client:registerService(metadata_buffer, backend_http_uri)
+function Client:reportServiceInstance(metadata_buffer, backend_http_uri)
local log = ngx.log
local DEBUG = ngx.DEBUG
local ERR = ngx.ERR
local serviceName = metadata_buffer:get('serviceName')
+ local serviceInstanceName = metadata_buffer:get('serviceInstanceName')
local cjson = require('cjson')
- local serviceRegister = require("register").newServiceRegister(serviceName)
- local serviceRegisterParam = cjson.encode(serviceRegister)
+ local reportInstance = require("management").newReportInstanceProperties(serviceName, serviceInstanceName)
+ local reportInstanceParam = cjson.encode(reportInstance)
local http = require('resty.http')
local httpc = http.new()
- local res, err = httpc:request_uri(backend_http_uri .. '/v2/service/register', {
+ local res, err = httpc:request_uri(backend_http_uri .. '/v3/management/reportProperties', {
method = "POST",
- body = serviceRegisterParam,
+ body = reportInstanceParam,
headers = {
["Content-Type"] = "application/json",
},
})
if not res then
- log(ERR, "Service register fails, " .. err)
+ log(ERR, "Instance report fails, " .. err)
elseif res.status == 200 then
- log(DEBUG, "Service register response = " .. res.body)
- local registerResults = cjson.decode(res.body)
-
- for i, result in ipairs(registerResults)
- do
- if result.key == serviceName then
- local serviceId = result.value
- log(DEBUG, "Service registered, service id = " .. serviceId)
- metadata_buffer:set('serviceId', serviceId)
- end
- end
+ log(DEBUG, "Instance report response = " .. res.body)
Review comment:
ditoo
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] moonming commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404671219
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
--- Format a trace/segment id into an array.
--- An official ID should have three parts separated by '.' and each part of it is a number
-function _M.formatID(str)
- local regex = '.'
- if _M.is_ngx_lua then
- regex = [[\.]]
- end
- local parts = split(str, regex)
- if #parts ~= 3 then
- return nil
- end
+math.randomseed(random_seed())
- return parts
+-- for Nginx Lua
+local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
Review comment:
yes, you can add this lib in https://github.com/apache/skywalking-nginx-lua/blob/master/skywalking-nginx-lua-1.0-0.rockspec#L15
by the way, you should add a new file `skywalking-nginx-lua-2.0-0.rockspec` for this release.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on issue #30: Adapt the
new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#issuecomment-610145790
And test results should feedback to mail `[8.0.0] Move forward to 8.0.0. REMOVE the register and inventory permanently`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] mrproliu commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404709611
##########
File path: lib/skywalking/util.lua
##########
@@ -80,30 +80,21 @@ local random_seed = function ()
return seed
end
-math.randomseed(random_seed())
-
-function _M.newID()
- return {timestamp(), math.random(0, MAX_ID_PART2), math.random(0, MAX_ID_PART3)}
+local newID = function()
+ return timestamp() .. '.' .. math.random(0, MAX_ID_PART2) .. '.' .. math.random(0, MAX_ID_PART3)
end
--- Format a trace/segment id into an array.
--- An official ID should have three parts separated by '.' and each part of it is a number
-function _M.formatID(str)
- local regex = '.'
- if _M.is_ngx_lua then
- regex = [[\.]]
- end
- local parts = split(str, regex)
- if #parts ~= 3 then
- return nil
- end
+math.randomseed(random_seed())
- return parts
+-- for Nginx Lua
+local ok, uuid = pcall(require, "dependencies/lua-resty-jit-uuid")
Review comment:
@moonming I have resolved this issue, please help to recheck.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on issue #30: Adapt the
new v3 protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#issuecomment-610199184
> one more thing, is it works with the current version of skywalking? Or we should merge this PR after skywalking 8.0 released?
It isn't compatible with old SkyWalking release.
@mrproliu please add some documentation in this section, https://github.com/apache/skywalking-nginx-lua#download, such as
- 0.1.0 release requires SkyWalking 7
- 0.2.0+ releases require SkyWalking 8
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] membphis commented on a change in pull
request #30: Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#discussion_r404529581
##########
File path: lib/skywalking/management.lua
##########
@@ -0,0 +1,46 @@
+--
+-- 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 _M = {}
+
+-- Return Services as service register parameter
+function _M.newReportInstanceProperties(serviceName, serviceInstance)
+ local instanceProperties = {
+ service = serviceName,
+ serviceInstance = serviceInstance,
+ properties = {}
Review comment:
`properties = {allProperties}` is enough
then we can remove this line: https://github.com/apache/skywalking-nginx-lua/pull/30/files#diff-44e9a9b4c4ff080f804374ffdb1930acR32
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] mrproliu removed a comment on issue #30:
Adapt the new v3 protocol
Posted by GitBox <gi...@apache.org>.
mrproliu removed a comment on issue #30: Adapt the new v3 protocol
URL: https://github.com/apache/skywalking-nginx-lua/pull/30#issuecomment-609684500
/run ci
----------------------------------------------------------------
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
With regards,
Apache Git Services