Re: [PR] fix lost trace when multi middleware handlerFunc [skywalking-go]

2024-02-06 Thread via GitHub


mrproliu merged PR #167:
URL: https://github.com/apache/skywalking-go/pull/167


-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] fix lost trace when multi middleware handlerFunc [skywalking-go]

2024-02-05 Thread via GitHub


mrproliu commented on code in PR #167:
URL: https://github.com/apache/skywalking-go/pull/167#discussion_r1479262761


##
CHANGES.md:
##
@@ -22,6 +22,7 @@ Release Notes.
 * Fix enhance method error when unknown parameter type.
 * Fix wrong tracing context when trace have been sampled.
 * Fix enhance param error when there are multiple params.
+* Fix lost trace when multi middleware handlerFunc.

Review Comment:
   ```suggestion
   * Fix lost trace when multi middleware `handlerFunc` in `gin` plugin.
   ```



-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] fix lost trace when multi middleware handlerFunc [skywalking-go]

2024-02-05 Thread via GitHub


mrproliu commented on PR #167:
URL: https://github.com/apache/skywalking-go/pull/167#issuecomment-1928825979

   Please update the 
[CHANGES.md](https://github.com/apache/skywalking-go/blob/main/CHANGES.md), 
thanks.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] fix lost trace when multi middleware handlerFunc [skywalking-go]

2024-02-05 Thread via GitHub


Ruff-nono commented on code in PR #167:
URL: https://github.com/apache/skywalking-go/pull/167#discussion_r1479161253


##
plugins/gin/intercepter.go:
##
@@ -59,3 +62,10 @@ func (h *ContextInterceptor) AfterInvoke(invocation 
operator.Invocation, result
span.End()
return nil
 }
+
+func isFirstHandle(c interface{}) bool {
+   if context, ok := c.(*nativeContext); ok {
+   return context.index < 0

Review Comment:
   ok



-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] fix lost trace when multi middleware handlerFunc [skywalking-go]

2024-02-05 Thread via GitHub


mrproliu commented on code in PR #167:
URL: https://github.com/apache/skywalking-go/pull/167#discussion_r1479149579


##
plugins/gin/intercepter.go:
##
@@ -59,3 +62,10 @@ func (h *ContextInterceptor) AfterInvoke(invocation 
operator.Invocation, result
span.End()
return nil
 }
+
+func isFirstHandle(c interface{}) bool {
+   if context, ok := c.(*nativeContext); ok {
+   return context.index < 0

Review Comment:
   Then, could you help to add a comment to explain this mechanism in the code?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] fix lost trace when multi middleware handlerFunc [skywalking-go]

2024-02-05 Thread via GitHub


Ruff-nono commented on code in PR #167:
URL: https://github.com/apache/skywalking-go/pull/167#discussion_r1479140895


##
plugins/gin/intercepter.go:
##
@@ -59,3 +62,10 @@ func (h *ContextInterceptor) AfterInvoke(invocation 
operator.Invocation, result
span.End()
return nil
 }
+
+func isFirstHandle(c interface{}) bool {
+   if context, ok := c.(*nativeContext); ok {
+   return context.index < 0

Review Comment:
   In fact, the index is the index of HandlersChain,  it is reset to -1 at the 
beginning of each request, and then incremented and executes the 
handlers[index] in the `#Next()`.
   ```
   func (c *Context) Next() {
c.index++
for c.index < int8(len(c.handlers)) {
c.handlers[c.index](c)
c.index++
}
   }
   ```



-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] fix lost trace when multi middleware handlerFunc [skywalking-go]

2024-02-05 Thread via GitHub


mrproliu commented on code in PR #167:
URL: https://github.com/apache/skywalking-go/pull/167#discussion_r1478104412


##
plugins/gin/intercepter.go:
##
@@ -59,3 +62,10 @@ func (h *ContextInterceptor) AfterInvoke(invocation 
operator.Invocation, result
span.End()
return nil
 }
+
+func isFirstHandle(c interface{}) bool {
+   if context, ok := c.(*nativeContext); ok {
+   return context.index < 0

Review Comment:
   Following your [diagram in the 
comment](https://github.com/apache/skywalking/issues/11852#issuecomment-1918727190),
 I think It should equal zero? Please correct me if am wrong. 



-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] fix lost trace when multi middleware handlerFunc [skywalking-go]

2024-01-31 Thread via GitHub


wu-sheng commented on PR #167:
URL: https://github.com/apache/skywalking-go/pull/167#issuecomment-1918927495

   Let's wait for project lead back from vacation. It should be next Monday.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org