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:
[email protected]


With regards,
Apache Git Services

Reply via email to