Skip to content

Remove the capability of sending events to clearcut from the Performance SDK. #3086

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

Merged
merged 3 commits into from
May 20, 2020

Conversation

visumickey
Copy link
Contributor

Remove the capability of sending events to clearcut from the Performance SDK. This removes the need for fetching the RC configs to decide on the dispatch destination - so that is removed in the settings service and remote config service.

…nce SDK. This removes the need for fetching the RC configs to decide on the dispatch destination - so that is removed in the settings service and remote config service.
@google-oss-bot
Copy link
Contributor

google-oss-bot commented May 19, 2020

Binary Size Report

Affected SDKs

  • @firebase/firestore

    Type Base (1325b58) Head (1b29fc7) Diff
    browser 251 kB 251 kB +50 B (+0.0%)
    esm2017 194 kB 194 kB +26 B (+0.0%)
    main 492 kB 492 kB +89 B (+0.0%)
    module 248 kB 248 kB +50 B (+0.0%)
  • @firebase/firestore/memory

    Type Base (1325b58) Head (1b29fc7) Diff
    browser 191 kB 191 kB +50 B (+0.0%)
    esm2017 149 kB 149 kB +26 B (+0.0%)
    main 368 kB 368 kB +89 B (+0.0%)
    module 189 kB 189 kB +50 B (+0.0%)
  • @firebase/performance

    Type Base (1325b58) Head (1b29fc7) Diff
    browser 28.1 kB 26.7 kB -1.43 kB (-5.1%)
    esm2017 26.1 kB 24.7 kB -1.40 kB (-5.4%)
    main 28.1 kB 26.7 kB -1.43 kB (-5.1%)
    module 27.9 kB 26.4 kB -1.43 kB (-5.1%)
  • firebase

    Type Base (1325b58) Head (1b29fc7) Diff
    firebase-firestore.js 289 kB 289 kB +48 B (+0.0%)
    firebase-firestore.memory.js 231 kB 231 kB +48 B (+0.0%)
    firebase-performance-standalone.es2017.js 72.7 kB 71.9 kB -733 B (-1.0%)
    firebase-performance-standalone.js 48.1 kB 47.4 kB -765 B (-1.6%)
    firebase-performance.js 38.5 kB 37.7 kB -765 B (-2.0%)
    firebase.js 823 kB 822 kB -717 B (-0.1%)

Test Logs

@ramanpreetSinghKhinda
Copy link

Quick thought: The codebase doesn't mention the name clearcut but the PR mentions. If the purpose is to hide the actual name than we need to create another PR and abandon this one.

@ramanpreetSinghKhinda
Copy link

LGTM on a high level. I am not able to approve it though. May be only code owners can do that.

@visumickey visumickey requested a review from zijianjoy May 20, 2020 00:45
Copy link
Contributor

@alikn alikn left a comment

Choose a reason for hiding this comment

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

Thanks for the clean up!
Can you please build and try on a test app to make sure logs are sent fine before merging?

@visumickey visumickey merged commit 7672638 into master May 20, 2020
@visumickey visumickey deleted the removeClearcutFromPerf branch May 20, 2020 05:38
@firebase firebase locked and limited conversation to collaborators Jun 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants