SkyAPM / go2sky

Distributed tracing and monitor SDK in Go for Apache SkyWalking APM
https://skywalking.apache.org/
Apache License 2.0
448 stars 122 forks source link

panic: send on closed channel #48

Closed heiyhia closed 4 years ago

heiyhia commented 4 years ago

go2sky-gRPC2020/03/20 15:20:53 the id of service 'libra' is 2 go2sky-gRPC2020/03/20 15:20:53 register error fail to instance service go2sky-gRPC2020/03/20 15:20:54 the id of instance '55a92af0-6a7b-11ea-aedd-c4b3018d0c5e' id is 10 go2sky-gRPC2020/03/20 15:20:54 pinging error rpc error: code = Unavailable desc = transport is closing

[GIN] 2020/03/20 - 15:23:55 | 200 | 11.69461ms | ::1 | POST /api/v1/ldapLogin panic: send on closed channel

goroutine 106 [running]: github.com/SkyAPM/go2sky/reporter.(*gRPCReporter).Send(0xc0001b4000, 0xc0002a1220, 0x1, 0xa) /Users/changsq/go/pkg/mod/github.com/!sky!a!p!m/go2sky@v0.3.0/reporter/grpc.go:269 +0x751 github.com/SkyAPM/go2sky.newSegmentRoot.func1(0xc000432960, 0xc0000c7200) /Users/changsq/go/pkg/mod/github.com/!sky!a!p!m/go2sky@v0.3.0/segment.go:237 +0x243 created by github.com/SkyAPM/go2sky.newSegmentRoot /Users/changsq/go/pkg/mod/github.com/!sky!a!p!m/go2sky@v0.3.0/segment.go:222 +0x168 exit status 2

arugal commented 4 years ago

Hi @soWill666 Thank you for the report. I think we should add automatic reconnection.

arugal commented 4 years ago

@soWill666 I have reviewed the error message you provided. The reason for the error seems to be that Reporter#Close was executed.

wu-sheng commented 4 years ago

Why reporter closed? Could you find out why?

arugal commented 4 years ago

go2sky will not close sendCh. It could be the wrong way to use it, for example:

    // Use gRPC reporter for production
    r, err := reporter.NewGRPCReporter()
    if err != nil {
        log.Fatalf("new reporter error %v \n", err)
    }
    tracer, err := go2sky.NewTracer("example", go2sky.WithReporter(r))
    if err != nil {
        log.Fatalf("create tracer error %v \n", err)
    }
    // This for test
    tracer.WaitUntilRegister()
    span, ctx, err := tracer.CreateLocalSpan(context.Background())
    if err != nil {
        log.Fatalf("create new local span error %v \n", err)
    }
        r.Close()
    subSpan.End()

Hi @soWill666 can you provide code to reproduce this error :)

hanahmily commented 4 years ago

@arugal May @soWill666 close the reporter before ending span. But I prefer we should replace the panic with a "warn" log. As a library, robust programming is a saner choice. Does it make sense?

arugal commented 4 years ago

@arugal May @soWill666 close the reporter before ending span. But I prefer we should replace the panic with a "warn" log. As a library, robust programming is a saner choice. Does it make sense?

Agreed

heiyhia commented 4 years ago

Here is my code:

package apm

import (
    "fmt"
    "github.com/SkyAPM/go2sky"
    "github.com/SkyAPM/go2sky/reporter"
)

func GetSkyTracer() *go2sky.Tracer{
    // Use gRPC reporter for production
    re, err := reporter.NewGRPCReporter("10.206.36.156:11800")
    if err != nil {
        fmt.Println("new reporter error %v \n", err)
        return nil
    }
    defer re.Close()

    tracer, err := go2sky.NewTracer("libra", go2sky.WithReporter(re))
    if err != nil {
        fmt.Println("create tracer error %v \n", err)
        return nil
    }
    tracer.WaitUntilRegister()

    return tracer
}
package apm

import (
    "fmt"
    "strconv"
    "sync"
    "time"

    "github.com/SkyAPM/go2sky"
    "github.com/SkyAPM/go2sky/propagation"
    "github.com/SkyAPM/go2sky/reporter/grpc/common"
    "github.com/gin-gonic/gin"
)

const (
    httpServerComponentID int32 = 49
)

type routeInfo struct {
    operationName string
}

type middleware struct {
    routeMap     map[string]map[string]routeInfo
    routeMapOnce sync.Once
}

//Middleware gin middleware return HandlerFunc  with tracing.
func Middleware(engine *gin.Engine, tracer *go2sky.Tracer) gin.HandlerFunc {
    if engine == nil || tracer == nil {
        return func(c *gin.Context) {
            c.Next()
        }
    }
    m := new(middleware)

    return func(c *gin.Context) {
        if c.Request.Method == "OPTIONS" {
            return
        }
        m.routeMapOnce.Do(func() {
            routes := engine.Routes()
            rm := make(map[string]map[string]routeInfo)
            for _, r := range routes {
                mm := rm[r.Method]
                if mm == nil {
                    mm = make(map[string]routeInfo)
                    rm[r.Method] = mm
                }
                mm[r.Handler] = routeInfo{
                    operationName: c.Request.Host + c.Request.URL.Path,
                }
                m.routeMap = rm
            }
        })
        var operationName string
        handlerName := c.HandlerName()
        if routeInfo, ok := m.routeMap[c.Request.Method][handlerName]; ok {
            operationName = routeInfo.operationName
        }
        if operationName == "" {
            operationName = c.Request.Method
        }
        span, ctx, err := tracer.CreateEntrySpan(c.Request.Context(), operationName, func() (string, error) {
            return c.Request.Header.Get(propagation.Header), nil
        })
        if err != nil {
            c.Next()
            return
        }
        span.SetComponent(httpServerComponentID)
        span.Tag(go2sky.TagHTTPMethod, c.Request.Method)
        span.Tag(go2sky.TagURL, c.Request.Host+c.Request.URL.Path)
        span.SetSpanLayer(common.SpanLayer_Http)

        c.Request = c.Request.WithContext(ctx)

        c.Next()

        if len(c.Errors) > 0 {
            span.Error(time.Now(), c.Errors.String())
        }

        span.Tag(go2sky.TagStatusCode, strconv.Itoa(c.Writer.Status()))
        span.End()

    }
}
func main() {
    r := gin.New()
    gin.SetMode("release")
    tracer := apm.GetSkyTracer()
    r.Use(apm.Middleware(r, tracer))
}
arugal commented 4 years ago

@soWill666 Thank you for your reply. sendCh is closed by defer re.Close()

func GetSkyTracer() *go2sky.Tracer{
    // Use gRPC reporter for production
    re, err := reporter.NewGRPCReporter("10.206.36.156:11800")
    if err != nil {
        fmt.Println("new reporter error %v \n", err)
        return nil
    }
    defer re.Close()

    tracer, err := go2sky.NewTracer("libra", go2sky.WithReporter(re))
    if err != nil {
        fmt.Println("create tracer error %v \n", err)
        return nil
    }
    tracer.WaitUntilRegister()

    return tracer
}
heiyhia commented 4 years ago

Thanks. @arugal