Skip to content

Expose SpanFromContext#672

Merged
tonyo merged 2 commits intogetsentry:masterfrom
zhezhel:expose-span-from-context
Jul 27, 2023
Merged

Expose SpanFromContext#672
tonyo merged 2 commits intogetsentry:masterfrom
zhezhel:expose-span-from-context

Conversation

@zhezhel
Copy link
Contributor

@zhezhel zhezhel commented Jul 24, 2023

I'm working on integration pgx with Sentry.

The new version of pgx provides an interface for tracing, which work in the way when you need to implement two different methods: TraceQueryStart & TraceQueryEnd.

To implement TraceQueryEnd, I need to get access to span in context and call Finish on it.

It would be great to provide a method to get span from context.
Current workaround:

func SpanFromContext(ctx interface{}) (*sentry.Span, bool) {
	values := reflect.ValueOf(ctx).Elem()

	for i := 0; i < values.NumField(); i++ {
		reflectValue := values.Field(i)
		reflectValue = reflect.NewAt(reflectValue.Type(), unsafe.Pointer(reflectValue.UnsafeAddr())).Elem()
		span, ok := reflectValue.Interface().(*sentry.Span)
		if !ok {
			continue
		}
		return span, true
	}
	return nil, false
}

@codecov
Copy link

codecov bot commented Jul 24, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (b551438) 80.56% compared to head (ba1e9b0) 80.56%.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master     #672   +/-   ##
=======================================
  Coverage   80.56%   80.56%           
=======================================
  Files          44       44           
  Lines        4460     4460           
=======================================
  Hits         3593     3593           
  Misses        736      736           
  Partials      131      131
Files Changed Coverage Δ
tracing.go 85.95% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cleptric
cleptric approved these changes Jul 26, 2023
View reviewed changes
Copy link
Member

@cleptric cleptric left a comment

Choose a reason for hiding this comment

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

Works for me, @tonyo ?

@cleptric cleptric requested a review from tonyo July 26, 2023 08:56
tonyo
tonyo reviewed Jul 26, 2023
View reviewed changes
@zhezhel zhezhel requested a review from tonyo July 26, 2023 15:08
Andrii Zhezhel and others added 2 commits July 26, 2023 17:50
@zhezhel
Expose SpanFromContext
d95b1ec
@zhezhel @tonyo
Update tracing.go
ba1e9b0 
Co-authored-by: Anton Ovchinnikov <anton@tonyo.info>
@zhezhel zhezhel force-pushed the expose-span-from-context branch from 933751b to ba1e9b0 Compare July 26, 2023 15:50
@zhezhel
Copy link
Contributor Author

zhezhel commented Jul 27, 2023

Hey @cleptric, @tonyo, do you have plans to merge this PR?

@tonyo
Copy link
Contributor

tonyo commented Jul 27, 2023

Yes, we do!

@tonyo tonyo merged commit dde4d36 into getsentry:master Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cleptric cleptric cleptric approved these changes

+1 more reviewer

@tonyo tonyo tonyo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zhezhel @tonyo @cleptric