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.

Sign up for GitHub

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

Jump to bottom

convert perf session to use aqs support session id #6832

Merged
merged 1 commit into from
Apr 7, 2025
Merged

convert perf session to use aqs support session id #6832

merged 1 commit into from
Apr 7, 2025
+138 −102

Conversation

themiswang
Copy link
Contributor

@themiswang themiswang commented Apr 2, 2025 •
edited
Loading

  • Override PerfSession session id to aqs session id when possible
  • Add assertion for debug mode to double check if aqs session id available by the time of use

@google-oss-bot
Copy link
Contributor

1 Warning
⚠️ Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 2, 2025 •
edited
Loading

Coverage Report 1

Affected Products

No changes between base commit (54f5602) and merge commit (0efc1dc).

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/nbmzxyIs5w.html

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 2, 2025 •
edited
Loading

Test Results

4 files  4 suites   13s ⏱️
4 tests 3 ✅ 0 💤 1 ❌
8 runs  7 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit c5a64b6.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 2, 2025 •
edited
Loading

Size Report 1

Affected Products

  • base

    TypeBase (54f5602)Merge (0efc1dc)Diff
    apk (aggressive)?8.80 kB? (?)
    apk (release)?9.77 kB? (?)

  • firebase-perf

    TypeBase (54f5602)Merge (0efc1dc)Diff
    aar?319 kB? (?)
    apk (aggressive)?1.62 MB? (?)
    apk (release)?10.3 MB? (?)

  • firebase-sessions

    TypeBase (54f5602)Merge (0efc1dc)Diff
    aar?208 kB? (?)
    apk (aggressive)?539 kB? (?)
    apk (release)?5.78 MB? (?)

  • protolite-well-known-types

    TypeBase (54f5602)Merge (0efc1dc)Diff
    aar?1.02 MB? (?)
    apk (aggressive)?134 kB? (?)
    apk (release)?1.71 MB? (?)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/x8EbeWSkIP.html

@themiswang themiswang force-pushed the aqsPerfSession branch 3 times, most recently from 2dd5f1e to 720f1c1 Compare April 2, 2025 20:10
@themiswang themiswang marked this pull request as ready for review April 2, 2025 20:12
@themiswang themiswang requested review from mrober and tejasd April 2, 2025 20:12
tejasd
tejasd reviewed Apr 2, 2025
View reviewed changes
firebase-perf/src/test/java/com/google/firebase/perf/session/PerfSessionTest.java
@@ -62,7 +62,7 @@ public void setUp() {

@Test
public void instanceCreation() {
PerfSession session = new PerfSession("sessionId", mockClock);
PerfSession session = new PerfSession("sessionId", mockClock, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment in FirebasePerfSessionSubscriber. I'm not sure passing this in the constructor is a good idea right now - but I will defer it to @mrober .

@tejasd tejasd mentioned this pull request Apr 3, 2025
Open
@themiswang themiswang merged commit 1e8b629 into fireperf-aqs Apr 7, 2025
25 of 32 checks passed
@themiswang themiswang deleted the aqsPerfSession branch April 7, 2025 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tejasd tejasd tejasd left review comments

@mrober mrober mrober approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@themiswang @google-oss-bot @tejasd @mrober