Skip to content

Add option for setting span names#1

Merged
KolbyMcGarrah merged 3 commits intomasterfrom
add-span-name
Mar 4, 2021
Merged

Add option for setting span names#1
KolbyMcGarrah merged 3 commits intomasterfrom
add-span-name

Conversation

@KolbyMcGarrah
Copy link
Copy Markdown
Owner

No description provided.

driver.go Outdated
if c.options.Ping && (c.options.AllowRoot || trace.FromContext(ctx) != nil) {
var span *trace.Span
ctx, span = trace.StartSpan(ctx, "sql:ping",
spanName := "sql.ping"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

changed from sql:ping to sql.ping - we should keep the default name the same.

driver.go Outdated
var span *trace.Span
ctx, span = trace.StartSpan(ctx, "sql:ping",
spanName := "sql.ping"
if c.options.FormatSpanName != nil && c.options.FormatSpanName(ctx) != "" {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe this should be pulled out into a function and expanded a bit (to check for empty span name)

func (c ocConn) getSpanName(ctx context.Context, defaultName string) (spanName string) {
	if c.options.FormatSpanName != nil && c.options.FormatSpanName(ctx) != "" {
		spanName = c.options.FormatSpanName(ctx)
	}

	if spanName == "" {
		spanName = defaultName
	}

	return spanName
}

@KolbyMcGarrah KolbyMcGarrah merged commit fa8b852 into master Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants