dd-trace-go icon indicating copy to clipboard operation
dd-trace-go copied to clipboard

contrib/gorm.io/gorm.v1: child database spans are not attached to the gorm parent span

Open hatstand opened this issue 3 years ago • 13 comments

(As discussed on https://github.com/DataDog/dd-trace-go/pull/759)

If both gorm & the underlying *sql.DB connection are setup for tracing, spans for the underlying database connection queries are not attached to the gorm span created for the query.

Screenshot 2021-02-18 at 11 10 43

It's possible I've misconfigured something but it does seem like an actual issue from the above screenshot (the purple postgres.query span should be below the gorm.query span). I think this isn't an issue for jinzhu/gorm as there's no way to inject a traced sql.DB in the old API so you wouldn't have any sql spans at all.

For reproducing, we've got something like this setup:

import (
  sqltrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/database/sql"
  gormtrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/gorm.io/gorm.v1"
)

sqltrace.Register("postgres", &pq.Driver{})
db, _ := sqltrace.Open("postgres", "postgres://")
orm, _ := gormtrace.Open(postgres.New(postgres.Config{
  Conn: db,
})

hatstand avatar Feb 18 '21 11:02 hatstand

Perfect. This is very helpful and paints a very clear picture. Thank you for taking the time to create this issue. Would you have any interest in doing the work? Otherwise we can schedule some work internally.

gbbr avatar Feb 18 '21 13:02 gbbr

I might be able to find time to do this assuming it's as simple as creating the span in before instead of after.

hatstand avatar Feb 18 '21 13:02 hatstand

I think it should be much easier than that. No need to move it to before. Wouldn't this work?

Replacing:

span, _ := tracer.StartSpanFromContext(ctx, operationName, opts...)

With:

var span ddtrace.Span
span, db.Statement.Context = tracer.StartSpanFromContext(ctx, operationName, opts...)

gbbr avatar Feb 18 '21 13:02 gbbr

But I see what you mean. Never mind the above :) Perhaps we could store the span in the context, just like we do the start time now... I wonder if there's any risk to create a leak like that, considering someone never triggers the after...

gbbr avatar Feb 18 '21 13:02 gbbr

I'm pretty sure this is impossible without rewriting Gorm? Since Gorm doesn't call ExecContext, or QueryContext, the context won't even be passed to link the two. I'm not sure how you do this. I imagine this is why Begin/Commit does show up tied to the parent spans - because Gorm calls BeginTx(ctx...)

Gorm uses these interfaces:

// SQLCommon is the minimal database connection functionality gorm requires.  Implemented by *sql.DB.
type SQLCommon interface {
	Exec(query string, args ...interface{}) (sql.Result, error)
	Prepare(query string) (*sql.Stmt, error)
	Query(query string, args ...interface{}) (*sql.Rows, error)
	QueryRow(query string, args ...interface{}) *sql.Row
}

type sqlDb interface {
	Begin() (*sql.Tx, error)
	BeginTx(ctx context.Context, opts *sql.TxOptions) (*sql.Tx, error)
}

type sqlTx interface {
	Commit() error
	Rollback() error
}

sjmtan avatar Mar 02 '21 19:03 sjmtan

I guess that's what I see in my traces - only Begin/Commit show up associated, all others are their own root spans (which is really really annoying)

sjmtan avatar Mar 02 '21 19:03 sjmtan

Any progress or updates on this? i am facing same issue. If required i can help with the PR.

prasad-marne avatar Jun 10 '21 06:06 prasad-marne

Hi, @prasad-marne. We have not made any progress on this issue and any community help is welcome.

knusbaum avatar Aug 26 '21 04:08 knusbaum

Hey -- wondering if there's any update on this? Have other people found it a blocker from using? I want to upgrade from Gorm 1.0 to 2.0 but need to make sure the gorm2 wrapper for ddtrace is usable

admarko avatar Sep 29 '21 13:09 admarko

Any updates on this issue? looks like it has not been addressed on the latest release of contrib/gorm.io/gorm.v1

VinhVu95 avatar Oct 26 '22 07:10 VinhVu95

I don't know if it's related, but recently we updated gorm from v1 to v2 and used this example to help us to configure datadog with gorm https://github.com/DataDog/dd-trace-go/pull/759. But the traces from the http and gorm database traces are not correlated and they are shown as separated traces as I show in the capture images.

Screenshot 2024-03-14 at 08 29 24

So, searching for possible workarounds, I found this issue in the gorm repository: https://github.com/go-gorm/gorm/issues/1231

And use the function WithContext(ctx) to propagate the context. With this approach, the full trace is shown in DataDog panel.

Screenshot 2024-03-13 at 16 56 40

Could it be possible that DataDog cannot get the context?

ervitis avatar Mar 13 '24 23:03 ervitis

@ervitis I'm currently encountering the same problem where I can't see the correlated traces between Gin and Gorm. They seem to be separated. However, the issue lies in the fact that I can see the Gorm tracing and the Gin tracing have their own root spans. On the other hand, the Gin Context doesn't seem to carry the right information. In my case, I passed the Gin Context from the handler to the repository layer where Gorm executed the query. Once I used db.WithContext(c.Request.Context) instead of the Gin Context, the correct tracing appeared in Datadog.

However, there's still one question remaining: if I want to use a goroutine to perform some asynchronous jobs, should I copy the Gin Context to maintain the information? Does anyone know how to deal with this?

p0937507934 avatar May 07 '24 17:05 p0937507934