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

feat(flask): Add sentry_trace() template helper #1336

Merged
merged 22 commits into from
Feb 14, 2022
Merged

feat(flask): Add sentry_trace() template helper #1336

merged 22 commits into from
Feb 14, 2022

Conversation

BYK
Copy link
Member

@BYK BYK commented Feb 14, 2022

To setup distributed tracing links between a Flask app and a front-end app, one needs to figure out how to get the current hub, safely get the traceparent and then properly pass it into a template and then finally use that properly in a meta tag. The guide is woefully inadequate and error-prone so this PR adds a built-in helper sentry_trace() to the Flask integration to simplify this linking.

I'll follow up with documentation updates once the approach here is approved.

BYK added 2 commits February 14, 2022 15:41
To setup distributed tracing links between a Flask app and a front-end app, one needs to figure out how to get the current hub, safely get the traceparent and then properly pass it into a template and then finally use that properly in a `meta` tag. [The guide](https://docs.sentry.io/platforms/javascript/performance/connect-services/) is woefully inadequete and error-prone so this PR adds a built-in helper `sentry_trace()` to the Flask integration to simplfy this linking.
@sl0thentr0py
Copy link
Member

@BYK the tests on py3.4/old flask versions fail because there's no add_template_global apparently, maybe add a hasattr check and patch only if exists.

@sl0thentr0py
Copy link
Member

hmm no ignore that, add_template_global exists since version 0.10, so something else going on here.

@BYK
Copy link
Member Author

BYK commented Feb 14, 2022

@sl0thentr0py I figured out the issue here. It's what @untitaker said. Flask.__call__() creates you a wsgi app from an already existing Flask instance. I got confused by that. I'll try to use before_render_template signal but not sure if I can modify the context at that point. Will try.

@BYK
Copy link
Member Author

BYK commented Feb 14, 2022

@silent1mezzo finally, ready to merge. Sorry for all that noise!

@sl0thentr0py sl0thentr0py enabled auto-merge (squash) February 14, 2022 15:58
@BYK
Copy link
Member Author

BYK commented Feb 14, 2022

@untitaker @mitsuhiko @sl0thentr0py I don't think that format job will ever run

@BYK
Copy link
Member Author

BYK commented Feb 14, 2022

@sl0thentr0py it won't because this is set to run on push: https://github.com/getsentry/sentry-python/blob/master/.github/workflows/black.yml#L3

But I can only push to my own repo.

@sl0thentr0py
Copy link
Member

yea we need to kill that black action and move to pre-commit, it's super annoying.
I can't merge without it passing because we removed admin overrides 2 weeks ago, sigh..

@BYK
Copy link
Member Author

BYK commented Feb 14, 2022

I can't merge without it passing because we removed admin overrides 2 weeks ago, sigh..

How do you release with Craft then?

@sl0thentr0py
Copy link
Member

sl0thentr0py commented Feb 14, 2022

I just can't override required checks and merge now. I run the release workflow here and someone else approves. Dunno how/if those are related?

image

@BYK
Copy link
Member Author

BYK commented Feb 14, 2022

@sl0thentr0py see item 3 here: https://github.com/getsentry/publish#setup

Either you no longer have admin permissions, or you folks started using the "Allow specific actors to bypass pull request requirements" in branch protection rules (which means that README on publish needs to be updated), or you'll have trouble when you try to make the next release 😀

@AbhiPrasad
Copy link
Member

We might want to do something similar for django as well. See: #1171

@BYK
Copy link
Member Author

BYK commented Feb 14, 2022

@AbhiPrasad if only Functional Software Inc. was still paying me... 😛

@AbhiPrasad
Copy link
Member

if only Functional Software Inc. was still paying me

Well if ever get the itch to help out, opened #1341 to track. Thanks for your help BYK, glad to see you're still around :)

@BYK BYK deleted the byk/feat/flask-sentry-trace-template-helper branch February 14, 2022 18:28
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.

5 participants