Skip to content

Add bundle definitions and its measurement script. #5706

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 6 commits into from
Nov 12, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .github/workflows/health-metrics-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@ name: Health Metrics
on: [push, pull_request]

env:
METRICS_SERVICE_URL: ${{ secrets.METRICS_SERVICE_URL }}
GITHUB_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }}
# TODO(yifany): parse from git commit history directly
# Reason: actions/checkout@v2 does not always honor ${{ github.event.pull_request.base.sha }},
# therefore "base.sha" sometimes is not the commit that actually gets merged with the
# pull request head commit for CI test.
# See:
# - https://github.com/actions/checkout/issues/27
# - https://github.com/actions/checkout/issues/237
GITHUB_PULL_REQUEST_BASE_SHA: ${{ github.event.pull_request.base.sha }}
NODE_OPTIONS: "--max-old-space-size=4096"

Expand Down
104 changes: 104 additions & 0 deletions repo-scripts/size-analysis/analyze-all-bundles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* @license
* Copyright 2021 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as fs from 'fs';
import * as path from 'path';
import * as tmp from 'tmp';

import { Bundler, Mode, run as runBundleAnalysis } from './bundle-analysis';
import { Report } from '../../scripts/size_report/report_binary_size';

/**
* Runs bundle analysis for all bundle definition files under:
*
* `firebase-js-sdk/repo-scripts/size-analysis/bundle-definitions`
*
* The method accepts an optional parameter `version`:
* 1. when presented (for example, `version` = '9.0.0'), this method measures the bundle size by
* building test bundles with dependencies at that specific version downloaded from npm
* 2. when omitted (in this case, `version` = null), this method measures the bundle size by
* building test bundles with dependencies from local artifacts (e.g. produced by `yarn build`)
*
* #1 is intended only for manual runs for the purpose of back-filling historical size data. #2 is
* intended for CI runs that measure size for the current commit.
*
* More details on how a test bundle is built can be found in `bundle-analysis.ts`.
*
* @param {string} [version] - If present, the SDK version to run measurement against
* @returns {Promise<Report[]>} A list of bundle size measurements
Copy link
Member

Choose a reason for hiding this comment

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

We use tsdoc instead of jsdoc. Please update. You can take a look at any of our packages for reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that just a standard of doc format or something I should use to generate the doc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Changed to tsdoc format.

*/
export async function generateReportForBundles(version?: string) {
const definitionDir = `${__dirname}/bundle-definitions`;
const outputDir = tmp.dirSync().name;
console.log(`Bundle definitions are located at "${definitionDir}".`);
console.log(`Analysis output are located at "${outputDir}".`);

const bundles = fs.readdirSync(definitionDir);
const results: Report[] = [];
for (const bundle of bundles) {
const product = path.basename(bundle, '.json');
const output = `${outputDir}/${product}.analysis.json`;
if (version) {
overwriteVersion(definitionDir, bundle, outputDir, version);
}
const option = {
input: version ? `${outputDir}/${bundle}` : `${definitionDir}/${bundle}`,
bundler: Bundler.Rollup,
mode: version ? Mode.Npm : Mode.Local,
output: output,
debug: true
};
console.log(`Running for bundle "${bundle}" with mode "${option.mode}".`);
await runBundleAnalysis(option);
const measurements = parseAnalysisOutput(product, output);
results.push(...measurements);
}
console.log(results);
return results;
}

function overwriteVersion(
definitionDir: string,
bundle: string,
temp: string,
version: string
) {
const definitions = JSON.parse(
fs.readFileSync(`${definitionDir}/${bundle}`, { encoding: 'utf-8' })
);
for (const definition of definitions) {
const dependencies = definition.dependencies;
for (const dependency of dependencies) {
dependency.versionOrTag = version;
}
}
fs.writeFileSync(`${temp}/${bundle}`, JSON.stringify(definitions, null, 2), {
encoding: 'utf-8'
});
}

function parseAnalysisOutput(product: string, output: string) {
const analyses = JSON.parse(fs.readFileSync(output, { encoding: 'utf-8' }));
const results: Report[] = [];
for (const analysis of analyses) {
const sdk = 'bundle';
Copy link
Member

Choose a reason for hiding this comment

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

What does bundle mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The name we put here will be appear as the name of a new section under "affected sdks" in a PR report. For example, there is another section named "bundle" in the report above in this PR [1]. We can put whatever name we want here.

[1] #5706 (comment)

I suppose it is a limitation of the metric backend at the moment. When the API for receiving size measurements was first designed, the backend only recognizes http requests with a json body strictly in this format:

[
  {
    sdk: <some-string>,
    type: <some-string>,
    value: <some-integer>
  },
  ......
]

Writing a new API in the backend and deploying a new version of the service to GCP can be some work, therefore I'm reusing the API for bundle measurements here, although the semantic of the API doesn't make any sense in the context of "bundle-analysis". What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine. It would be great if you can copy your comment here to the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some comment in the code to explain the rationale here.

const value = analysis.results[0].size;
const type = `${product} (${analysis.name})`;
results.push({ sdk, type, value });
}
return results;
}
4 changes: 2 additions & 2 deletions repo-scripts/size-analysis/bundle-analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ interface SubModuleImport {
imports: string[];
}

enum Bundler {
export enum Bundler {
Rollup = 'rollup',
Webpack = 'webpack',
Both = 'both'
}

enum Mode {
export enum Mode {
Npm = 'npm',
Local = 'local'
}
Expand Down

This file was deleted.

32 changes: 32 additions & 0 deletions repo-scripts/size-analysis/bundle-definitions/analytics.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
[
{
"name": "logEvent",
"dependencies": [
{
"packageName": "firebase",
"versionOrTag": "latest",
"imports": [
{
"path": "app",
"imports": [
"initializeApp"
]
}
]
},
{
"packageName": "firebase",
"versionOrTag": "latest",
"imports": [
{
"path": "analytics",
"imports": [
"getAnalytics",
"logEvent"
]
}
]
}
]
}
]
95 changes: 95 additions & 0 deletions repo-scripts/size-analysis/bundle-definitions/app-check.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
[
{
"name": "ReCaptchaV3Provider",
"dependencies": [
{
"packageName": "firebase",
"versionOrTag": "latest",
"imports": [
{
"path": "app",
"imports": [
"initializeApp"
]
}
]
},
{
"packageName": "firebase",
"versionOrTag": "latest",
"imports": [
{
"path": "app-check",
"imports": [
"initializeAppCheck",
Copy link
Contributor

Choose a reason for hiding this comment

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

Did these used to include getToken? I thought I remembered seeing them. It makes sense because if app check is being used, getToken() will eventually be called, if not by the user then by another library (storage, functions, etc).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it was there before. I got confused this morning when reading the code sample for App Check Enterprise, which doesn't reference getToken, and I wondered why it was there. I forgot that it was actually Feiyang who added getToken for me in the bundle doc last week. I was actually about to ask you, but got stuck in some changeset warnings in this pull request.

Anyway, I have added it back both in the bundle definition doc and the json files in this pull request. Please take a look. Thanks!

"ReCaptchaV3Provider",
"getToken"
]
}
]
}
]
},
{
"name": "ReCaptchaEnterpriseProvider",
"dependencies": [
{
"packageName": "firebase",
"versionOrTag": "latest",
"imports": [
{
"path": "app",
"imports": [
"initializeApp"
]
}
]
},
{
"packageName": "firebase",
"versionOrTag": "latest",
"imports": [
{
"path": "app-check",
"imports": [
"initializeAppCheck",
"ReCaptchaEnterpriseProvider",
"getToken"
]
}
]
}
]
},
{
"name": "CustomProvider",
"dependencies": [
{
"packageName": "firebase",
"versionOrTag": "latest",
"imports": [
{
"path": "app",
"imports": [
"initializeApp"
]
}
]
},
{
"packageName": "firebase",
"versionOrTag": "latest",
"imports": [
{
"path": "app-check",
"imports": [
"initializeAppCheck",
"CustomProvider",
"getToken"
]
}
]
}
]
}
]
Loading