XSAM/otelsql

sql.conn.reset_session

Closed this issue · 4 comments

Hey, I am using this in my application. Nice work!
An issue is, I am seeing constantly a sql.conn.reset_session span. Not sure what it is, but it seems to be happening on each Ping? Very spammy. Should this span be disabled when Ping's are disabled?

XSAM commented

@DejeroDaniel Thanks for raising this issue.

From the Go Doc about the SessionResetter:

ResetSession is called prior to executing a query on the connection
if the connection has been used before. If the driver returns ErrBadConn
the connection is discarded.

It will happen on a second or more calls no matter the call is Ping or Query. It's an actual operation that the driver will do and might help while debugging.

I not sure we should drop this reset_session span by default then use a flag to enable it or contrarily.

@seh @fsaintjacques @nvx Any suggestions on this one? 😄

seh commented

I haven't noticed these spans. If this is something the driver is performing involving network activity (or even something that might take a while if interacting with SQLite locally), I'd prefer to see it represented in the traces. If we'd consider filtering this one, I'd prefer that we treat the problem more generally, to avoid adding something to the exported interface for just this particular operation.

I use both ddtrace and otel, and when I am using gopkg.in/DataDog/dd-trace-go.v1/contrib/database/sql and gopkg.in/DataDog/dd-trace-go.v1/contrib/jmoiron/sqlx I don't see these spans at all. They must be filtering it in that library somewhere.

Right now I gave the db driver a different service name so I can filter these out in the dashboard, but it is pretty spammy.

Even if we think these are important info, I think the sql.conn.reset_session that are related to pings should go away if we have disabled ping in the SpanOptions.

XSAM commented

I think the sql.conn.reset_session that are related to pings should go away if we have disabled ping in the SpanOptions.

Definitely agree with that. Also, I think there is no SpanContext passed to the Ping function, so the sql.conn.reset_session span will separate from the Ping span.

This brings me the idea that we should not instrument a sql function by default if there is no Span in the Context since discrete spans might have little help for analysing what happens in the program. It can also address @DejeroDaniel 's concern.