From 616f65725c43ff3fe1cf7d0eef616a0817be1b5f Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 23 Oct 2023 20:34:58 +0200 Subject: [PATCH 1/5] rfc(decision): mobile-transactions-and-spans --- README.md | 1 + text/0117-mobile-transactions-and-spans.md | 35 ++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 text/0117-mobile-transactions-and-spans.md diff --git a/README.md b/README.md index 3eaba12a..b49ec393 100644 --- a/README.md +++ b/README.md @@ -57,3 +57,4 @@ This repository contains RFCs and DACIs. Lost? - [0096-client-sampling-decision-dsc](text/0096-client-sampling-decision-dsc.md): Client Sampling Decision in Dynamic Sampling Context - [0101-revamping-the-sdk-performance-api](text/0101-revamping-the-sdk-performance-api.md): Revamping the SDK Performance API - [0106-artifact-indices](text/0106-artifact-indices.md): Improvements to Source Maps Processing +- [0117-mobiletransactionsandspans](text/0117-mobiletransactionsandspans.md): mobile-transactions-and-spans diff --git a/text/0117-mobile-transactions-and-spans.md b/text/0117-mobile-transactions-and-spans.md new file mode 100644 index 00000000..08edbc7c --- /dev/null +++ b/text/0117-mobile-transactions-and-spans.md @@ -0,0 +1,35 @@ +- Start Date: 2023-10-23 +- RFC Type: decision +- RFC PR: https://github.com/getsentry/rfcs/pull/117 +- RFC Status: draft + +# Summary + +One paragraph explanation of the feature or document purpose. + +# Motivation + +Why are we doing this? What use cases does it support? What is the expected outcome? + +# Background + +The reason this decision or document is required. This section might not always exist. + +# Supporting Data + +[Metrics to help support your decision (if applicable).] + +# Options Considered + +If an RFC does not know yet what the options are, it can propose multiple options. The +preferred model is to propose one option and to provide alternatives. + +# Drawbacks + +Why should we not do this? What are the drawbacks of this RFC or a particular option if +multiple options are presented. + +# Unresolved questions + +- What parts of the design do you expect to resolve through this RFC? +- What issues are out of scope for this RFC but are known? From 3225bb72a478041661b62034abeddd6fddd13a59 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 23 Oct 2023 22:06:18 +0200 Subject: [PATCH 2/5] Initial draft --- text/0117-mobile-transactions-and-spans.md | 58 +++++++++++++++++----- 1 file changed, 45 insertions(+), 13 deletions(-) diff --git a/text/0117-mobile-transactions-and-spans.md b/text/0117-mobile-transactions-and-spans.md index 08edbc7c..108fa3f6 100644 --- a/text/0117-mobile-transactions-and-spans.md +++ b/text/0117-mobile-transactions-and-spans.md @@ -5,31 +5,63 @@ # Summary -One paragraph explanation of the feature or document purpose. +This RFC proposes to adapt the way transactions and spans are generated and sent on mobile platforms. In the recent light of [the performance API revamp](0101-revamping-the-sdk-performance-api.md) and the mobile starfish efforts we aim to provide a way of aggregating span data on a screen level. # Motivation -Why are we doing this? What use cases does it support? What is the expected outcome? +For mobile starfish we want to be able to aggregate span data on a screen level. Metrics like TTID, TTFD, slow & frozen frames are especially useful if they can be grouped by screen, giving more actionable insights into the fast and slow parts of an app. # Background -The reason this decision or document is required. This section might not always exist. +As of now the way transactions are created has several drawbacks: -# Supporting Data +1. Missing root transaction: We automatically create transactions whenever an Activity (Android) or ViewController (iOS) is created, but they idle-timeout after 3s. This means we sometimes end up in situations where we can’t attach spans to a transaction as there's no running transaction. -[Metrics to help support your decision (if applicable).] +2. Multiple automatic transaction sources: We automatically start transactions for activities, user interactions and navigation events. These different sources are racing against each other as there can only be one transaction running (if a transaction is already running, a new one won’t be started), making it hard to predict the overall behavior. + +3. Automatic vs. manual transactions: Any user-created transaction potentially breaks the behavior of the existing automatic-transaction generation, since there can only be a single transaction on the scope. + +4. Transaction durations are sometimes pointless: As they’re based on idle transaction. Any relevant or even non-relevant child span can influence the transaction duration. # Options Considered -If an RFC does not know yet what the options are, it can propose multiple options. The -preferred model is to propose one option and to provide alternatives. +### 1. Transactions as Carriers for Spans (preferred approach) + +The idea is to use transactions as carriers for measurements on mobile instead of trying to make the backend heavy concept work on mobile. There is no active "open" transaction, but we rather create transactions on-demand, whenever there is span data to send. +In Sentry, we need pick apart these special transactions (that we shouldn’t consider as such anymore, maybe use `transaction->transaction_info->source` to identify carrier transaction) and only use their content, which are the spans we measured. + +TBD: +* How can we ensure that spans are packaged up efficiently, ultimately creating not to little and not too many web requests. +* As of now, some performance grouping is done based on transactions op and description. With this change they will simply act as carieers, so we need to ensure the span context has enough information that aggregation (e.g. by screen) can still be performed. +* Profiles are right now captured based on transaction start signals, we need to find a reliable way to keep the remaining functionality. + + +**Pros** + +- We always get what we want when we want it +- Once we can ingest spans, it’ll be easy to switch. + +**Cons** + +- We need to change how we interact with transaction in the SDK and in the product + +### 2. One Transaction per Screen + +Have one transaction per screen. This automatic transaction can be fully managed within the Sentry SDK. If no spans are generated an empty transaction would still be sent, as it contains slow/frozen frame metrics and ttid/ttfd data. + +**Pros** + +- There’s always a transaction running, which spans can be attached to +- There is no need for idle transactions anymore, as the lifetime matches a screen lifetime (a max deadline maybe still makes sense), making the overall behavior more predictable +- User interactions are modeled as spans instead of transactions, so they don’t interfere with transactions +- Slow and frozen frames could be added as child spans to the running transaction -# Drawbacks +**Cons** -Why should we not do this? What are the drawbacks of this RFC or a particular option if -multiple options are presented. +- The transaction duration still makes no sense, as it’s length is determined by the time the screen was visible. Also probably needs a max timeout again too +- Any manually added / background work spans (e.g. a long running sync) could extend the transaction lifetime, which could result in overlapping transactions +- We need new (manual) APIs to track the current screen -# Unresolved questions +### 3. Leave it as-is -- What parts of the design do you expect to resolve through this RFC? -- What issues are out of scope for this RFC but are known? +Whilst being the least effort, this option doesn't add any value and we remain with all the drawbacks as outlined in the [background section](#background). From 50cb914ba3030fc01fe4cabb121a65c66507be1e Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 24 Oct 2023 09:06:37 +0200 Subject: [PATCH 3/5] Fix RFC PR link --- text/0117-mobile-transactions-and-spans.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0117-mobile-transactions-and-spans.md b/text/0117-mobile-transactions-and-spans.md index 108fa3f6..d13a5170 100644 --- a/text/0117-mobile-transactions-and-spans.md +++ b/text/0117-mobile-transactions-and-spans.md @@ -1,6 +1,6 @@ - Start Date: 2023-10-23 - RFC Type: decision -- RFC PR: https://github.com/getsentry/rfcs/pull/117 +- RFC PR: https://github.com/getsentry/rfcs/pull/118 - RFC Status: draft # Summary From 54ad034369045562f4edeabb4a7872595d91d02b Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 30 Oct 2023 07:36:01 +0100 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Philipp Hofmann Co-authored-by: Daniel Griesser Co-authored-by: Abhijeet Prasad --- README.md | 2 +- text/0117-mobile-transactions-and-spans.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index b49ec393..c5905a87 100644 --- a/README.md +++ b/README.md @@ -57,4 +57,4 @@ This repository contains RFCs and DACIs. Lost? - [0096-client-sampling-decision-dsc](text/0096-client-sampling-decision-dsc.md): Client Sampling Decision in Dynamic Sampling Context - [0101-revamping-the-sdk-performance-api](text/0101-revamping-the-sdk-performance-api.md): Revamping the SDK Performance API - [0106-artifact-indices](text/0106-artifact-indices.md): Improvements to Source Maps Processing -- [0117-mobiletransactionsandspans](text/0117-mobiletransactionsandspans.md): mobile-transactions-and-spans +- [0118-mobiletransactionsandspans](text/0118-mobiletransactionsandspans.md): mobile-transactions-and-spans diff --git a/text/0117-mobile-transactions-and-spans.md b/text/0117-mobile-transactions-and-spans.md index d13a5170..af6ff5c4 100644 --- a/text/0117-mobile-transactions-and-spans.md +++ b/text/0117-mobile-transactions-and-spans.md @@ -28,11 +28,11 @@ As of now the way transactions are created has several drawbacks: ### 1. Transactions as Carriers for Spans (preferred approach) The idea is to use transactions as carriers for measurements on mobile instead of trying to make the backend heavy concept work on mobile. There is no active "open" transaction, but we rather create transactions on-demand, whenever there is span data to send. -In Sentry, we need pick apart these special transactions (that we shouldn’t consider as such anymore, maybe use `transaction->transaction_info->source` to identify carrier transaction) and only use their content, which are the spans we measured. +In Sentry, we need pick apart these special transactions (that we shouldn’t consider as such anymore, maybe use `transaction->transaction_info->source` to identify carrier transaction) and only use their content, which are the spans we measured. In short, we only use transactions to understand which Spans (measurements) should be aggregated. TBD: * How can we ensure that spans are packaged up efficiently, ultimately creating not to little and not too many web requests. -* As of now, some performance grouping is done based on transactions op and description. With this change they will simply act as carieers, so we need to ensure the span context has enough information that aggregation (e.g. by screen) can still be performed. +* As of now, some performance grouping is done based on transactions op and description. With this change they will simply act as carriers, so we need to ensure the span context has enough information that aggregation (e.g. by screen) can still be performed. * Profiles are right now captured based on transaction start signals, we need to find a reliable way to keep the remaining functionality. @@ -43,7 +43,7 @@ TBD: **Cons** -- We need to change how we interact with transaction in the SDK and in the product +- We need to change how we interact with transactions in the SDK and in the product. ### 2. One Transaction per Screen From 22e358a33381e14a3f9421da73feb74c9262dfda Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 3 Nov 2023 10:09:37 +0100 Subject: [PATCH 5/5] Update TBDs, fix link in readme --- README.md | 2 +- text/0117-mobile-transactions-and-spans.md | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index c5905a87..d33864fc 100644 --- a/README.md +++ b/README.md @@ -57,4 +57,4 @@ This repository contains RFCs and DACIs. Lost? - [0096-client-sampling-decision-dsc](text/0096-client-sampling-decision-dsc.md): Client Sampling Decision in Dynamic Sampling Context - [0101-revamping-the-sdk-performance-api](text/0101-revamping-the-sdk-performance-api.md): Revamping the SDK Performance API - [0106-artifact-indices](text/0106-artifact-indices.md): Improvements to Source Maps Processing -- [0118-mobiletransactionsandspans](text/0118-mobiletransactionsandspans.md): mobile-transactions-and-spans +- [0117-mobile-transactions-and-spans](text/0117-mobile-transactions-and-spans.md): Transactions and Spans for Mobile Platforms diff --git a/text/0117-mobile-transactions-and-spans.md b/text/0117-mobile-transactions-and-spans.md index af6ff5c4..3dfd8445 100644 --- a/text/0117-mobile-transactions-and-spans.md +++ b/text/0117-mobile-transactions-and-spans.md @@ -1,7 +1,7 @@ - Start Date: 2023-10-23 - RFC Type: decision - RFC PR: https://github.com/getsentry/rfcs/pull/118 -- RFC Status: draft +- RFC Status: approved # Summary @@ -30,10 +30,10 @@ As of now the way transactions are created has several drawbacks: The idea is to use transactions as carriers for measurements on mobile instead of trying to make the backend heavy concept work on mobile. There is no active "open" transaction, but we rather create transactions on-demand, whenever there is span data to send. In Sentry, we need pick apart these special transactions (that we shouldn’t consider as such anymore, maybe use `transaction->transaction_info->source` to identify carrier transaction) and only use their content, which are the spans we measured. In short, we only use transactions to understand which Spans (measurements) should be aggregated. -TBD: +Future considerations: * How can we ensure that spans are packaged up efficiently, ultimately creating not to little and not too many web requests. -* As of now, some performance grouping is done based on transactions op and description. With this change they will simply act as carriers, so we need to ensure the span context has enough information that aggregation (e.g. by screen) can still be performed. -* Profiles are right now captured based on transaction start signals, we need to find a reliable way to keep the remaining functionality. +* As of now, some performance grouping is done based on transactions op and description. With this change, transactions will simply act as carriers, thus we need to ensure the span context has enough information that aggregation (e.g. by screen) can still be performed. +* Profiles are bound to transactions via a 1:1 mapping right now, we'd need to move towards a "continous profiling" model. **Pros**