Skip to content

Commit

Permalink
[ci-stats] move shipper to a package, validate limits in on-merge job (
Browse files Browse the repository at this point in the history
…#149474)

We just had an issue where two PRs were merged and it caused the limit
of the `triggerActionsUi` bundle to be exceeded, breaking PR builds. The
issue is that we didn't see any indication of this in the on-merge jobs
because we don't produce the PR report for on-merge jobs or ask ci-stats
if we should fail the job. Instead, we just ship the metrics for
baseline purposes. This fixes that problem by adding a `--validate` flag
to `node scripts/ship_ci_stats`, which takes care of sending at least
some ci-stats and will verify that the bundle limits are not exceeded.

Since we didn't catch this issue in the on-merge job the limits were
incorrect for over an hour and merged into many PRs, wasting engineering
and CI time.

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
Spencer and kibanamachine authored Jan 25, 2023
1 parent 179b36f commit 4e7560f
Show file tree
Hide file tree
Showing 15 changed files with 149 additions and 72 deletions.
14 changes: 11 additions & 3 deletions .buildkite/scripts/post_build_kibana.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,18 @@
set -euo pipefail

if [[ ! "${DISABLE_CI_STATS_SHIPPING:-}" ]]; then
cmd=(
"node" "scripts/ship_ci_stats"
"--metrics" "target/optimizer_bundle_metrics.json"
"--metrics" "build/kibana/node_modules/@kbn/ui-shared-deps-src/shared_built_assets/metrics.json"
)

if [ "$BUILDKITE_PIPELINE_SLUG" == "kibana-on-merge" ]; then
cmd+=("--validate")
fi

echo "--- Ship Kibana Distribution Metrics to CI Stats"
node scripts/ship_ci_stats \
--metrics target/optimizer_bundle_metrics.json \
--metrics build/kibana/node_modules/@kbn/ui-shared-deps-src/shared_built_assets/metrics.json
"${cmd[@]}"
fi

echo "--- Upload Build Artifacts"
Expand Down
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,7 @@ packages/kbn-chart-icons @elastic/kibana-visualizations
packages/kbn-ci-stats-core @elastic/kibana-operations
packages/kbn-ci-stats-performance-metrics @elastic/kibana-operations
packages/kbn-ci-stats-reporter @elastic/kibana-operations
packages/kbn-ci-stats-shipper-cli @elastic/kibana-operations
packages/kbn-cli-dev-mode @elastic/kibana-operations
packages/kbn-coloring @elastic/kibana-visualizations
packages/kbn-config @elastic/kibana-core
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,7 @@
"@kbn/ci-stats-core": "link:packages/kbn-ci-stats-core",
"@kbn/ci-stats-performance-metrics": "link:packages/kbn-ci-stats-performance-metrics",
"@kbn/ci-stats-reporter": "link:packages/kbn-ci-stats-reporter",
"@kbn/ci-stats-shipper-cli": "link:packages/kbn-ci-stats-shipper-cli",
"@kbn/cli-dev-mode": "link:packages/kbn-cli-dev-mode",
"@kbn/core-test-helpers-kbn-server": "link:packages/core/test-helpers/core-test-helpers-kbn-server",
"@kbn/cypress-config": "link:packages/kbn-cypress-config",
Expand Down
5 changes: 5 additions & 0 deletions packages/kbn-ci-stats-shipper-cli/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# @kbn/ci-stats-shipper-cli

Simple CLI that runs in CI to ship the metrics produced by the build process. We used to ship these metrics as part of the build, but in order to enable distributed caching of optimizer bundles (which we still don't do) we broke the metrics out of the build and write them to disk instead, to be shipped at a later time.

Run `node scripts/ship_ci_stats --help` for usage information.
13 changes: 13 additions & 0 deletions packages/kbn-ci-stats-shipper-cli/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

module.exports = {
preset: '@kbn/test/jest_node',
rootDir: '../..',
roots: ['<rootDir>/packages/kbn-ci-stats-shipper-cli'],
};
6 changes: 6 additions & 0 deletions packages/kbn-ci-stats-shipper-cli/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "shared-common",
"id": "@kbn/ci-stats-shipper-cli",
"owner": "@elastic/kibana-operations",
"devOnly": true
}
7 changes: 7 additions & 0 deletions packages/kbn-ci-stats-shipper-cli/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "@kbn/ci-stats-shipper-cli",
"private": true,
"version": "1.0.0",
"license": "SSPL-1.0 OR Elastic License 2.0",
"main": "./ship_ci_stats_cli"
}
77 changes: 77 additions & 0 deletions packages/kbn-ci-stats-shipper-cli/ship_ci_stats_cli.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import Path from 'path';
import Fs from 'fs';

import { CiStatsReporter, CiStatsMetric } from '@kbn/ci-stats-reporter';

import { createFailError } from '@kbn/dev-cli-errors';
import { run } from '@kbn/dev-cli-runner';

run(
async ({ log, flagsReader }) => {
const validate = flagsReader.boolean('validate');
const metricPaths = flagsReader.arrayOfStrings('metrics') ?? [];

const maybeFail = (message: string) => {
const error = createFailError(message);
if (process.env.IGNORE_SHIP_CI_STATS_ERROR === 'true') {
error.exitCode = 0;
}
return error;
};

const reporter = CiStatsReporter.fromEnv(log);

if (!reporter.isEnabled()) {
throw maybeFail('unable to initilize the CI Stats reporter');
}

const overLimit: string[] = [];

for (const path of metricPaths) {
// resolve path from CLI relative to CWD
const abs = Path.resolve(path);
const json = Fs.readFileSync(abs, 'utf8');
const metrics: CiStatsMetric[] = JSON.parse(json);
if (await reporter.metrics(metrics)) {
log.success('shipped metrics from', path);
} else {
throw maybeFail('failed to ship metrics');
}

for (const metric of metrics) {
if (metric.limit !== undefined && metric.limit < metric.value) {
overLimit.push(
`${metric.group} > ${metric.id} with value of ${metric.value} is greater than the limit of ${metric.limit}`
);
}
}
}

if (validate && overLimit.length) {
throw maybeFail(`Metric overages:\n${overLimit.map((l) => ` ${l}`).join('\n')}`);
}
},
{
description: 'ship ci-stats which have been written to files',
usage: `node scripts/ship_ci_stats`,
log: {
defaultLevel: 'debug',
},
flags: {
string: ['metrics'],
boolean: ['validate'],
help: `
--metrics [path] A path to a JSON file that includes metrics which should be sent. Multiple instances supported
--validate When passed, the process will exit with an error message and a non-zero exit status if any of the bundle limits are exceeded.
`,
},
}
);
21 changes: 21 additions & 0 deletions packages/kbn-ci-stats-shipper-cli/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"outDir": "target/types",
"types": [
"jest",
"node"
]
},
"include": [
"**/*.ts",
],
"exclude": [
"target/**/*"
],
"kbn_references": [
"@kbn/ci-stats-reporter",
"@kbn/dev-cli-errors",
"@kbn/dev-cli-runner",
]
}
1 change: 0 additions & 1 deletion packages/kbn-dev-utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export {
KBN_P12_PASSWORD,
} from './src/certs';
export * from './src/axios';
export * from './src/ship_ci_stats_cli';
export * from './src/plugin_list';
export * from './src/streams';
export * from './src/extract';
Expand Down
66 changes: 0 additions & 66 deletions packages/kbn-dev-utils/src/ship_ci_stats_cli.ts

This file was deleted.

1 change: 0 additions & 1 deletion packages/kbn-dev-utils/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"@kbn/dev-cli-runner",
"@kbn/dev-cli-errors",
"@kbn/plugin-discovery",
"@kbn/ci-stats-reporter",
"@kbn/repo-info",
],
"exclude": [
Expand Down
2 changes: 1 addition & 1 deletion scripts/ship_ci_stats.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@
*/

require('../src/setup_node_env');
require('@kbn/dev-utils').shipCiStatsCli();
require('@kbn/ci-stats-shipper-cli');
2 changes: 2 additions & 0 deletions tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@
"@kbn/ci-stats-performance-metrics/*": ["packages/kbn-ci-stats-performance-metrics/*"],
"@kbn/ci-stats-reporter": ["packages/kbn-ci-stats-reporter"],
"@kbn/ci-stats-reporter/*": ["packages/kbn-ci-stats-reporter/*"],
"@kbn/ci-stats-shipper-cli": ["packages/kbn-ci-stats-shipper-cli"],
"@kbn/ci-stats-shipper-cli/*": ["packages/kbn-ci-stats-shipper-cli/*"],
"@kbn/cli-dev-mode": ["packages/kbn-cli-dev-mode"],
"@kbn/cli-dev-mode/*": ["packages/kbn-cli-dev-mode/*"],
"@kbn/cloud-chat-plugin": ["x-pack/plugins/cloud_integrations/cloud_chat"],
Expand Down
4 changes: 4 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2849,6 +2849,10 @@
version "0.0.0"
uid ""

"@kbn/ci-stats-shipper-cli@link:packages/kbn-ci-stats-shipper-cli":
version "0.0.0"
uid ""

"@kbn/cli-dev-mode@link:packages/kbn-cli-dev-mode":
version "0.0.0"
uid ""
Expand Down

0 comments on commit 4e7560f

Please sign in to comment.