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

Add new app context that tells if the app is in the background or foreground #36

Closed
6 tasks done
marandaneto opened this issue Jul 20, 2022 · 12 comments
Closed
6 tasks done

Comments

@marandaneto
Copy link

marandaneto commented Jul 20, 2022

This field should be queriable in the discover dashboard.
Today people rely on was active: yes|no but it's not 100% correct and aligned between platforms.
Other option is looking into breadcrumbs, but that is not queriable in the discover dashboard.

We could add a tag but afaik SDKs should not add tags themselves.

main flag.

Preview Give feedback

Related issues for surfacing on product:

@adinauer
Copy link
Member

adinauer commented Aug 3, 2022

was active tells whether it was main thread

Add to context first then talk to search and storage for features on top of it.

@markushi
Copy link
Member

Having looked a bit further into this I think we should introduce a new attribute for this, as the existing ones do not cover this use case.

Existing Protocol

Quoting @marandaneto this issue comment describes the thread attributes best

current: if the thread was scheduled/running or not, as the docs say: A flag indicating whether the thread was in the foreground
crashed: if it was the thread that captured the event, either a message or exception, it's just a bad/legacy naming, it does not mean that this thread threw an exception or crashed the App.

Existing Front End

Here we have a was active label which is shown when thread->current evaluates true, see implementation

@marandaneto
Copy link
Author

Yes, it needs to be a new field.
Consider looking into https://www.notion.so/sentry/Thread-isCurrent-UIThread-etc-9ad82725c4af412f950e92acf1e6eba4 and #47 as well.
Ideally, this would be done together, because all the fields are a bit confusing, and adding a new one would make it even more, if not properly named and placed in the right place.

@marandaneto
Copy link
Author

@markushi getsentry/develop#728 helps for telling if the thread is the ui thread or not.
I believe that for this issue, it requires also a new field in the app protocol https://develop.sentry.dev/sdk/event-payloads/contexts/#app-context
Transactions don't have threads, and this info would be useful for transactions as well.

@markushi
Copy link
Member

As far as I can see we need some front-end changes for this as well:

app.in_foreground
Should automatically be visible, as all unknown keys are handled here, probably should be added to the list of known keys at some point.

thread.main
As far as I can see this needs some more work to appear, both on js front-end and backend.

@marandaneto
Copy link
Author

Sentry PR getsentry/sentry#43964

@marandaneto
Copy link
Author

UI search keys getsentry/sentry#47446
Docs getsentry/sentry#46183

@marandaneto marandaneto moved this from In Progress to Backlog in Mobile & Cross Platform SDK Apr 18, 2023
@marandaneto marandaneto removed their assignment Apr 18, 2023
@marandaneto
Copy link
Author

SDKs and BE work is done, the only missing bits now is the FE, to show those properties on the UI.
#198
getsentry/sentry#44251
Since MDX is no more, is there someone that could take this?

@romtsn
Copy link
Member

romtsn commented Apr 20, 2023

Btw for getsentry/sentry#44251 we already display In Foreground in the contexts section
image

I guess the question is if we also want to promote it as tag and display in the tags section. If not, we could close the issue

@marandaneto
Copy link
Author

The in-foreground label is there, but the issue is still open because of #198
We could either rename the title of this issue, or close and open a new one?

We should not promote anything as a tag, the info is in the event already, and the FE should just display it elsewhere.
That field is already indexed, no need to be a tag.

@kahest what's the approach we should take when everything on our end is done, but it depends on external work? this is a good example, this issue will remain open while the other team doesn't tackle the 2 issues left (FE work only), although they are created under the sentry repo already.

@kahest
Copy link
Member

kahest commented Apr 21, 2023

Perfectly fine to close this on our end, no need to change the title IMO. The FE related items are clearly pointed out.

@kahest kahest closed this as completed Apr 21, 2023
@github-project-automation github-project-automation bot moved this from Backlog to Done in Mobile & Cross Platform SDK Apr 21, 2023
@kahest
Copy link
Member

kahest commented Apr 21, 2023

I'll check who can take on the FE tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

5 participants