Skip to content
This repository has been archived by the owner on Jun 14, 2023. It is now read-only.

feat(sql): add database/sql plugin #22

Merged
merged 34 commits into from
Aug 25, 2021
Merged

feat(sql): add database/sql plugin #22

merged 34 commits into from
Aug 25, 2021

Conversation

kagaya85
Copy link
Member

@kagaya85 kagaya85 commented Jul 7, 2021

Add go2sky plugin for database/sql

This PR is still WIP and may need sufficient tests before merge

Because this is a new plugin, we may need to discuss some implementation details

Any comments or suggestions are welcome, thanks in advance

@mrproliu mrproliu requested review from mrproliu and arugal July 7, 2021 08:21
@mrproliu mrproliu added the plugin label Jul 7, 2021
@mrproliu mrproliu added this to the 1.2.0 milestone Jul 7, 2021
sql/stmt.go Outdated Show resolved Hide resolved
sql/common.go Outdated Show resolved Hide resolved
@arugal
Copy link
Member

arugal commented Jul 30, 2021

image

Hi @kagaya85 please update the license header :)

@kagaya85 kagaya85 changed the title [WIP]feat(sql): add database/sql plugin feat(sql): add database/sql plugin Aug 11, 2021
@kagaya85 kagaya85 requested a review from arugal August 11, 2021 08:37
@kagaya85
Copy link
Member Author

Now, the Tx struct contains a context of the transaction local span

if user use methods with context that contains span context, like ExecContext, new exit span will follow the span in the context, in addition, for methods without context(Exec, Query) or context do not contain span, new exit span will follow the local span in Tx struct

type Tx struct {
	*sql.Tx

	db   *DB
	span go2sky.Span
	ctx  context.Context
}

func (tx *Tx) Exec(query string, args ...interface{}) (sql.Result, error) {
	return tx.ExecContext(tx.ctx, query, args)
}

func (tx *Tx) ExecContext(ctx context.Context, query string, args ...interface{}) (sql.Result, error) {
	if id := go2sky.SpanID(ctx); id == go2sky.EmptySpanID {
		// if ctx do not contain parent span, use transaction ctx instead
		ctx = tx.ctx
	}
	span, err := createSpan(ctx, tx.db.tracer, tx.db.opts, "execute")
	if err != nil {
		return nil, err
	}
	defer span.End()

	...

	return res, err
}

@kagaya85
Copy link
Member Author

@arugal For the previous implementation, is it possible to keep it as a new plugin which based on database/sql/driver?
this can lead to more detailed tracking and flexibility, such as using in some third-party sql libaray like sqlx

sql/tx.go Outdated Show resolved Hide resolved
@arugal
Copy link
Member

arugal commented Aug 25, 2021

@arugal For the previous implementation, is it possible to keep it as a new plugin which based on database/sql/driver?
this can lead to more detailed tracking and flexibility, such as using in some third-party sql libaray like sqlx

Good idea, we can do this in a new PR, after the current PR is merged.

Copy link
Member

@arugal arugal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Local Test Results

Database

image

Trace

image

image

@arugal arugal merged commit 8dab0ad into SkyAPM:master Aug 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
3 participants