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: Add platforms to events and frames #1560

Merged
merged 3 commits into from
Nov 29, 2024
Merged

Conversation

loewenheim
Copy link
Contributor

This adds platform values to all stack frame and symbolication request types and endpoints.

We're intentionally using Platform everywhere instead of the more narrow types appropriate to native, JS, or JVM, just in case Sentry sends an unexpected value.

This adds `platform` values to all stack frame
and symbolication request types and endpoints.

We're intentionally using `Platform` everywhere
instead of the more narrow types appropriate to
native, JS, or JVM, just in case Sentry sends an
unexpected value.
@loewenheim loewenheim self-assigned this Nov 28, 2024
@loewenheim loewenheim requested review from Swatinem and a team November 28, 2024 11:47
Copy link
Member

@jjbayer jjbayer left a comment

Choose a reason for hiding this comment

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

Is this preparation for when we decide what type of symbolication to run on a frame-by-frame basis?

@loewenheim
Copy link
Contributor Author

Exactly. Before we implement any new behavior, I want Sentry to just send these platform values and make sure we can actually use them to base decisions on.

@@ -38,6 +38,7 @@ impl ConfigureScope for SymbolicationRequestQueryParams {
/// JSON body of the symbolication request.
#[derive(Serialize, Deserialize)]
pub struct SymbolicationRequestBody {
pub platform: Option<Platform>,
Copy link
Member

Choose a reason for hiding this comment

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

I believe this should also have a #[serde(default)]?

Copy link
Member

Choose a reason for hiding this comment

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

#[serde(default)] is already implied for Option.

@loewenheim loewenheim merged commit 8faeb5c into master Nov 29, 2024
13 checks passed
@loewenheim loewenheim deleted the feat/event-frame-platform branch November 29, 2024 11:58
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.

4 participants