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

Event processor misuse #1752

Open
1 of 3 tasks
smeubank opened this issue Nov 22, 2022 · 1 comment
Open
1 of 3 tasks

Event processor misuse #1752

smeubank opened this issue Nov 22, 2022 · 1 comment
Labels
Triaged Has been looked at recently during old issue triage

Comments

@smeubank
Copy link
Member

smeubank commented Nov 22, 2022

Background:

It is not easy to set/change the transaction name in many Sentry SDKs. This is because the SDK is setting the name in the event processor. Which is a misuse. This is only one example, of the SDK design making it difficult to manipulate their transaction (but not only txn events) before sending.

Doesn't work:

sentry_sdk.Hub.current.scope.transaction.name = "new name"
#
transaction = sentry_sdk.Hub.current.scope.transaction
transaction.name = "new name"

Outcome:

the customer wanting to change the transaction name is that all transactions from graphql requests have the same name

i tried to build graphene support at some point and couldnt because our txn apis just didnt work as i would need them to
why?

the story: i have a middleware entry point, and i need to set the transaction name from there.. thats basically every framework and what i tried to do in graphene and it just wouldnt work because of [something i dont understand]

Short term fixes:

  • For errors there is beforeSend, and beforeSendTransaction is being added on different SDKs to provide a better experience, but this is not a perfect fix.
  • remove

Underlying problem statement

There is a set_transaction api but the SDK explicitly ignores it

def _make_request_event_processor(req, integration):
# type: (Any, Any) -> Callable[[Dict[str, Any], Dict[str, Any]], Dict[str, Any]]
def event_processor(event, hint):
# type: (Dict[str, Any], Dict[str, Any]) -> Dict[str, Any]
# Extract information from request
request_info = event.get("request", {})
if info:
if "cookies" in info and _should_send_default_pii():
request_info["cookies"] = info["cookies"]
if "data" in info:
request_info["data"] = info["data"]
event["request"] = request_info
_set_transaction_name_and_source(
event, integration.transaction_style, req
)

https://sentry.slack.com/archives/C02T4BB83AS/p1669105735303619?thread_ts=1669100987.749389&cid=C02T4BB83AS

We use event processors in almost all our integrations. But event processors aren't intended to be used by the user . (At least thats what I thought, but I now searched the documentation and in Python/PHP/Ruby it is documented nowhere how to use event processors. In JS there is this page: https://docs.sentry.io/platforms/javascript/enriching-events/event-processors/ So maybe my world view of event processors is wrong.)

Solution proposal:

  1. why isnt there a Sentry.setTransaction API like i though there used to be
  2. can we fix this so it actually works - this is critical to instrumenting sentry correctly

There is a latent desire of some of the teams to have proper life cycle events to hook into to get rid of the event processors in integrations

The problem is that in the life cycle of an event the event processors are run at one certain point. (when the scope is applied to the event, this happens when the event is prepared to be sent over the wire/air) And our integrations try to have all the data ready by this points. But it leads to very hard to read and/or understand code.
So the solution is: life cycle hooks! 🧬 ⭕ 🪝
So we can run code to get information not only in event processors but on all the points in the life cycle of an event that we specify in this RFC: getsentry/rfcs#34

using event processors internally in integrations is just an anti-pattern imo
just add it to the current scope/transaction whenever data is ready

with capture_internal_exceptions():
DjangoRequestExtractor(request).extract_into_event(event)
if _should_send_default_pii():
with capture_internal_exceptions():
_set_user_info(request, event)

Related issues:

#1751

@smeubank
Copy link
Member Author

also dependent on lifecycle hooks

@antonpirker antonpirker removed their assignment Feb 6, 2023
@sentrivana sentrivana added the Triaged Has been looked at recently during old issue triage label Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Triaged Has been looked at recently during old issue triage
Projects
None yet
Development

No branches or pull requests

5 participants