[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull request #42: add error check, status code great or equal than 500 would be in erro…

2020-09-28 Thread GitBox 


wu-sheng commented on a change in pull request #42:
URL: 
https://github.com/apache/skywalking-nginx-lua/pull/42#discussion_r495822580



##
File path: lib/skywalking/tracer.lua
##
@@ -77,7 +81,11 @@ function Tracer:prepareForReport()
 local TC = require('tracing_context')
 local Segment = require('segment')
 if ngx.ctx.entrySpan ~= nil then
-Span.tag(ngx.ctx.entrySpan, 'http.status', ngx.var.status)
+local ngxstatus = ngx.var.status
+Span.tag(ngx.ctx.entrySpan, 'http.status', ngxstatus)
+if tonumber(ngxstatus) >= 500 then
+   Span.errorOccurred(ngx.ctx.entrySpan)
+end

Review comment:
   Why do this again in the reporting stage? I think all logic should be 
included in `Tracer:finish`. Could you explain?





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:
us...@infra.apache.org

[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull request #42: add error check, status code great or equal than 500 would be in erro…

2020-09-26 Thread GitBox 


wu-sheng commented on a change in pull request #42:
URL: 
https://github.com/apache/skywalking-nginx-lua/pull/42#discussion_r495518659



##
File path: lib/skywalking/tracer.lua
##
@@ -68,6 +68,10 @@ end
 function Tracer:finish()
 -- Finish the exit span when received the first response package from 
upstream
 if ngx.ctx.exitSpan ~= nil then
+local upstream_status = tonumber(ngx.var.upstream_status)
+if upstream_status and upstream_status >= 500 then
+   Span.errorOccurred(ngx.ctx.exitSpan)

Review comment:
   @Jijun 





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:
us...@infra.apache.org

[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull request #42: add error check, status code great or equal than 500 would be in erro…

2020-09-26 Thread GitBox 


wu-sheng commented on a change in pull request #42:
URL: 
https://github.com/apache/skywalking-nginx-lua/pull/42#discussion_r495518643



##
File path: lib/skywalking/tracer.lua
##
@@ -68,6 +68,10 @@ end
 function Tracer:finish()
 -- Finish the exit span when received the first response package from 
upstream
 if ngx.ctx.exitSpan ~= nil then
+local upstream_status = tonumber(ngx.var.upstream_status)
+if upstream_status and upstream_status >= 500 then
+   Span.errorOccurred(ngx.ctx.exitSpan)

Review comment:
   According to the discussion, I suggest you add `status_code` tag here.





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:
us...@infra.apache.org