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.

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

Use brotli compression for some KP assets #64367

Merged
merged 3 commits into from
May 4, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
"@types/tar": "^4.0.3",
"JSONStream": "1.3.5",
"abortcontroller-polyfill": "^1.4.0",
"accept": "3.0.2",
"angular": "^1.7.9",
"angular-aria": "^1.7.9",
"angular-elastic": "^2.5.1",
Expand Down Expand Up @@ -310,6 +311,7 @@
"@percy/agent": "^0.26.0",
"@testing-library/react": "^9.3.2",
"@testing-library/react-hooks": "^3.2.1",
"@types/accept": "3.1.1",
"@types/angular": "^1.6.56",
"@types/angular-mocks": "^1.7.0",
"@types/babel__core": "^7.1.2",
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-optimizer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"@kbn/babel-preset": "1.0.0",
"@kbn/dev-utils": "1.0.0",
"@kbn/ui-shared-deps": "1.0.0",
"@types/compression-webpack-plugin": "^2.0.1",
"@types/estree": "^0.0.44",
"@types/loader-utils": "^1.1.3",
"@types/watchpack": "^1.1.5",
Expand All @@ -23,6 +24,7 @@
"autoprefixer": "^9.7.4",
"babel-loader": "^8.0.6",
"clean-webpack-plugin": "^3.0.0",
"compression-webpack-plugin": "^3.1.0",
"cpy": "^8.0.0",
"css-loader": "^3.4.2",
"del": "^5.1.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import Path from 'path';
import Fs from 'fs';
import Zlib from 'zlib';
import { inspect } from 'util';

import cpy from 'cpy';
Expand Down Expand Up @@ -124,17 +125,12 @@ it('builds expected bundles, saves bundle counts to metadata', async () => {
);
assert('produce zero unexpected states', otherStates.length === 0, otherStates);

expect(
Fs.readFileSync(Path.resolve(MOCK_REPO_DIR, 'plugins/foo/target/public/foo.plugin.js'), 'utf8')
).toMatchSnapshot('foo bundle');

expect(
Fs.readFileSync(Path.resolve(MOCK_REPO_DIR, 'plugins/foo/target/public/1.plugin.js'), 'utf8')
).toMatchSnapshot('1 async bundle');

expect(
Fs.readFileSync(Path.resolve(MOCK_REPO_DIR, 'plugins/bar/target/public/bar.plugin.js'), 'utf8')
).toMatchSnapshot('bar bundle');
expectFileMatchesSnapshotWithCompression('plugins/foo/target/public/foo.plugin.js', 'foo bundle');
expectFileMatchesSnapshotWithCompression(
'plugins/foo/target/public/1.plugin.js',
'1 async bundle'
);
expectFileMatchesSnapshotWithCompression('plugins/bar/target/public/bar.plugin.js', 'bar bundle');

const foo = config.bundles.find(b => b.id === 'foo')!;
expect(foo).toBeTruthy();
Expand Down Expand Up @@ -203,3 +199,24 @@ it('uses cache on second run and exist cleanly', async () => {
]
`);
});

/**
* Verifies that the file matches the expected output and has matching compressed variants.
*/
const expectFileMatchesSnapshotWithCompression = (filePath: string, snapshotLabel: string) => {
const raw = Fs.readFileSync(Path.resolve(MOCK_REPO_DIR, filePath), 'utf8');
expect(raw).toMatchSnapshot(snapshotLabel);

// Verify the brotli variant matches
expect(
// @ts-ignore @types/node is missing the brotli functions
Zlib.brotliDecompressSync(
Fs.readFileSync(Path.resolve(MOCK_REPO_DIR, `${filePath}.br`))
).toString()
).toEqual(raw);

// Verify the gzip variant matches
expect(
Zlib.gunzipSync(Fs.readFileSync(Path.resolve(MOCK_REPO_DIR, `${filePath}.gz`))).toString()
).toEqual(raw);
};
11 changes: 11 additions & 0 deletions packages/kbn-optimizer/src/worker/webpack.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import TerserPlugin from 'terser-webpack-plugin';
import webpackMerge from 'webpack-merge';
// @ts-ignore
import { CleanWebpackPlugin } from 'clean-webpack-plugin';
import CompressionPlugin from 'compression-webpack-plugin';
import * as UiSharedDeps from '@kbn/ui-shared-deps';

import { Bundle, WorkerConfig, parseDirPath, DisallowedSyntaxPlugin } from '../common';
Expand Down Expand Up @@ -319,6 +320,16 @@ export function getWebpackConfig(bundle: Bundle, worker: WorkerConfig) {
IS_KIBANA_DISTRIBUTABLE: `"true"`,
},
}),
new CompressionPlugin({
algorithm: 'brotliCompress',
filename: '[path].br',
test: /\.(js|css)$/,
}),
new CompressionPlugin({
algorithm: 'gzip',
filename: '[path].gz',
test: /\.(js|css)$/,
}),
],

optimization: {
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-ui-shared-deps/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"@kbn/i18n": "1.0.0",
"abortcontroller-polyfill": "^1.4.0",
"angular": "^1.7.9",
"compression-webpack-plugin": "^3.1.0",
"core-js": "^3.6.4",
"custom-event-polyfill": "^0.3.0",
"elasticsearch-browser": "^16.7.0",
Expand Down
11 changes: 11 additions & 0 deletions packages/kbn-ui-shared-deps/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
const Path = require('path');

const MiniCssExtractPlugin = require('mini-css-extract-plugin');
const CompressionPlugin = require('compression-webpack-plugin');
const { REPO_ROOT } = require('@kbn/dev-utils');
const webpack = require('webpack');

Expand Down Expand Up @@ -117,5 +118,15 @@ exports.getWebpackConfig = ({ dev = false } = {}) => ({
new webpack.DefinePlugin({
'process.env.NODE_ENV': dev ? '"development"' : '"production"',
}),
new CompressionPlugin({
algorithm: 'brotliCompress',
filename: '[path].br',
test: /\.(js|css)$/,
}),
new CompressionPlugin({
algorithm: 'gzip',
filename: '[path].gz',
test: /\.(js|css)$/,
}),
],
});
2 changes: 2 additions & 0 deletions renovate.json5
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,8 @@
'@types/good-squeeze',
'inert',
'@types/inert',
'accept',
joshdover marked this conversation as resolved.
Show resolved Hide resolved
'@types/accept',
],
},
{
Expand Down
12 changes: 11 additions & 1 deletion src/dev/renovate/package_groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,17 @@ export const RENOVATE_PACKAGE_GROUPS: PackageGroup[] = [
{
name: 'hapi',
packageWords: ['hapi'],
packageNames: ['hapi', 'joi', 'boom', 'hoek', 'h2o2', '@elastic/good', 'good-squeeze', 'inert'],
packageNames: [
'hapi',
'joi',
'boom',
'hoek',
'h2o2',
'@elastic/good',
'good-squeeze',
'inert',
'accept',
],
},

{
Expand Down
45 changes: 44 additions & 1 deletion src/optimize/bundles_route/dynamic_asset_response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import Fs from 'fs';
import { resolve } from 'path';
import { promisify } from 'util';

import Accept from 'accept';
import Boom from 'boom';
import Hapi from 'hapi';

Expand All @@ -37,6 +38,41 @@ const asyncOpen = promisify(Fs.open);
const asyncClose = promisify(Fs.close);
const asyncFstat = promisify(Fs.fstat);

async function tryToOpenFile(filePath: string) {
try {
return await asyncOpen(filePath, 'r');
} catch (e) {
if (e.code === 'ENOENT') {
return undefined;
} else {
throw e;
}
}
}

async function selectCompressedFile(acceptEncodingHeader: string | undefined, path: string) {
let fd: number | undefined;
let fileEncoding: 'gzip' | 'br' | undefined;

const supportedEncodings = Accept.encodings(acceptEncodingHeader, ['br', 'gzip']);

if (supportedEncodings[0] === 'br') {
fileEncoding = 'br';
fd = await tryToOpenFile(`${path}.br`);
}
if (!fd && supportedEncodings.includes('gzip')) {
fileEncoding = 'gzip';
fd = await tryToOpenFile(`${path}.gz`);
}
if (!fd) {
fileEncoding = undefined;
// Use raw open to trigger exception if it does not exist
fd = await asyncOpen(path, 'r');
}

return { fd, fileEncoding };
}

/**
* Create a Hapi response for the requested path. This is designed
* to replicate a subset of the features provided by Hapi's Inert
Expand Down Expand Up @@ -74,6 +110,7 @@ export async function createDynamicAssetResponse({
isDist: boolean;
}) {
let fd: number | undefined;
let fileEncoding: 'gzip' | 'br' | undefined;

try {
const path = resolve(bundlesPath, request.params.path);
Expand All @@ -86,7 +123,7 @@ export async function createDynamicAssetResponse({
// we use and manage a file descriptor mostly because
// that's what Inert does, and since we are accessing
// the file 2 or 3 times per request it seems logical
fd = await asyncOpen(path, 'r');
({ fd, fileEncoding } = await selectCompressedFile(request.headers['accept-encoding'], path));

const stat = await asyncFstat(fd);
const hash = isDist ? undefined : await getFileHash(fileHashCache, path, stat, fd);
Expand All @@ -113,6 +150,12 @@ export async function createDynamicAssetResponse({
response.header('cache-control', 'must-revalidate');
}

// If we manually selected a compressed file, specify the encoding header.
// Otherwise, let Hapi automatically gzip the response.
if (fileEncoding) {
response.header('content-encoding', fileEncoding);
}

return response;
} catch (error) {
if (fd) {
Expand Down
73 changes: 73 additions & 0 deletions test/functional/apps/bundles/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you 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.
*/

/**
* These supertest-based tests live in the functional test suite because they depend on the optimizer bundles being built
* and served
*/
export default function({ getService }) {
const supertest = getService('supertest');

describe('bundle compression', function() {
this.tags('ciGroup12');

let buildNum;
before(async () => {
const resp = await supertest.get('/api/status').expect(200);
buildNum = resp.body.version.build_number;
});

it('returns gzip files when client only supports gzip', () =>
supertest
// We use the kbn-ui-shared-deps for these tests since they are always built with br compressed outputs,
// even in dev. Bundles built by @kbn/optimizer are only built with br compression in dist mode.
.get(`/${buildNum}/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.js`)
.set('Accept-Encoding', 'gzip')
.expect(200)
.expect('Content-Encoding', 'gzip'));

it('returns br files when client only supports br', () =>
supertest
.get(`/${buildNum}/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.js`)
.set('Accept-Encoding', 'br')
.expect(200)
.expect('Content-Encoding', 'br'));

it('returns br files when client only supports gzip and br', () =>
supertest
.get(`/${buildNum}/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.js`)
.set('Accept-Encoding', 'gzip, br')
.expect(200)
.expect('Content-Encoding', 'br'));

it('returns gzip files when client prefers gzip', () =>
supertest
.get(`/${buildNum}/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.js`)
.set('Accept-Encoding', 'gzip;q=1.0, br;q=0.5')
.expect(200)
.expect('Content-Encoding', 'gzip'));

it('returns gzip files when no brotli version exists', () =>
supertest
.get(`/${buildNum}/bundles/commons.style.css`) // legacy optimizer does not create brotli outputs
.set('Accept-Encoding', 'gzip, br')
.expect(200)
.expect('Content-Encoding', 'gzip'));
});
}
3 changes: 2 additions & 1 deletion test/functional/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ export default async function({ readConfigFile }) {

return {
testFiles: [
require.resolve('./apps/bundles'),
require.resolve('./apps/console'),
require.resolve('./apps/getting_started'),
require.resolve('./apps/context'),
require.resolve('./apps/dashboard'),
require.resolve('./apps/discover'),
require.resolve('./apps/getting_started'),
require.resolve('./apps/home'),
require.resolve('./apps/management'),
require.resolve('./apps/saved_objects_management'),
Expand Down
2 changes: 2 additions & 0 deletions test/functional/services/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { ToastsProvider } from './toasts';
import { PieChartProvider } from './visualizations';
import { ListingTableProvider } from './listing_table';
import { SavedQueryManagementComponentProvider } from './saved_query_management_component';
import { KibanaSupertestProvider } from './supertest';

export const services = {
...commonServiceProviders,
Expand Down Expand Up @@ -83,4 +84,5 @@ export const services = {
toasts: ToastsProvider,
savedQueryManagementComponent: SavedQueryManagementComponentProvider,
elasticChart: ElasticChartProvider,
supertest: KibanaSupertestProvider,
};
29 changes: 29 additions & 0 deletions test/functional/services/supertest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you 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 { FtrProviderContext } from 'test/functional/ftr_provider_context';
import { format as formatUrl } from 'url';

import supertestAsPromised from 'supertest-as-promised';

export function KibanaSupertestProvider({ getService }: FtrProviderContext) {
const config = getService('config');
const kibanaServerUrl = formatUrl(config.get('servers.kibana'));
return supertestAsPromised(kibanaServerUrl);
}
Loading