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

Jump to bottom

Migrate to peggy from pegjs #97914

Closed
wants to merge 1 commit into from
Closed

Migrate to peggy from pegjs #97914

wants to merge 1 commit into from

Conversation

wylieconlon
Copy link
Contributor

@wylieconlon wylieconlon commented Apr 21, 2021 •
edited
Loading

This PR is a riskier but more complete migration away from pegjs to peggyjs, which is an API compatible fork that is being maintained.

Replaces #97906 if we choose to implement the full version.

For reviewers on app-services who might be concerned about the large diffs in your apps: this happened because nobody regenerated the grammar when pegjs 0.10.0 was released. If you regenerate the grammar on master you will see the same large diffs, this PR just commits the changes.

Closes #97504

@wylieconlon wylieconlon added Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 Team:AppServices release_note:skip Skip the PR/issue when compiling release notes v7.14.0 labels Apr 21, 2021
@wylieconlon wylieconlon requested review from a team as code owners April 21, 2021 21:06
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@kibanamachine
Copy link
Contributor

kibanamachine commented Apr 21, 2021 •
edited
Loading

💔 Build Failed

Failed CI Steps

Test Failures

Kibana Pipeline / jest / Jest Tests.src/core/server/saved_objects/service/lib.SavedObjectsRepository #find errors throws when KQL filter syntax is invalid

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

Stack Trace

Error: expect(received).rejects.toMatchInlineSnapshot(snapshot)

Snapshot name: `SavedObjectsRepository #find errors throws when KQL filter syntax is invalid 1`

Snapshot: [Error: KQLSyntaxError: Expected "(", "{", value, whitespace but "<" found.
dashboard.attributes.otherField:<
--------------------------------^: Bad Request]
Received: [Error: KQLSyntaxError: Expected whitespace, , whitespace, , value, whitespace, , whitespace, , value, whitespace, , whitespace, , value, whitespace, , whitespace, , value but "<" found.
dashboard.attributes.otherField:<
--------------------------------^: Bad Request]
    at Object.toMatchInlineSnapshot (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/expect/build/index.js:241:20)
    at Object.<anonymous> (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/src/core/server/saved_objects/service/lib/repository.test.js:2900:69)
    at Object.asyncJestTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
Kibana Pipeline / jest / Jest Tests.src/core/server/saved_objects/service/lib.SavedObjectsRepository #find search dsl accepts KQL KueryNode filter and passes KueryNode to getSearchDsl

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

Stack Trace

Error: Can't start parsing from rule "Literal".
    at parse (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/src/plugins/data/common/es_query/kuery/ast/_generated_/kuery.js:383:13)
    at fromExpression (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/src/plugins/data/common/es_query/kuery/ast/ast.ts:27:10)
    at Object.fromLiteralExpression (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/src/plugins/data/common/es_query/kuery/ast/ast.ts:34:10)
    at Object.buildNodeParams (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/src/plugins/data/common/es_query/kuery/functions/is.ts:30:13)
    at Object.buildNode (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/src/plugins/data/common/es_query/kuery/node_types/function.ts:26:22)
    at Object.<anonymous> (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/src/core/server/saved_objects/service/lib/repository.test.js:3124:38)
    at Object.asyncJestTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
Kibana Pipeline / jest / Jest Tests.x-pack/plugins/lens/public/editor_frame_service/editor_frame.editor_frame initialization should render the resulting expression using the expression renderer

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

Stack Trace

Error: Unable to parse expression: Can't start parsing from rule "expression".
    at fromExpression (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/@kbn/interpreter/src/common/lib/ast.js:97:11)
    at map (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts:43:32)
    at Array.map (<anonymous>)
    at prependDatasourceExpression (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts:41:29)
    at buildExpression (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts:101:30)
    at getSavedObjectFormat (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/lens/public/editor_frame_service/editor_frame/save.ts:55:22)
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx:236:9
    at commitHookEffectList (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/react-dom/cjs/react-dom.development.js:22030:26)
    at commitPassiveHookEffects (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/react-dom/cjs/react-dom.development.js:22064:11)
    at HTMLUnknownElement.callCallback (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/react-dom/cjs/react-dom.development.js:336:14)
    at HTMLUnknownElement.callTheUserObjectsOperation (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/living/generated/EventListener.js:26:30)
    at innerInvokeEventListeners (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:318:25)
    at invokeEventListeners (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:274:3)
    at HTMLUnknownElementImpl._dispatch (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:221:9)
    at HTMLUnknownElementImpl.dispatchEvent (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:94:17)
    at HTMLUnknownElement.dispatchEvent (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:231:34)
    at Object.invokeGuardedCallbackDev (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/react-dom/cjs/react-dom.development.js:385:16)
    at invokeGuardedCallback (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/react-dom/cjs/react-dom.development.js:440:31)
    at flushPassiveEffectsImpl (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/react-dom/cjs/react-dom.development.js:25392:7)
    at unstable_runWithPriority (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/scheduler/cjs/scheduler.development.js:697:12)
    at runWithPriority$2 (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/react-dom/cjs/react-dom.development.js:12149:10)
    at flushPassiveEffects (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/react-dom/cjs/react-dom.development.js:25361:12)
    at /var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/react-dom/cjs/react-dom.development.js:25240:11
    at workLoop (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/scheduler/cjs/scheduler.development.js:641:34)
    at flushWork (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/scheduler/cjs/scheduler.development.js:596:16)
    at _flushCallback (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/scheduler/cjs/scheduler.development.js:48:9)
    at Timeout.task [as _onTimeout] (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/browser/Window.js:391:19)
    at listOnTimeout (internal/timers.js:554:17)
    at processTimers (internal/timers.js:497:7)

and 311 more failures, only showing the first 3.

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
timelion 91 88 -3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
canvas 1.2MB 1.2MB -134.0B
lens 948.5KB 948.4KB -121.0B
timelion 339.2KB 335.9KB -3.4KB
visTypeTimelion 69.2KB 68.5KB -685.0B
total -4.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
canvas 517.1KB 516.9KB -121.0B
data 792.9KB 785.9KB -7.0KB
expressions 193.0KB 192.8KB -255.0B
total -7.4KB

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

@lukeelmers
Copy link
Member

nobody regenerated the grammar when pegjs 0.10.0 was released.

This another reason why it would be good to prioritize #17284 (it looks like @jbudz was investigating recently but hit some roadblocks)

@wylieconlon wylieconlon mentioned this pull request Apr 21, 2021
Closed
@wylieconlon
Copy link
Contributor Author

@lukeelmers I wasn't aware of the existing efforts there, but I agree that we need to update the grammars automatically. The test failures here are happening because I failed to build the KQL grammar with the right command.

Closing this PR because it's pretty risky until we have automated builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukasolson lukasolson Awaiting requested review from lukasolson

@lukeelmers lukeelmers Awaiting requested review from lukeelmers

@mistic mistic Awaiting requested review from mistic

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.14.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[expressions][kql] Migrate from PEG.js to new "Peggy" fork.
4 participants
@wylieconlon @elasticmachine @kibanamachine @lukeelmers