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

[Ingest Manager] validate agent route using AJV instead kbn-config-schema #76546

Merged

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Sep 2, 2020

Summary

Resolve #76281

Validate agent routes using AJV instead of kbn-config-schema as kbn-config-schema rely on Joi that is not the most performant JS library for validation.

We should try to use AJV instead of JOI for the most frequently called of Fleet.

Perfomance improvements

Benchmark of the agent enroll request https://gist.github.com/nchaulet/894e00cf8bd649c3e1c2b063346cb621

AJV x 2,093,579 ops/sec ±6.22% (76 runs sampled)
kbn-config-schema x 10,621 ops/sec ±8.37% (84 runs sampled)

I am working on running a Fleet load test against this change.

@nchaulet nchaulet added Ingest Management:beta2 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.10.0 v8.0.0 labels Sep 2, 2020
@nchaulet nchaulet marked this pull request as ready for review September 2, 2020 18:53
@nchaulet nchaulet requested a review from a team September 2, 2020 18:53
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

Copy link
Contributor

@jfsiii jfsiii left a comment

Choose a reason for hiding this comment

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

Great to see this! I left some comments inline but 👍 to 🚀 and iterate

@@ -84,7 +116,10 @@ export const registerRoutes = (router: IRouter) => {
router.post(
{
path: AGENT_API_ROUTES.CHECKIN_PATTERN,
validate: PostAgentCheckinRequestSchema,
validate: {
params: makeValidator(PostAgentCheckinRequestParamsJSONSchema),
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

nchaulet and others added 2 commits September 3, 2020 08:45
Co-authored-by: John Schulz <john.schulz@elastic.co>
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@nchaulet nchaulet merged commit f7ad02d into elastic:master Sep 4, 2020
@nchaulet nchaulet deleted the feature-validate-agent-route-using-ajv branch September 4, 2020 13:13
nchaulet added a commit to nchaulet/kibana that referenced this pull request Sep 4, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 4, 2020
* master: (47 commits)
  Do not require id & description when creating a logstash pipeline (elastic#76616)
  Remove commented src/core/tsconfig file (elastic#76792)
  Replaced whitelistedHosts with allowedHosts in actions ascii docs (elastic#76731)
  [Dashboard First] Genericize Attribute Service (elastic#76057)
  [ci-metrics] unify distributable file count metrics (elastic#76448)
  [Security Solution][Detections] Handle conflicts on alert status update (elastic#75492)
  [eslint] convert to @typescript-eslint/no-unused-expressions (elastic#76471)
  [DOCS] Add default time range filter to advanced settings (elastic#76414)
  [Security Solution] Refactor NetworkTopNFlow to use Search Strategy (elastic#76249)
  [Dashboard] Update Index Patterns when Child Index Patterns Change (elastic#76356)
  [ML] Add option to Advanced Settings to set default time range filter for AD jobs (elastic#76347)
  Add CSM app to CODEOWNERS (elastic#76793)
  [Security Solution][Exceptions] - Updates exception item find sort field (elastic#76685)
  [Security Solution][Detections][Tech Debt] - Move to using common io-ts types (elastic#75009)
  [Lens] Drag dimension to replace (elastic#75895)
  URI encode the index names we fetch in the fetchIndices lib function. (elastic#76584)
  [Security Solution] Resolver retrieve entity id of documents without field mapped (elastic#76562)
  [Ingest Manager] validate agent route using AJV instead kbn-config-schema (elastic#76546)
  Updated non-dev usages of node-forge (elastic#76699)
  [Ingest Pipelines] Processor forms for processors K-S (elastic#75638)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Ingest Manager] Do not use @kbn/config-schema to validate Fleet request
4 participants