Skip to content

[HOLD for payment 2024-07-10] [HOLD for payment 2024-07-02] [HOLD for payment 2024-06-28] [HOLD for payment 2024-06-28] MEDIUM: Enable Function Symbolication for Detailed Performance Tracing #43649

Closed

Description

@muttmuure

Problem

Without symbols in our profile traces we can only guess which functions took so much time, based on the bottom-up stack (and try to manually map the amount of loops & internals to what we have in code).

Solution

Properly symbolicate the data in the performance profile feature into human-readable function names, line numbers, and file names so our team can identify the cause of slows in app.

Issue OwnerCurrent Issue Owner: @greg-schroeder

Activity

  1. converted this from a draft issue on Jun 13, 2024
  2. melvin-bot commented on Jun 13, 2024

    @melvin-bot

    Triggered auto assignment to @grgia (AutoAssignerNewDotQuality)

  3. muttmuure commented on Jun 13, 2024

    @muttmuure
    ContributorAuthor

    @grgia I think this is likely one that an agency contributor will need to take the lead on.

    @Szymon20000 @adhorodyski can you give us a path forward on achieving symbolication in our current profile tracing?

  4. adhorodyski commented on Jun 13, 2024

    @adhorodyski
    Contributor

    @Szymon20000 as far as I remember the ones we used to receive with different bug reports were already symbolicated correctly.

    Do you think anything could have changed in this regard lately for the react-native-release-profiler?

    Here's one from 1 day ago that I couldn't symbolicate myself against latest main (same on @hurali97's end). We don't know (yet) the git tag for the build so it's not possible to match it against a source map file manually either.
    Profile_trace_for_1.4.81-9.cpuprofile

    And that's the same one after matching it against this branch:
    image

  5. hannojg commented on Jun 17, 2024

    @hannojg
    Contributor

    Hey, Hanno from Margelo here. I will look into checking why we have no source maps using rn-release-profiler in expensify. Feel free to assign to me! 😊

  6. hannojg commented on Jun 17, 2024

    @hannojg
    Contributor

    The traces we record on the device are unsymbolicated. We need a way to symbolicate them. The flow would be:

    1. A user shares a profile trace
    2. A developer receives the trace
    3. The developer runs a script to symbolicate it

    We already have a script for symbolication in place. The issue however is that the profile the user created is specific to an app version. And the script only symbolicates for the current commit we are on.
    Additionally, switching versions as developer just to symbolicate the trace is quite the overhead (as a new release build needs to be created).

    So a better solution would be to:

    1. Add a GitHub action that runs when we create a new release version and uploads it as artefact to the GitHub action workflow run (also for the staging app versions).
    2. Update our script to:
      1. Get the app version from the file name of the trace
      2. Download the artefact from the GitHub action runs for that version

    I can help with implementing that.

  7. grgia commented on Jun 17, 2024

    @grgia
    Contributor

    @hannojg, thanks for looking into this. Shall I assign you this one?

  8. hannojg commented on Jun 17, 2024

    @hannojg
    Contributor

    Yes please, I'd go ahead and create two PRs, that will:

    1. Upload a source map when we make a new release version
    2. Update our scripts (and documentation) for symbolicating a trace using the uploaded source maps
  9. hannojg commented on Jun 17, 2024

    @hannojg
    Contributor

    Update for step 1 uploading the source maps:

    • I found that we already upload source maps, support was added in
    • However, its broken and not uploading for android and iOS
    • Additionally it would be nice if we can link the assets with the tag (have to see if that works)
      • Update: doesn't work 😅

    So I will fix the existing system first

    Screenshot 2024-06-17 at 10 18 41

  10. 33 remaining items

  11. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jul 1, 2024
  12. melvin-bot commented on Jul 1, 2024

    @melvin-bot

    Triggered auto assignment to @greg-schroeder (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

  13. grgia commented on Jul 1, 2024

    @grgia
    Contributor

    closing as complete, reviews were internal

  14. changed the title [-][HOLD for payment 2024-07-02] [HOLD for payment 2024-06-28] [HOLD for payment 2024-06-28] MEDIUM: Enable Function Symbolication for Detailed Performance Tracing[/-] [+][HOLD for payment 2024-07-10] [HOLD for payment 2024-07-02] [HOLD for payment 2024-06-28] [HOLD for payment 2024-06-28] MEDIUM: Enable Function Symbolication for Detailed Performance Tracing[/+] on Jul 3, 2024
  15. melvin-bot commented on Jul 3, 2024

    @melvin-bot

    The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.3-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

    If no regressions arise, payment will be issued on 2024-07-10. 🎊

    For reference, here are some details about the assignees on this issue:

    • @hannojg does not require payment (Contractor)
  16. melvin-bot commented on Jul 3, 2024

    @melvin-bot

    BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

    • [@grgia] The PR that introduced the bug has been identified. Link to the PR:
    • [@grgia] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
    • [@grgia] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
    • [@hannojg] Determine if we should create a regression test for this bug.
    • [@hannojg] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
    • [@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

AutoAssignerNewDotQualityUsed to assign quality issues to engineersAwaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.TaskWeeklyKSv2

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions