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

Support brotli compression on the server side #142334

Merged
merged 17 commits into from
Oct 24, 2022
Merged

Support brotli compression on the server side #142334

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5e1fd7b
Use brotli compression
rockdaboot Sep 29, 2022
0f0abfc
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Sep 30, 2022
127ede2
Add integration test for brotli support
rockdaboot Sep 30, 2022
226400e
Use import instead of require()
rockdaboot Sep 30, 2022
9972c19
Suppress build error on importing brok
rockdaboot Sep 30, 2022
3696ef6
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Sep 30, 2022
bf59350
add brok as explicit package dep
pgayvallet Oct 3, 2022
23c7ad0
Merge remote-tracking branch 'upstream/main' into rockdaboot/use-brot…
pgayvallet Oct 3, 2022
a7c30a8
Merge branch 'main' into rockdaboot/use-brotli-compression
kibanamachine Oct 5, 2022
5c94383
Merge remote-tracking branch 'upstream/main' into rockdaboot/use-brot…
pgayvallet Oct 20, 2022
9b6009c
add `server.compression.brotli` config settings
pgayvallet Oct 20, 2022
40283bf
update documentation
pgayvallet Oct 20, 2022
74ecbdb
fix test utils
pgayvallet Oct 20, 2022
40fd9b6
fix more test configs
pgayvallet Oct 21, 2022
e1e40d0
Merge remote-tracking branch 'upstream/main' into rockdaboot/use-brot…
pgayvallet Oct 24, 2022
a310d06
add tests for endpoints too
pgayvallet Oct 24, 2022
4eb6900
remove against endpoint for now
pgayvallet Oct 24, 2022
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
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@
"base64-js": "^1.3.1",
"bitmap-sdf": "^1.0.3",
"brace": "0.11.1",
"brok": "^5.0.2",
"byte-size": "^8.1.0",
"canvg": "^3.0.9",
"cbor-x": "^1.3.3",
Expand Down
2 changes: 2 additions & 0 deletions packages/core/http/core-http-server-internal/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ RUNTIME_DEPS = [
"@npm//@hapi/cookie",
"@npm//@hapi/inert",
"@npm//elastic-apm-node",
"@npm//brok",
"//packages/kbn-utils",
"//packages/kbn-std",
"//packages/kbn-config-schema",
Expand All @@ -68,6 +69,7 @@ TYPES_DEPS = [
"@npm//moment",
"@npm//@elastic/numeral",
"@npm//lodash",
"@npm//brok",
"@npm//@hapi/hapi",
"@npm//@hapi/boom",
"@npm//@hapi/cookie",
Expand Down
8 changes: 8 additions & 0 deletions packages/core/http/core-http-server-internal/src/http_server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import type { Duration } from 'moment';
import { firstValueFrom, Observable } from 'rxjs';
import { take } from 'rxjs/operators';
import apm from 'elastic-apm-node';
// @ts-expect-error no type definition
import Brok from 'brok';
import type { Logger, LoggerFactory } from '@kbn/logging';
import type { InternalExecutionContextSetup } from '@kbn/core-execution-context-server-internal';
import { isSafeMethod } from '@kbn/core-http-router-server-internal';
Expand Down Expand Up @@ -149,6 +151,12 @@ export class HttpServer {
const listenerOptions = getListenerOptions(config);
this.server = createServer(serverOptions, listenerOptions);
await this.server.register([HapiStaticFiles]);
await this.server.register({
plugin: Brok,
options: {
compress: { quality: 3 },
},
});
this.config = config;

// It's important to have setupRequestStateAssignment call the very first, otherwise context passing will be broken.
Expand Down
9 changes: 9 additions & 0 deletions test/api_integration/apis/core/compression.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,14 @@ export default function ({ getService }: FtrProviderContext) {
expect(response.header).not.to.have.property('content-encoding');
});
});

it(`supports brotli compression`, async () => {
await supertest
.get('/app/kibana')
Copy link
Member

Choose a reason for hiding this comment

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

Nit: should we explicitly test it with an API? Just in case /app and /api are served differently (as we do today with bundles)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, all these compression.ts tests are based on the postulate that application pages and API endpoints are served the same way. Seems like a good idea to duplicate the tests against an 'API' endpoint

.set('accept-encoding', 'br')
.then((response) => {
expect(response.header).to.have.property('content-encoding', 'br');
});
});
});
}
10 changes: 9 additions & 1 deletion yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2252,7 +2252,7 @@
dependencies:
"@hapi/hoek" "^9.0.0"

"@hapi/[email protected]", "@hapi/validate@^1.1.1":
"@hapi/[email protected]", "@hapi/validate@^1.1.1", "@hapi/validate@^1.1.3":
version "1.1.3"
resolved "https://registry.yarnpkg.com/@hapi/validate/-/validate-1.1.3.tgz#f750a07283929e09b51aa16be34affb44e1931ad"
integrity sha512-/XMR0N0wjw0Twzq2pQOzPBZlDzkekGcoCtzO314BpIEsbXdYGthQUbxgkGDf4nhk1+IPDAsXqWjMohRQYO06UA==
Expand Down Expand Up @@ -10811,6 +10811,14 @@ brfs@^2.0.0, brfs@^2.0.2:
static-module "^3.0.2"
through2 "^2.0.0"

brok@^5.0.2:
version "5.0.2"
resolved "https://registry.yarnpkg.com/brok/-/brok-5.0.2.tgz#b77e7203ce89d30939a5b877a9bb3acb4dffc848"
integrity sha512-mqsoOGPjcP9oltC8dD4PnRCiJREmFg+ee588mVYZgZNd8YV5Zo6eOLv/fp6HxdYffaxvkKfPHjc+sRWIkuIu7A==
dependencies:
"@hapi/hoek" "^9.0.4"
"@hapi/validate" "^1.1.3"

brorand@^1.0.1, brorand@^1.1.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/brorand/-/brorand-1.1.0.tgz#12c25efe40a45e3c323eb8675a0a0ce57b22371f"
Expand Down