Skip to content

Remove the logging of GaugeMetadata to allow using AQS #6678

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
Merged
merged 5 commits into from
Feb 7, 2025
Merged

Remove the logging of GaugeMetadata to allow using AQS #6678

merged 5 commits into from
Feb 7, 2025

Conversation

tejasd
Copy link
Contributor

@tejasd tejasd commented Feb 6, 2025 •
edited
Loading

Based on the behaviour of AQS w/ Fireperf, an AQS session isn't available when (currently) logging gauge metadata.

Changes:

  • Remove the current logging of gauge metadata - will be re-introduced in a future PR.
  • Switch Gauge collection from scheduleAtFixedRate to scheduleWithFixedDelay. As documented, this should prevent a potentially large amounts of gauge collection if a process is cached, and then restored during a verbose session - which should make it work better w/ AQS.
  • Remove API restricted behaviour which is no longer relevant.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 6, 2025

Vertex AI Mock Responses Check ⚠️

A newer major version of the mock responses for Vertex AI unit tests is available. update_responses.sh should be updated to clone the latest version of the responses: v6.1

@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 Feb 6, 2025 •
edited
Loading

Coverage Report 1

Affected Products

  • firebase-perf

    Overall coverage changed from 70.60% (b8803fc) to 70.59% (6b37c39) by -0.01%.

    FilenameBase (b8803fc)Merge (6b37c39)Diff
    CpuGaugeCollector.java92.77%93.83%+1.06%
    GaugeManager.java98.40%98.37%-0.03%
    GaugeMetadataManager.java77.78%100.00%+22.22%

Test Logs

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 6, 2025 •
edited
Loading

Test Results

  110 files   -   924    110 suites   - 924   2m 12s ⏱️ - 32m 16s
  966 tests  - 4 922    966 ✅  - 4 900  0 💤  - 22  0 ❌ ±0 
1 940 runs   - 9 899  1 940 ✅  - 9 855  0 💤  - 44  0 ❌ ±0 

Results for commit c2e355e. ± Comparison against base commit b8803fc.

This pull request removes 4923 and adds 1 tests. Note that renamed tests count towards both. 
com.google.android.datatransport.cct.CctBackendFactoryTest ‑ create_returnCCTBackend_WhenBackendNameIsCCT
com.google.android.datatransport.cct.CctDestinationTest ‑ cctDestination_shouldOnlySupportProtoAndJson
com.google.android.datatransport.cct.CctDestinationTest ‑ cctDestination_shouldSupportProtoAndJson
com.google.android.datatransport.cct.CctTransportBackendTest ‑ decorate_whenOffline_shouldProperlyPopulateNetworkInfo
com.google.android.datatransport.cct.CctTransportBackendTest ‑ decorate_whenOnline_shouldProperlyPopulateNetworkInfo
com.google.android.datatransport.cct.CctTransportBackendTest ‑ schedule_shouldAddCookieOnPseudonymousIds
com.google.android.datatransport.cct.CctTransportBackendTest ‑ schedule_shouldDropCookieOnMixedPseudonymousIds
com.google.android.datatransport.cct.CctTransportBackendTest ‑ send_CompressedResponseIsUncompressed
com.google.android.datatransport.cct.CctTransportBackendTest ‑ send_whenBackendRedirectsMoreThan5Times_shouldOnlyRedirect4Times
com.google.android.datatransport.cct.CctTransportBackendTest ‑ send_whenBackendRedirects_shouldCorrectlyFollowTheRedirectViaPost
…

com.google.firebase.perf.session.SessionManagerTest ‑ setApplicationContext_initializeGaugeMetadataManager

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 6, 2025 •
edited
Loading

Size Report 1

Affected Products

  • firebase-perf

    TypeBase (b8803fc)Merge (6b37c39)Diff
    aar316 kB315 kB-1.36 kB (-0.4%)
    apk (aggressive)1.63 MB1.63 MB-408 B (-0.0%)
    apk (release)10.1 MB10.1 MB-848 B (-0.0%)

Test Logs

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

@tejasd tejasd changed the title Remove the logging of GaugeMetadata based on AppStart Remove the logging of GaugeMetadata to allow using AQS Feb 6, 2025
@tejasd tejasd marked this pull request as ready for review February 7, 2025 16:05
@tejasd tejasd requested a review from mrober February 7, 2025 16:05
@tejasd tejasd merged commit 9c03b7b into fireperf-aqs Feb 7, 2025
31 checks passed
@tejasd tejasd deleted the td/fireperf-gauge-metadata branch February 7, 2025 19:58
@tejasd tejasd mentioned this pull request Feb 10, 2025
Merged
mrober pushed a commit that referenced this pull request Mar 7, 2025
@tejasd @mrober
Remove the logging of GaugeMetadata to allow using AQS (#6678)
0e58ef3 
Based on the behaviour of AQS w/ Fireperf, an AQS session isn't
available when (currently) logging gauge metadata.

Changes:
- Remove the current logging of gauge metadata - will be re-introduced
in a future PR.
- Switch Gauge collection from `scheduleAtFixedRate` to
`scheduleWithFixedDelay`. As
[documented](https://stackoverflow.com/a/78405653), this *should*
prevent a potentially large amounts of gauge collection if a process is
cached, and then restored during a verbose session - which *should* make
it work better w/ AQS.
- Remove API restricted behaviour which is no longer relevant.
@firebase firebase locked and limited conversation to collaborators Mar 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mrober mrober Awaiting requested review from mrober

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

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