Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop creating root spans without existing spans #13

Merged
merged 5 commits into from
May 13, 2021
Merged

Stop creating root spans without existing spans #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
56d525f
Add AllowRoot option
XSAM May 12, 2021
19aaa0a
Add tests
XSAM May 12, 2021
0706878
Update description about AllowRoot option
XSAM May 12, 2021
34c942b
Update CHANGELOG
XSAM May 12, 2021
0bc9d52
Add more tests for recordSpanError
XSAM May 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,14 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

### Added

- Add AllowRoot option to prevent backward incompatible. (#13)

### Changed

- Upgrade to v0.20.0 of `go.opentelemetry.io/otel`. (#8)
- otelsql will not create root spans in absence of existing spans by default. (#13)

## [0.2.1] - 2021-03-28

Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ $ go get github.com/XSAM/otelsql
| Ping | If set to true, will enable the creation of spans on Ping requests. | Implemented | Ping has context argument, but it might no needs to record. |
| RowsNext | If set to true, will enable the creation of events on corresponding calls. This can result in many events. | Implemented | It provides more visibility. |
| DisableErrSkip | If set to true, will suppress driver.ErrSkip errors in spans. | Implemented | ErrSkip error might annoying |
| AllowRoot | If set to true, will allow otelsql to create root spans in absence of existing spans or even context. | Implemented | It might helpful while debugging missing operations. |
| RowsAffected, LastInsertID | If set to true, will enable the creation of spans on RowsAffected/LastInsertId calls. | Dropped | Don't know its use cases. We might add this later based on the users' feedback. |
| QueryParams | If set to true, will enable recording of parameters used with parametrized queries. | Dropped | It will cause high cardinality values and security problems. |
| AllowRoot | If set to true, will allow ocsql to create root spans in absence of existing spans or even context. | Dropped | I don't think traces data have meaning without context. |

## Example

Expand Down
3 changes: 3 additions & 0 deletions config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ type SpanOptions struct {

// DisableErrSkip, if set to true, will suppress driver.ErrSkip errors in spans.
DisableErrSkip bool

// AllowRoot, if set to true, will create root spans in absence of existing spans or even context.
AllowRoot bool
}

type defaultSpanNameFormatter struct{}
Expand Down
87 changes: 52 additions & 35 deletions conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (c *otConn) Ping(ctx context.Context) (err error) {
return driver.ErrSkip
}

if c.cfg.SpanOptions.Ping {
if c.cfg.SpanOptions.Ping && (c.cfg.SpanOptions.AllowRoot || trace.SpanContextFromContext(ctx).IsValid()) {
var span trace.Span
ctx, span = c.cfg.Tracer.Start(ctx, c.cfg.SpanNameFormatter.Format(ctx, MethodConnPing, ""),
trace.WithSpanKind(trace.SpanKindClient),
Expand Down Expand Up @@ -85,14 +85,17 @@ func (c *otConn) ExecContext(ctx context.Context, query string, args []driver.Na
return nil, driver.ErrSkip
}

ctx, span := c.cfg.Tracer.Start(ctx, c.cfg.SpanNameFormatter.Format(ctx, MethodConnExec, query),
trace.WithSpanKind(trace.SpanKindClient),
trace.WithAttributes(
append(c.cfg.Attributes,
semconv.DBStatementKey.String(query),
)...),
)
defer span.End()
var span trace.Span
if c.cfg.SpanOptions.AllowRoot || trace.SpanContextFromContext(ctx).IsValid() {
ctx, span = c.cfg.Tracer.Start(ctx, c.cfg.SpanNameFormatter.Format(ctx, MethodConnExec, query),
trace.WithSpanKind(trace.SpanKindClient),
trace.WithAttributes(
append(c.cfg.Attributes,
semconv.DBStatementKey.String(query),
)...),
)
defer span.End()
}

res, err = execer.ExecContext(ctx, query, args)
if err != nil {
Expand All @@ -116,14 +119,18 @@ func (c *otConn) QueryContext(ctx context.Context, query string, args []driver.N
return nil, driver.ErrSkip
}

queryCtx, span := c.cfg.Tracer.Start(ctx, c.cfg.SpanNameFormatter.Format(ctx, MethodConnQuery, query),
trace.WithSpanKind(trace.SpanKindClient),
trace.WithAttributes(
append(c.cfg.Attributes,
semconv.DBStatementKey.String(query),
)...),
)
defer span.End()
var span trace.Span
queryCtx := ctx
if c.cfg.SpanOptions.AllowRoot || trace.SpanContextFromContext(ctx).IsValid() {
queryCtx, span = c.cfg.Tracer.Start(ctx, c.cfg.SpanNameFormatter.Format(ctx, MethodConnQuery, query),
trace.WithSpanKind(trace.SpanKindClient),
trace.WithAttributes(
append(c.cfg.Attributes,
semconv.DBStatementKey.String(query),
)...),
)
defer span.End()
}

rows, err = queryer.QueryContext(queryCtx, query, args)
if err != nil {
Expand All @@ -139,14 +146,17 @@ func (c *otConn) PrepareContext(ctx context.Context, query string) (stmt driver.
return nil, driver.ErrSkip
}

ctx, span := c.cfg.Tracer.Start(ctx, c.cfg.SpanNameFormatter.Format(ctx, MethodConnPrepare, query),
trace.WithSpanKind(trace.SpanKindClient),
trace.WithAttributes(
append(c.cfg.Attributes,
semconv.DBStatementKey.String(query),
)...),
)
defer span.End()
var span trace.Span
if c.cfg.SpanOptions.AllowRoot || trace.SpanContextFromContext(ctx).IsValid() {
ctx, span = c.cfg.Tracer.Start(ctx, c.cfg.SpanNameFormatter.Format(ctx, MethodConnPrepare, query),
trace.WithSpanKind(trace.SpanKindClient),
trace.WithAttributes(
append(c.cfg.Attributes,
semconv.DBStatementKey.String(query),
)...),
)
defer span.End()
}

stmt, err = preparer.PrepareContext(ctx, query)
if err != nil {
Expand All @@ -162,11 +172,15 @@ func (c *otConn) BeginTx(ctx context.Context, opts driver.TxOptions) (tx driver.
return nil, driver.ErrSkip
}

beginTxCtx, span := c.cfg.Tracer.Start(ctx, c.cfg.SpanNameFormatter.Format(ctx, MethodConnBeginTx, ""),
trace.WithSpanKind(trace.SpanKindClient),
trace.WithAttributes(c.cfg.Attributes...),
)
defer span.End()
var span trace.Span
beginTxCtx := ctx
if c.cfg.SpanOptions.AllowRoot || trace.SpanContextFromContext(ctx).IsValid() {
beginTxCtx, span = c.cfg.Tracer.Start(ctx, c.cfg.SpanNameFormatter.Format(ctx, MethodConnBeginTx, ""),
trace.WithSpanKind(trace.SpanKindClient),
trace.WithAttributes(c.cfg.Attributes...),
)
defer span.End()
}

tx, err = connBeginTx.BeginTx(beginTxCtx, opts)
if err != nil {
Expand All @@ -182,11 +196,14 @@ func (c *otConn) ResetSession(ctx context.Context) (err error) {
return driver.ErrSkip
}

ctx, span := c.cfg.Tracer.Start(ctx, c.cfg.SpanNameFormatter.Format(ctx, MethodConnResetSession, ""),
trace.WithSpanKind(trace.SpanKindClient),
trace.WithAttributes(c.cfg.Attributes...),
)
defer span.End()
var span trace.Span
if c.cfg.SpanOptions.AllowRoot || trace.SpanContextFromContext(ctx).IsValid() {
ctx, span = c.cfg.Tracer.Start(ctx, c.cfg.SpanNameFormatter.Format(ctx, MethodConnResetSession, ""),
trace.WithSpanKind(trace.SpanKindClient),
trace.WithAttributes(c.cfg.Attributes...),
)
defer span.End()
}

err = sessionResetter.ResetSession(ctx)
if err != nil {
Expand Down
Loading