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.

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

Pass around contexts in as sentry intends #4

Merged
merged 3 commits into from
Jun 18, 2021

Conversation

GTB3NW
Copy link
Contributor

@GTB3NW GTB3NW commented Jun 18, 2021

  • Any spans within the handler will now inherit the interceptor span
  • If the sentry-trace metadata exists it will update the span so that it continues. This is the exact approach it takes with the http library except that does it with headers.

@GTB3NW
Copy link
Contributor Author

GTB3NW commented Jun 18, 2021

I'm going to submit a couple of other changes and make it a slightly bigger PR, so please don't merge for now

@GTB3NW GTB3NW changed the title Overwrite with span context Pass around contexts in as sentry intends Jun 18, 2021
@GTB3NW
Copy link
Contributor Author

GTB3NW commented Jun 18, 2021

I think this should work. I'm going to run some local testing first and confirm it works as I intended (traces work between services). I've tried to keep it as idiomatic as the http library. I had to copy over the logic from sentry's lib because they don't expose it annoyingly (they do for the equivalent client calls). I'm hoping to upstream this method to read from grpc metadata since I imagine it's a useful function for others too.

@GTB3NW
Copy link
Contributor Author

GTB3NW commented Jun 18, 2021

Ohh and I took the liberty of updating sentry-go

@GTB3NW
Copy link
Contributor Author

GTB3NW commented Jun 18, 2021

I've tested, the span upgrade fails gracefully if the metadata isn't there. I've not had chance to test the client. I'll test that next week and post an update here :)

@johnbellone johnbellone merged commit 8e6180c into johnbellone:master Jun 18, 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