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/02/29 14:32:03 UTC
[GitHub] [skywalking-nginx-lua] moonming opened a new pull request #10: used
the nginx lua code style.
moonming opened a new pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r386102983
##########
File path: lib/skywalking/tracing_context_test.lua
##########
@@ -30,24 +31,24 @@ TestTracingContext = {}
end
function TestTracingContext:testInternal_NextSpanSeqID()
- local context = TC:new(1, 1)
+ local context = TC.new(1, 1)
- lu.assertEquals(context.internal:nextSpanID(), 0)
+ lu.assertEquals(TC_Internal.nextSpanID(context.internal), 0)
end
function TestTracingContext:testInternal_addActive()
- local context = TC:new(1, 1)
+ local context = TC.new(1, 1)
local mockSpan = {span_id = 0}
- context.internal:addActive(mockSpan)
+ TC_Internal.addActive(context.internal, mockSpan)
- lu.assertEquals(#(context.internal.active_spans), 1)
+ lu.assertEquals(#context.internal.active_spans, 1)
end
function TestTracingContext:testSpanStack()
- local context = TC:new(1, 1)
- local span1 = context:createEntrySpan('entry_op')
- local span2 = context:createExitSpan("exit_op", span1, "127.0.0.1")
+ local context = TC.new(1, 1)
+ local span1 = TC.createEntrySpan(context, 'entry_op')
+ local span2 = TC.createExitSpan(context, "exit_op", span1, "127.0.0.1")
Review comment:
> There is no class in Lua, which needs to be simulated by table. Therefore, in nginx Lua, object-oriented programming mode is generally not recommended
I am aware of this, but if refactoring all codes to out of OO, it will be hard to across check the the first-class java agent.
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r387713629
##########
File path: lib/skywalking/span.lua
##########
@@ -120,210 +121,202 @@ function Span:createExitSpan(operationName, context, parent, peer, contextCarrie
end
entryServiceInstanceId = context.service_inst_id
end
-
+
injectableRef.entry_service_instance_id = entryServiceInstanceId
injectableRef.parent_service_instance_id = context.service_inst_id
injectableRef.entry_endpoint_name = entryEndpointName
injectableRef.entry_endpoint_id = entryEndpointId
local parentEndpointName
local parentEndpointId = -1
-
+
if firstSpan.is_entry then
parentEndpointName = firstSpan.operation_name
parentEndpointId = 0
end
injectableRef.parent_endpoint_name = parentEndpointName
injectableRef.parent_endpoint_id = parentEndpointId
- contextCarrier[CONTEXT_CARRIER_KEY] = injectableRef:serialize()
+ contextCarrier[CONTEXT_CARRIER_KEY] = SegmentRef.serialize(injectableRef)
end
return span
end
--- Create an local span. Local span is usually not used.
+-- Create an local span. Local span is usually not used.
-- Typically, only one entry span and one exit span in the Nginx tracing segment.
-function Span:createLocalSpan(operationName, context, parent)
- local span = self:new(operationName, context, parent)
+function _M.createLocalSpan(operationName, context, parent)
+ local span = _M.new(operationName, context, parent)
return span
end
-- Create a default span.
-- Usually, this method wouldn't be called by outside directly.
-- Read newEntrySpan, newExitSpan and newLocalSpan for more details
-function Span:new(operationName, context, parent)
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- o.operation_name = operationName
- o.span_id = context.internal:nextSpanID()
-
+function _M.new(operationName, context, parent)
+ local span = _M.newNoOP()
+ span.is_noop = false
+
+ span.operation_name = operationName
+ span.span_id = context.internal.nextSpanID(context.internal)
+
if parent == nil then
-- As the root span, the parent span id is -1
- o.parent_span_id = -1
+ span.parent_span_id = -1
else
- o.parent_span_id = parent.span_id
- end
-
- context.internal:addActive(o)
- -- o.start_time = Util.timestamp()
- o.refs = {}
- o.owner = context
-
- return o
-end
+ span.parent_span_id = parent.span_id
+ end
-function Span:newNoOP()
- local o = {}
- setmetatable(o, self)
- self.__index = self
+ context.internal.addActive(context.internal, span)
+ -- span.start_time = Util.timestamp()
Review comment:
Remove it, please.
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r386033285
##########
File path: lib/skywalking/tracing_context_test.lua
##########
@@ -30,24 +31,24 @@ TestTracingContext = {}
end
function TestTracingContext:testInternal_NextSpanSeqID()
- local context = TC:new(1, 1)
+ local context = TC.new(1, 1)
- lu.assertEquals(context.internal:nextSpanID(), 0)
+ lu.assertEquals(TC_Internal.nextSpanID(context.internal), 0)
end
function TestTracingContext:testInternal_addActive()
- local context = TC:new(1, 1)
+ local context = TC.new(1, 1)
local mockSpan = {span_id = 0}
- context.internal:addActive(mockSpan)
+ TC_Internal.addActive(context.internal, mockSpan)
- lu.assertEquals(#(context.internal.active_spans), 1)
+ lu.assertEquals(#context.internal.active_spans, 1)
end
function TestTracingContext:testSpanStack()
- local context = TC:new(1, 1)
- local span1 = context:createEntrySpan('entry_op')
- local span2 = context:createExitSpan("exit_op", span1, "127.0.0.1")
+ local context = TC.new(1, 1)
+ local span1 = TC.createEntrySpan(context, 'entry_op')
+ local span2 = TC.createExitSpan(context, "exit_op", span1, "127.0.0.1")
Review comment:
Why use this code style? In the tracing concept, there should be one context, which hosts all inform to connect spans.
I put the codes in the old way in order to keep the internal context in the shadow, and open limited APIs.
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r387531738
##########
File path: lib/skywalking/span.lua
##########
@@ -120,210 +121,202 @@ function Span:createExitSpan(operationName, context, parent, peer, contextCarrie
end
entryServiceInstanceId = context.service_inst_id
end
-
+
injectableRef.entry_service_instance_id = entryServiceInstanceId
injectableRef.parent_service_instance_id = context.service_inst_id
injectableRef.entry_endpoint_name = entryEndpointName
injectableRef.entry_endpoint_id = entryEndpointId
local parentEndpointName
local parentEndpointId = -1
-
+
if firstSpan.is_entry then
parentEndpointName = firstSpan.operation_name
parentEndpointId = 0
end
injectableRef.parent_endpoint_name = parentEndpointName
injectableRef.parent_endpoint_id = parentEndpointId
- contextCarrier[CONTEXT_CARRIER_KEY] = injectableRef:serialize()
+ contextCarrier[CONTEXT_CARRIER_KEY] = SegmentRef.serialize(injectableRef)
end
return span
end
--- Create an local span. Local span is usually not used.
+-- Create an local span. Local span is usually not used.
-- Typically, only one entry span and one exit span in the Nginx tracing segment.
-function Span:createLocalSpan(operationName, context, parent)
- local span = self:new(operationName, context, parent)
+function _M.createLocalSpan(operationName, context, parent)
+ local span = _M.new(operationName, context, parent)
return span
end
-- Create a default span.
-- Usually, this method wouldn't be called by outside directly.
-- Read newEntrySpan, newExitSpan and newLocalSpan for more details
-function Span:new(operationName, context, parent)
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- o.operation_name = operationName
- o.span_id = context.internal:nextSpanID()
-
+function _M.new(operationName, context, parent)
+ local span = _M.newNoOP()
+ span.is_noop = false
+
+ span.operation_name = operationName
+ span.span_id = context.internal.nextSpanID(context.internal)
+
if parent == nil then
-- As the root span, the parent span id is -1
- o.parent_span_id = -1
+ span.parent_span_id = -1
else
- o.parent_span_id = parent.span_id
- end
-
- context.internal:addActive(o)
- -- o.start_time = Util.timestamp()
- o.refs = {}
- o.owner = context
-
- return o
-end
+ span.parent_span_id = parent.span_id
+ end
-function Span:newNoOP()
- local o = {}
- setmetatable(o, self)
- self.__index = self
+ context.internal.addActive(context.internal, span)
+ -- span.start_time = Util.timestamp()
+ span.refs = {}
+ span.owner = context
- o.is_noop = true
- return o
+ return span
end
-function SpanProtocol:new()
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- return o
+function _M.newNoOP()
+ return {
+ layer = spanLayer.NONE,
+ is_entry = false,
+ is_exit = false,
+ error_occurred = false,
+ is_noop = true
Review comment:
Why new op span needs these values?
----------------------------------------------------------------
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 issue #10: used the
nginx lua code style.
Posted by GitBox <gi...@apache.org>.
membphis commented on issue #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#issuecomment-593184138
We can use `luacheck` to do some static code checking.
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r386101595
##########
File path: lib/skywalking/tracing_context_test.lua
##########
@@ -30,24 +31,24 @@ TestTracingContext = {}
end
function TestTracingContext:testInternal_NextSpanSeqID()
- local context = TC:new(1, 1)
+ local context = TC.new(1, 1)
- lu.assertEquals(context.internal:nextSpanID(), 0)
+ lu.assertEquals(TC_Internal.nextSpanID(context.internal), 0)
end
function TestTracingContext:testInternal_addActive()
- local context = TC:new(1, 1)
+ local context = TC.new(1, 1)
local mockSpan = {span_id = 0}
- context.internal:addActive(mockSpan)
+ TC_Internal.addActive(context.internal, mockSpan)
- lu.assertEquals(#(context.internal.active_spans), 1)
+ lu.assertEquals(#context.internal.active_spans, 1)
end
function TestTracingContext:testSpanStack()
- local context = TC:new(1, 1)
- local span1 = context:createEntrySpan('entry_op')
- local span2 = context:createExitSpan("exit_op", span1, "127.0.0.1")
+ local context = TC.new(1, 1)
+ local span1 = TC.createEntrySpan(context, 'entry_op')
+ local span2 = TC.createExitSpan(context, "exit_op", span1, "127.0.0.1")
Review comment:
got it. I will hide the internal context.
There are two reasons for this code style:
- In nginx lua, top level variables are shared by all requests in a worker, so top level variables are easy to race condition, so it needs to be read-only
- There is no class in Lua, which needs to be simulated by table. Therefore, in nginx Lua, object-oriented programming mode is generally not recommended
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r387030302
##########
File path: lib/skywalking/segment_ref.lua
##########
@@ -16,142 +16,147 @@
--
local Util = require('util')
local Base64 = require('dependencies/base64')
+local encode_base64 = Base64.encode
+local decode_base64 = Base64.decode
-local SegmentRef = {
- -- There is no multiple-threads scenario in the LUA, no only hard coded as CROSS_PROCESS
- type = 'CROSS_PROCESS',
- 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,
-}
-
--- Due to nesting relationship inside Segment/Span/TracingContext at the runtime,
--- RefProtocol is created to prepare JSON format serialization.
--- Following SkyWalking official trace protocol v2
--- https://github.com/apache/skywalking-data-collect-protocol/blob/master/language-agent-v2/trace.proto
-local RefProtocol = {
- -- Constant in LUA, no cross-thread
- refType = 'CrossProcess',
- parentTraceSegmentId,
- parentSpanId,
- parentServiceInstanceId,
- networkAddress,
- networkAddressId,
- entryServiceInstanceId,
- entryEndpoint,
- entryEndpointId,
- parentEndpoint,
- parentEndpointId,
-}
-
-function SegmentRef:new()
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- return o
+if Util.is_ngx_lua then
+ encode_base64 = ngx.encode_base64
+ decode_base64 = ngx.decode_base64
end
-function RefProtocol:new()
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- return o
+local _M = {}
+-- local SegmentRef = {
+-- -- There is no multiple-threads scenario in the LUA, no only hard coded as CROSS_PROCESS
+-- type = 'CROSS_PROCESS',
+-- 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,
+-- }
+
+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 SegmentRef:fromSW6Value(value)
+function _M.fromSW6Value(value)
+ local ref = _M.new()
+
local parts = Util.split(value, '-')
if #parts ~= 9 then
return nil
end
- self.trace_id = Util.formatID(Base64.decode(parts[2]))
- self.segment_id = Util.formatID(Base64.decode(parts[3]))
- self.span_id = tonumber(parts[4])
- self.parent_service_instance_id = tonumber(parts[5])
- self.entry_service_instance_id = tonumber(parts[6])
- local peerStr = Base64.decode(parts[7])
+ ref.trace_id = Util.formatID(decode_base64(parts[2]))
+ ref.segment_id = Util.formatID(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
- self.network_address = string.sub(peerStr, 2)
+ ref.network_address = string.sub(peerStr, 2)
else
- self.network_address_id = tonumber(peerStr)
+ ref.network_address_id = tonumber(peerStr)
end
- local entryEndpointStr = Base64.decode(parts[8])
+ local entryEndpointStr = decode_base64(parts[8])
if string.sub(entryEndpointStr, 1, 1) == '#' then
- self.entry_endpoint_name = string.sub(entryEndpointStr, 2)
+ ref.entry_endpoint_name = string.sub(entryEndpointStr, 2)
else
- self.entry_endpoint_id = tonumber(entryEndpointStr)
+ ref.entry_endpoint_id = tonumber(entryEndpointStr)
end
- local parentEndpointStr = Base64.decode(parts[9])
+ local parentEndpointStr = decode_base64(parts[9])
if string.sub(parentEndpointStr, 1, 1) == '#' then
- self.parent_endpoint_name = string.sub(parentEndpointStr, 2)
+ ref.parent_endpoint_name = string.sub(parentEndpointStr, 2)
else
- self.parent_endpoint_id = tonumber(parentEndpointStr)
+ ref.parent_endpoint_id = tonumber(parentEndpointStr)
end
- return self
+ return ref
end
-- Return string to represent this ref.
-function SegmentRef:serialize()
+function _M.serialize(ref)
local encodedRef = '1'
- encodedRef = encodedRef .. '-' .. Base64.encode(Util.id2String(self.trace_id))
- encodedRef = encodedRef .. '-' .. Base64.encode(Util.id2String(self.segment_id))
- encodedRef = encodedRef .. '-' .. self.span_id
- encodedRef = encodedRef .. '-' .. self.parent_service_instance_id
- encodedRef = encodedRef .. '-' .. self.entry_service_instance_id
+ encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.trace_id))
+ encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(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 self.network_address_id ~= 0 then
- networkAddress = self.network_address_id .. ''
+ if ref.network_address_id ~= 0 then
+ networkAddress = ref.network_address_id .. ''
else
- networkAddress = '#' .. self.network_address
+ networkAddress = '#' .. ref.network_address
end
- encodedRef = encodedRef .. '-' .. Base64.encode(networkAddress)
+ encodedRef = encodedRef .. '-' .. encode_base64(networkAddress)
local entryEndpoint
- if self.entry_endpoint_id ~= 0 then
- entryEndpoint = self.entry_endpoint_id .. ''
+ if ref.entry_endpoint_id ~= 0 then
+ entryEndpoint = ref.entry_endpoint_id .. ''
else
- entryEndpoint = '#' .. self.entry_endpoint_name
+ entryEndpoint = '#' .. ref.entry_endpoint_name
end
- encodedRef = encodedRef .. '-' .. Base64.encode(entryEndpoint)
+ encodedRef = encodedRef .. '-' .. encode_base64(entryEndpoint)
local parentEndpoint
- if self.parent_endpoint_id ~= 0 then
- parentEndpoint = self.parent_endpoint_id .. ''
+ if ref.parent_endpoint_id ~= 0 then
+ parentEndpoint = ref.parent_endpoint_id .. ''
else
- parentEndpoint = '#' .. self.parent_endpoint_name
+ parentEndpoint = '#' .. ref.parent_endpoint_name
end
- encodedRef = encodedRef .. '-' .. Base64.encode(parentEndpoint)
+ encodedRef = encodedRef .. '-' .. encode_base64(parentEndpoint)
return encodedRef
end
+-- Due to nesting relationship inside Segment/Span/TracingContext at the runtime,
+-- RefProtocol is created to prepare JSON format serialization.
+-- Following SkyWalking official trace protocol v2
+-- https://github.com/apache/skywalking-data-collect-protocol/blob/master/language-agent-v2/trace.proto
+-- local RefProtocol = {
+-- -- Constant in LUA, no cross-thread
+-- refType = 'CrossProcess',
+-- parentTraceSegmentId,
+-- parentSpanId,
+-- parentServiceInstanceId,
+-- networkAddress,
+-- networkAddressId,
+-- entryServiceInstanceId,
+-- entryEndpoint,
+-- entryEndpointId,
+-- parentEndpoint,
+-- parentEndpointId,
+-- }
-- Return RefProtocol
-function SegmentRef:transform()
- local refBuilder = RefProtocol:new()
- refBuilder.parentTraceSegmentId = {idParts = self.segment_id }
- refBuilder.parentSpanId = self.span_id
- refBuilder.parentServiceInstanceId = self.parent_service_instance_id
- refBuilder.networkAddress = self.network_address
- refBuilder.networkAddressId = self.network_address_id
- refBuilder.entryServiceInstanceId = self.entry_service_instance_id
- refBuilder.entryEndpoint = self.entry_endpoint_name
- refBuilder.entryEndpointId = self.entry_endpoint_id
- refBuilder.parentEndpoint = self.parent_endpoint_name
- refBuilder.parentEndpointId = self.parent_endpoint_id
+function _M.transform(ref)
+ local refBuilder = {}
Review comment:
this style is better, better performance.
```lua
return {
refBuilder.refType = 'CrossProcess',
... ...
}
```
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r387713539
##########
File path: lib/skywalking/span.lua
##########
@@ -120,210 +121,202 @@ function Span:createExitSpan(operationName, context, parent, peer, contextCarrie
end
entryServiceInstanceId = context.service_inst_id
end
-
+
injectableRef.entry_service_instance_id = entryServiceInstanceId
injectableRef.parent_service_instance_id = context.service_inst_id
injectableRef.entry_endpoint_name = entryEndpointName
injectableRef.entry_endpoint_id = entryEndpointId
local parentEndpointName
local parentEndpointId = -1
-
+
if firstSpan.is_entry then
parentEndpointName = firstSpan.operation_name
parentEndpointId = 0
end
injectableRef.parent_endpoint_name = parentEndpointName
injectableRef.parent_endpoint_id = parentEndpointId
- contextCarrier[CONTEXT_CARRIER_KEY] = injectableRef:serialize()
+ contextCarrier[CONTEXT_CARRIER_KEY] = SegmentRef.serialize(injectableRef)
end
return span
end
--- Create an local span. Local span is usually not used.
+-- Create an local span. Local span is usually not used.
-- Typically, only one entry span and one exit span in the Nginx tracing segment.
-function Span:createLocalSpan(operationName, context, parent)
- local span = self:new(operationName, context, parent)
+function _M.createLocalSpan(operationName, context, parent)
+ local span = _M.new(operationName, context, parent)
return span
end
-- Create a default span.
-- Usually, this method wouldn't be called by outside directly.
-- Read newEntrySpan, newExitSpan and newLocalSpan for more details
-function Span:new(operationName, context, parent)
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- o.operation_name = operationName
- o.span_id = context.internal:nextSpanID()
-
+function _M.new(operationName, context, parent)
+ local span = _M.newNoOP()
+ span.is_noop = false
+
+ span.operation_name = operationName
+ span.span_id = context.internal.nextSpanID(context.internal)
+
if parent == nil then
-- As the root span, the parent span id is -1
- o.parent_span_id = -1
+ span.parent_span_id = -1
else
- o.parent_span_id = parent.span_id
- end
-
- context.internal:addActive(o)
- -- o.start_time = Util.timestamp()
- o.refs = {}
- o.owner = context
-
- return o
-end
+ span.parent_span_id = parent.span_id
+ end
-function Span:newNoOP()
- local o = {}
- setmetatable(o, self)
- self.__index = self
+ context.internal.addActive(context.internal, span)
+ -- span.start_time = Util.timestamp()
+ span.refs = {}
+ span.owner = context
- o.is_noop = true
- return o
+ return span
end
-function SpanProtocol:new()
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- return o
+function _M.newNoOP()
+ return {
+ layer = spanLayer.NONE,
+ is_entry = false,
+ is_exit = false,
+ error_occurred = false,
+ is_noop = true
Review comment:
All methods of span have this,
```lua
if self.is_noop then
return self
end
```
But, I notice, there is an error in code, in `TracingContext:drainAfterFinished`, if tracing context is no OP, then this should change to this.
```
if self.is_noop then
return false, nil
end
```
Because as a no op trace context, there is no need to enqueue.
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r387414156
##########
File path: lib/skywalking/segment_ref.lua
##########
@@ -16,142 +16,147 @@
--
local Util = require('util')
local Base64 = require('dependencies/base64')
+local encode_base64 = Base64.encode
+local decode_base64 = Base64.decode
-local SegmentRef = {
- -- There is no multiple-threads scenario in the LUA, no only hard coded as CROSS_PROCESS
- type = 'CROSS_PROCESS',
- 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,
-}
-
--- Due to nesting relationship inside Segment/Span/TracingContext at the runtime,
--- RefProtocol is created to prepare JSON format serialization.
--- Following SkyWalking official trace protocol v2
--- https://github.com/apache/skywalking-data-collect-protocol/blob/master/language-agent-v2/trace.proto
-local RefProtocol = {
- -- Constant in LUA, no cross-thread
- refType = 'CrossProcess',
- parentTraceSegmentId,
- parentSpanId,
- parentServiceInstanceId,
- networkAddress,
- networkAddressId,
- entryServiceInstanceId,
- entryEndpoint,
- entryEndpointId,
- parentEndpoint,
- parentEndpointId,
-}
-
-function SegmentRef:new()
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- return o
+if Util.is_ngx_lua then
+ encode_base64 = ngx.encode_base64
+ decode_base64 = ngx.decode_base64
end
-function RefProtocol:new()
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- return o
+local _M = {}
+-- local SegmentRef = {
+-- -- There is no multiple-threads scenario in the LUA, no only hard coded as CROSS_PROCESS
+-- type = 'CROSS_PROCESS',
+-- 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,
+-- }
+
+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 SegmentRef:fromSW6Value(value)
+function _M.fromSW6Value(value)
+ local ref = _M.new()
+
local parts = Util.split(value, '-')
if #parts ~= 9 then
return nil
end
- self.trace_id = Util.formatID(Base64.decode(parts[2]))
- self.segment_id = Util.formatID(Base64.decode(parts[3]))
- self.span_id = tonumber(parts[4])
- self.parent_service_instance_id = tonumber(parts[5])
- self.entry_service_instance_id = tonumber(parts[6])
- local peerStr = Base64.decode(parts[7])
+ ref.trace_id = Util.formatID(decode_base64(parts[2]))
+ ref.segment_id = Util.formatID(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
- self.network_address = string.sub(peerStr, 2)
+ ref.network_address = string.sub(peerStr, 2)
else
- self.network_address_id = tonumber(peerStr)
+ ref.network_address_id = tonumber(peerStr)
end
- local entryEndpointStr = Base64.decode(parts[8])
+ local entryEndpointStr = decode_base64(parts[8])
if string.sub(entryEndpointStr, 1, 1) == '#' then
- self.entry_endpoint_name = string.sub(entryEndpointStr, 2)
+ ref.entry_endpoint_name = string.sub(entryEndpointStr, 2)
else
- self.entry_endpoint_id = tonumber(entryEndpointStr)
+ ref.entry_endpoint_id = tonumber(entryEndpointStr)
end
- local parentEndpointStr = Base64.decode(parts[9])
+ local parentEndpointStr = decode_base64(parts[9])
if string.sub(parentEndpointStr, 1, 1) == '#' then
- self.parent_endpoint_name = string.sub(parentEndpointStr, 2)
+ ref.parent_endpoint_name = string.sub(parentEndpointStr, 2)
else
- self.parent_endpoint_id = tonumber(parentEndpointStr)
+ ref.parent_endpoint_id = tonumber(parentEndpointStr)
end
- return self
+ return ref
end
-- Return string to represent this ref.
-function SegmentRef:serialize()
+function _M.serialize(ref)
local encodedRef = '1'
- encodedRef = encodedRef .. '-' .. Base64.encode(Util.id2String(self.trace_id))
- encodedRef = encodedRef .. '-' .. Base64.encode(Util.id2String(self.segment_id))
- encodedRef = encodedRef .. '-' .. self.span_id
- encodedRef = encodedRef .. '-' .. self.parent_service_instance_id
- encodedRef = encodedRef .. '-' .. self.entry_service_instance_id
+ encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.trace_id))
Review comment:
Got it. But I think we can deal with performance issues later.
We keep the same structure as other language agents to find bugs more easily now.
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r387031063
##########
File path: lib/skywalking/segment_ref.lua
##########
@@ -16,142 +16,147 @@
--
local Util = require('util')
local Base64 = require('dependencies/base64')
+local encode_base64 = Base64.encode
+local decode_base64 = Base64.decode
-local SegmentRef = {
- -- There is no multiple-threads scenario in the LUA, no only hard coded as CROSS_PROCESS
- type = 'CROSS_PROCESS',
- 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,
-}
-
--- Due to nesting relationship inside Segment/Span/TracingContext at the runtime,
--- RefProtocol is created to prepare JSON format serialization.
--- Following SkyWalking official trace protocol v2
--- https://github.com/apache/skywalking-data-collect-protocol/blob/master/language-agent-v2/trace.proto
-local RefProtocol = {
- -- Constant in LUA, no cross-thread
- refType = 'CrossProcess',
- parentTraceSegmentId,
- parentSpanId,
- parentServiceInstanceId,
- networkAddress,
- networkAddressId,
- entryServiceInstanceId,
- entryEndpoint,
- entryEndpointId,
- parentEndpoint,
- parentEndpointId,
-}
-
-function SegmentRef:new()
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- return o
+if Util.is_ngx_lua then
+ encode_base64 = ngx.encode_base64
+ decode_base64 = ngx.decode_base64
end
-function RefProtocol:new()
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- return o
+local _M = {}
+-- local SegmentRef = {
+-- -- There is no multiple-threads scenario in the LUA, no only hard coded as CROSS_PROCESS
+-- type = 'CROSS_PROCESS',
+-- 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,
+-- }
+
+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 SegmentRef:fromSW6Value(value)
+function _M.fromSW6Value(value)
+ local ref = _M.new()
+
local parts = Util.split(value, '-')
if #parts ~= 9 then
return nil
end
- self.trace_id = Util.formatID(Base64.decode(parts[2]))
- self.segment_id = Util.formatID(Base64.decode(parts[3]))
- self.span_id = tonumber(parts[4])
- self.parent_service_instance_id = tonumber(parts[5])
- self.entry_service_instance_id = tonumber(parts[6])
- local peerStr = Base64.decode(parts[7])
+ ref.trace_id = Util.formatID(decode_base64(parts[2]))
+ ref.segment_id = Util.formatID(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
- self.network_address = string.sub(peerStr, 2)
+ ref.network_address = string.sub(peerStr, 2)
else
- self.network_address_id = tonumber(peerStr)
+ ref.network_address_id = tonumber(peerStr)
end
- local entryEndpointStr = Base64.decode(parts[8])
+ local entryEndpointStr = decode_base64(parts[8])
if string.sub(entryEndpointStr, 1, 1) == '#' then
- self.entry_endpoint_name = string.sub(entryEndpointStr, 2)
+ ref.entry_endpoint_name = string.sub(entryEndpointStr, 2)
else
- self.entry_endpoint_id = tonumber(entryEndpointStr)
+ ref.entry_endpoint_id = tonumber(entryEndpointStr)
end
- local parentEndpointStr = Base64.decode(parts[9])
+ local parentEndpointStr = decode_base64(parts[9])
if string.sub(parentEndpointStr, 1, 1) == '#' then
- self.parent_endpoint_name = string.sub(parentEndpointStr, 2)
+ ref.parent_endpoint_name = string.sub(parentEndpointStr, 2)
else
- self.parent_endpoint_id = tonumber(parentEndpointStr)
+ ref.parent_endpoint_id = tonumber(parentEndpointStr)
end
- return self
+ return ref
end
-- Return string to represent this ref.
-function SegmentRef:serialize()
+function _M.serialize(ref)
local encodedRef = '1'
- encodedRef = encodedRef .. '-' .. Base64.encode(Util.id2String(self.trace_id))
- encodedRef = encodedRef .. '-' .. Base64.encode(Util.id2String(self.segment_id))
- encodedRef = encodedRef .. '-' .. self.span_id
- encodedRef = encodedRef .. '-' .. self.parent_service_instance_id
- encodedRef = encodedRef .. '-' .. self.entry_service_instance_id
+ encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.trace_id))
Review comment:
better performance.
```lua
encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.trace_id))
.. encodedRef .. '-' .. encode_base64(Util.id2String(ref.segment_id))
.. -- ...
```
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r387031063
##########
File path: lib/skywalking/segment_ref.lua
##########
@@ -16,142 +16,147 @@
--
local Util = require('util')
local Base64 = require('dependencies/base64')
+local encode_base64 = Base64.encode
+local decode_base64 = Base64.decode
-local SegmentRef = {
- -- There is no multiple-threads scenario in the LUA, no only hard coded as CROSS_PROCESS
- type = 'CROSS_PROCESS',
- 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,
-}
-
--- Due to nesting relationship inside Segment/Span/TracingContext at the runtime,
--- RefProtocol is created to prepare JSON format serialization.
--- Following SkyWalking official trace protocol v2
--- https://github.com/apache/skywalking-data-collect-protocol/blob/master/language-agent-v2/trace.proto
-local RefProtocol = {
- -- Constant in LUA, no cross-thread
- refType = 'CrossProcess',
- parentTraceSegmentId,
- parentSpanId,
- parentServiceInstanceId,
- networkAddress,
- networkAddressId,
- entryServiceInstanceId,
- entryEndpoint,
- entryEndpointId,
- parentEndpoint,
- parentEndpointId,
-}
-
-function SegmentRef:new()
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- return o
+if Util.is_ngx_lua then
+ encode_base64 = ngx.encode_base64
+ decode_base64 = ngx.decode_base64
end
-function RefProtocol:new()
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- return o
+local _M = {}
+-- local SegmentRef = {
+-- -- There is no multiple-threads scenario in the LUA, no only hard coded as CROSS_PROCESS
+-- type = 'CROSS_PROCESS',
+-- 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,
+-- }
+
+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 SegmentRef:fromSW6Value(value)
+function _M.fromSW6Value(value)
+ local ref = _M.new()
+
local parts = Util.split(value, '-')
if #parts ~= 9 then
return nil
end
- self.trace_id = Util.formatID(Base64.decode(parts[2]))
- self.segment_id = Util.formatID(Base64.decode(parts[3]))
- self.span_id = tonumber(parts[4])
- self.parent_service_instance_id = tonumber(parts[5])
- self.entry_service_instance_id = tonumber(parts[6])
- local peerStr = Base64.decode(parts[7])
+ ref.trace_id = Util.formatID(decode_base64(parts[2]))
+ ref.segment_id = Util.formatID(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
- self.network_address = string.sub(peerStr, 2)
+ ref.network_address = string.sub(peerStr, 2)
else
- self.network_address_id = tonumber(peerStr)
+ ref.network_address_id = tonumber(peerStr)
end
- local entryEndpointStr = Base64.decode(parts[8])
+ local entryEndpointStr = decode_base64(parts[8])
if string.sub(entryEndpointStr, 1, 1) == '#' then
- self.entry_endpoint_name = string.sub(entryEndpointStr, 2)
+ ref.entry_endpoint_name = string.sub(entryEndpointStr, 2)
else
- self.entry_endpoint_id = tonumber(entryEndpointStr)
+ ref.entry_endpoint_id = tonumber(entryEndpointStr)
end
- local parentEndpointStr = Base64.decode(parts[9])
+ local parentEndpointStr = decode_base64(parts[9])
if string.sub(parentEndpointStr, 1, 1) == '#' then
- self.parent_endpoint_name = string.sub(parentEndpointStr, 2)
+ ref.parent_endpoint_name = string.sub(parentEndpointStr, 2)
else
- self.parent_endpoint_id = tonumber(parentEndpointStr)
+ ref.parent_endpoint_id = tonumber(parentEndpointStr)
end
- return self
+ return ref
end
-- Return string to represent this ref.
-function SegmentRef:serialize()
+function _M.serialize(ref)
local encodedRef = '1'
- encodedRef = encodedRef .. '-' .. Base64.encode(Util.id2String(self.trace_id))
- encodedRef = encodedRef .. '-' .. Base64.encode(Util.id2String(self.segment_id))
- encodedRef = encodedRef .. '-' .. self.span_id
- encodedRef = encodedRef .. '-' .. self.parent_service_instance_id
- encodedRef = encodedRef .. '-' .. self.entry_service_instance_id
+ encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.trace_id))
Review comment:
better performance.
```lua
encodedRef = encodedRef .. '-' .. encode_base64(Util.id2String(ref.trace_id))
.. encodedRef .. '-' .. encode_base64(Util.id2String(ref.segment_id))
.. -- ...
```
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r387707925
##########
File path: lib/skywalking/span.lua
##########
@@ -120,210 +121,202 @@ function Span:createExitSpan(operationName, context, parent, peer, contextCarrie
end
entryServiceInstanceId = context.service_inst_id
end
-
+
injectableRef.entry_service_instance_id = entryServiceInstanceId
injectableRef.parent_service_instance_id = context.service_inst_id
injectableRef.entry_endpoint_name = entryEndpointName
injectableRef.entry_endpoint_id = entryEndpointId
local parentEndpointName
local parentEndpointId = -1
-
+
if firstSpan.is_entry then
parentEndpointName = firstSpan.operation_name
parentEndpointId = 0
end
injectableRef.parent_endpoint_name = parentEndpointName
injectableRef.parent_endpoint_id = parentEndpointId
- contextCarrier[CONTEXT_CARRIER_KEY] = injectableRef:serialize()
+ contextCarrier[CONTEXT_CARRIER_KEY] = SegmentRef.serialize(injectableRef)
end
return span
end
--- Create an local span. Local span is usually not used.
+-- Create an local span. Local span is usually not used.
-- Typically, only one entry span and one exit span in the Nginx tracing segment.
-function Span:createLocalSpan(operationName, context, parent)
- local span = self:new(operationName, context, parent)
+function _M.createLocalSpan(operationName, context, parent)
+ local span = _M.new(operationName, context, parent)
return span
end
-- Create a default span.
-- Usually, this method wouldn't be called by outside directly.
-- Read newEntrySpan, newExitSpan and newLocalSpan for more details
-function Span:new(operationName, context, parent)
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- o.operation_name = operationName
- o.span_id = context.internal:nextSpanID()
-
+function _M.new(operationName, context, parent)
+ local span = _M.newNoOP()
+ span.is_noop = false
+
+ span.operation_name = operationName
+ span.span_id = context.internal.nextSpanID(context.internal)
+
if parent == nil then
-- As the root span, the parent span id is -1
- o.parent_span_id = -1
+ span.parent_span_id = -1
else
- o.parent_span_id = parent.span_id
- end
-
- context.internal:addActive(o)
- -- o.start_time = Util.timestamp()
- o.refs = {}
- o.owner = context
-
- return o
-end
+ span.parent_span_id = parent.span_id
+ end
-function Span:newNoOP()
- local o = {}
- setmetatable(o, self)
- self.__index = self
+ context.internal.addActive(context.internal, span)
+ -- span.start_time = Util.timestamp()
Review comment:
copy from https://github.com/apache/skywalking-nginx-lua/blob/master/lib/skywalking/span.lua#L171
----------------------------------------------------------------
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 #10: used the
nginx lua code style.
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r387532157
##########
File path: lib/skywalking/span.lua
##########
@@ -120,210 +121,202 @@ function Span:createExitSpan(operationName, context, parent, peer, contextCarrie
end
entryServiceInstanceId = context.service_inst_id
end
-
+
injectableRef.entry_service_instance_id = entryServiceInstanceId
injectableRef.parent_service_instance_id = context.service_inst_id
injectableRef.entry_endpoint_name = entryEndpointName
injectableRef.entry_endpoint_id = entryEndpointId
local parentEndpointName
local parentEndpointId = -1
-
+
if firstSpan.is_entry then
parentEndpointName = firstSpan.operation_name
parentEndpointId = 0
end
injectableRef.parent_endpoint_name = parentEndpointName
injectableRef.parent_endpoint_id = parentEndpointId
- contextCarrier[CONTEXT_CARRIER_KEY] = injectableRef:serialize()
+ contextCarrier[CONTEXT_CARRIER_KEY] = SegmentRef.serialize(injectableRef)
end
return span
end
--- Create an local span. Local span is usually not used.
+-- Create an local span. Local span is usually not used.
-- Typically, only one entry span and one exit span in the Nginx tracing segment.
-function Span:createLocalSpan(operationName, context, parent)
- local span = self:new(operationName, context, parent)
+function _M.createLocalSpan(operationName, context, parent)
+ local span = _M.new(operationName, context, parent)
return span
end
-- Create a default span.
-- Usually, this method wouldn't be called by outside directly.
-- Read newEntrySpan, newExitSpan and newLocalSpan for more details
-function Span:new(operationName, context, parent)
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- o.operation_name = operationName
- o.span_id = context.internal:nextSpanID()
-
+function _M.new(operationName, context, parent)
+ local span = _M.newNoOP()
+ span.is_noop = false
+
+ span.operation_name = operationName
+ span.span_id = context.internal.nextSpanID(context.internal)
+
if parent == nil then
-- As the root span, the parent span id is -1
- o.parent_span_id = -1
+ span.parent_span_id = -1
else
- o.parent_span_id = parent.span_id
- end
-
- context.internal:addActive(o)
- -- o.start_time = Util.timestamp()
- o.refs = {}
- o.owner = context
-
- return o
-end
+ span.parent_span_id = parent.span_id
+ end
-function Span:newNoOP()
- local o = {}
- setmetatable(o, self)
- self.__index = self
+ context.internal.addActive(context.internal, span)
+ -- span.start_time = Util.timestamp()
Review comment:
What is this comment for?
----------------------------------------------------------------
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 #10: used the nginx lua code style.
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #10: used the nginx lua code style.
URL: https://github.com/apache/skywalking-nginx-lua/pull/10#discussion_r387708479
##########
File path: lib/skywalking/span.lua
##########
@@ -120,210 +121,202 @@ function Span:createExitSpan(operationName, context, parent, peer, contextCarrie
end
entryServiceInstanceId = context.service_inst_id
end
-
+
injectableRef.entry_service_instance_id = entryServiceInstanceId
injectableRef.parent_service_instance_id = context.service_inst_id
injectableRef.entry_endpoint_name = entryEndpointName
injectableRef.entry_endpoint_id = entryEndpointId
local parentEndpointName
local parentEndpointId = -1
-
+
if firstSpan.is_entry then
parentEndpointName = firstSpan.operation_name
parentEndpointId = 0
end
injectableRef.parent_endpoint_name = parentEndpointName
injectableRef.parent_endpoint_id = parentEndpointId
- contextCarrier[CONTEXT_CARRIER_KEY] = injectableRef:serialize()
+ contextCarrier[CONTEXT_CARRIER_KEY] = SegmentRef.serialize(injectableRef)
end
return span
end
--- Create an local span. Local span is usually not used.
+-- Create an local span. Local span is usually not used.
-- Typically, only one entry span and one exit span in the Nginx tracing segment.
-function Span:createLocalSpan(operationName, context, parent)
- local span = self:new(operationName, context, parent)
+function _M.createLocalSpan(operationName, context, parent)
+ local span = _M.new(operationName, context, parent)
return span
end
-- Create a default span.
-- Usually, this method wouldn't be called by outside directly.
-- Read newEntrySpan, newExitSpan and newLocalSpan for more details
-function Span:new(operationName, context, parent)
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- o.operation_name = operationName
- o.span_id = context.internal:nextSpanID()
-
+function _M.new(operationName, context, parent)
+ local span = _M.newNoOP()
+ span.is_noop = false
+
+ span.operation_name = operationName
+ span.span_id = context.internal.nextSpanID(context.internal)
+
if parent == nil then
-- As the root span, the parent span id is -1
- o.parent_span_id = -1
+ span.parent_span_id = -1
else
- o.parent_span_id = parent.span_id
- end
-
- context.internal:addActive(o)
- -- o.start_time = Util.timestamp()
- o.refs = {}
- o.owner = context
-
- return o
-end
+ span.parent_span_id = parent.span_id
+ end
-function Span:newNoOP()
- local o = {}
- setmetatable(o, self)
- self.__index = self
+ context.internal.addActive(context.internal, span)
+ -- span.start_time = Util.timestamp()
+ span.refs = {}
+ span.owner = context
- o.is_noop = true
- return o
+ return span
end
-function SpanProtocol:new()
- local o = {}
- setmetatable(o, self)
- self.__index = self
-
- return o
+function _M.newNoOP()
+ return {
+ layer = spanLayer.NONE,
+ is_entry = false,
+ is_exit = false,
+ error_occurred = false,
+ is_noop = true
Review comment:
copy from https://github.com/apache/skywalking-nginx-lua/blob/master/lib/skywalking/span.lua#L24-L42.
Is a new op span needs these values?
----------------------------------------------------------------
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