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

[Maps] POC adding experimental vector drawing layer functionality to the Maps app #96365

Closed
wants to merge 26 commits into from

Conversation

kindsun
Copy link
Contributor

@kindsun kindsun commented Apr 6, 2021

DO NOT MERGE: POC

Adds a new way of creating a maps vector layer via draw tools. Drawing vector layers is hidden behind an experimental flag. To enable, add the following to your kibana.yml:

xpack.maps.enableDrawingFeature: true

Peek 2021-04-08 13-58

@kibanamachine
Copy link
Contributor

kibanamachine commented Apr 8, 2021

💔 Build Failed

Failed CI Steps


Test Failures

Kibana Pipeline / jest / Jest Tests.x-pack/plugins/maps/public/classes/sources/new_vector_layer_source.GeoJsonFileSource getName should get default display name

Link to Jenkins

Standard Out

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


Stack Trace

TypeError: _new_vector_layer_source.GeoJsonFileSource is not a constructor
    at Object.<anonymous> (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/maps/public/classes/sources/new_vector_layer_source/new_vector_layer.test.ts:15:33)
    at Promise.then.completed (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/utils.js:276:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/utils.js:216:10)
    at _callCircusTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:212:40)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at _runTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:149:3)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:63:9)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:57:9)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:57:9)
    at run (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:25:3)
    at runAndTransformResultsToJestFormat (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:176:21)
    at jestAdapter (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:109:19)
    at runTestInternal (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/runTest.js:380:16)
    at runTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/runTest.js:472:34)
    at Object.worker (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/testWorker.js:133:12)

Kibana Pipeline / jest / Jest Tests.x-pack/plugins/maps/public/classes/sources/new_vector_layer_source.GeoJsonFileSource getBounds should get null bounds

Link to Jenkins

Standard Out

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


Stack Trace

TypeError: _new_vector_layer_source.GeoJsonFileSource is not a constructor
    at Object.<anonymous> (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/maps/public/classes/sources/new_vector_layer_source/new_vector_layer.test.ts:21:33)
    at Promise.then.completed (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/utils.js:276:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/utils.js:216:10)
    at _callCircusTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:212:40)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at _runTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:149:3)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:63:9)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:57:9)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:57:9)
    at run (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:25:3)
    at runAndTransformResultsToJestFormat (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:176:21)
    at jestAdapter (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:109:19)
    at runTestInternal (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/runTest.js:380:16)
    at runTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/runTest.js:472:34)
    at Object.worker (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/testWorker.js:133:12)

Kibana Pipeline / jest / Jest Tests.x-pack/plugins/maps/public/classes/sources/new_vector_layer_source.GeoJsonFileSource getBounds should get bounds from feature collection

Link to Jenkins

Standard Out

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


Stack Trace

TypeError: _new_vector_layer_source.GeoJsonFileSource is not a constructor
    at Object.<anonymous> (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/maps/public/classes/sources/new_vector_layer_source/new_vector_layer.test.ts:28:33)
    at Promise.then.completed (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/utils.js:276:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/utils.js:216:10)
    at _callCircusTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:212:40)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at _runTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:149:3)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:63:9)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:57:9)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:57:9)
    at run (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:25:3)
    at runAndTransformResultsToJestFormat (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:176:21)
    at jestAdapter (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:109:19)
    at runTestInternal (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/runTest.js:380:16)
    at runTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/runTest.js:472:34)
    at Object.worker (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/testWorker.js:133:12)

and 1 more failures, only showing the first 3.

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [752308f]

History

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

@nickpeihl
Copy link
Member

If I understand correctly, any user who wants to create a new drawing layer requires the create_index, delete_index privileges. I could see this being problematic for administrators.

Additionally, I think this has the possibility of causing an "indices explosion" where, over time, hundreds or thousands of tiny, throwaway, or one-off indices are created by tens or hundreds of users. This could also be a headache for administrators.

Could we instead consider having a system index to store drawings? In the UI we might use an index alias with a user-defined alias filter? For example, when Alice creates a new drawing, she can name it Alice's Restaurant and a new index alias is created on the .kibana-map-drawings system index.

Example Console Logs
DELETE kibana-map-drawings

PUT kibana-map-drawings

POST kibana-map-drawings/_mapping
{
  "properties": {
    "geometry": {
      "type": "geo_shape"
    },
    "drawing_name": {
      "type": "keyword"
    },
    "user_id": {
      "type": "keyword"
    }
  }
}

PUT /kibana-map-drawings/_alias/alices-restaurant
{
  "filter": [
    {
      "term": {
        "drawing_name": "Alice's Restaurant"
      }
    },
    {
      "term": {
        "user_id": "alice"
      }
    }
  ]
}

POST alices-restaurant/_doc?require_alias=true
{
  "geometry": {
    "type": "polygon",
    "coordinates": [
      [
        [
          100,
          0
        ],
        [
          101,
          0
        ],
        [
          101,
          1
        ],
        [
          100,
          1
        ],
        [
          100,
          0
        ]
      ]
    ]
  },
  "drawing_name": "Alice's Restaurant",
  "user_id": "alice"
}

PUT kibana-map-drawings/_alias/bobs-tires
{
  "filter": [
    {
      "term": {
        "drawing_name": "Bob's Tires"
      }
    },
    {
      "term": {
        "user_id": "bob"
      }
    }
  ]
}

POST bobs-tires/_doc?require_alias=true
{
  "geometry": {
    "type": "polygon",
    "coordinates": [
      [
        [
          50,
          0
        ],
        [
          51,
          0
        ],
        [
          51,
          1
        ],
        [
          50,
          1
        ],
        [
          50,
          0
        ]
      ]
    ]
  },
  "drawing_name": "Bob's Tires",
  "user_id": "bob"
}

GET kibana-map-drawings/_search

GET alices-restaurant/_search

GET bobs-tires/_search

This way, we only have one index for all of Alice and Bob's drawings, but we have a way to maintain them separately. If Alice wants to expand her restaurant, she will only see the features she has drawn because the filter from the alias is applied to the index.

And I think Alice and Bob only need to have the manage permission to create index aliases.

@kindsun kindsun changed the title [Maps] Add experimental vector drawing layer functionality [Maps] POC adding experimental vector drawing layer functionality to the Maps app Apr 8, 2021
@ghudgins
Copy link

this works so well, exactly how I expect. really nice!! I only wish I had freehand options too. Here's an example has the prompt "press drag and release" and works nicely
image

@kindsun
Copy link
Contributor Author

kindsun commented Apr 22, 2021

Closing since this has been tested by key stakeholders and is now planned as a new Maps feature. An issue describing the steps for integration has been created here.

@nickpeihl Thanks for the comments. I think how we'll manage indexes moving forward will be an ongoing topic of conversation. At this point it is planned to allow users to modify current indexes and create new ones so we probably won't be leaning towards using system indexes. If this utility does indeed create an index explosion, it means people are using it (yay!) and we need to handle the side-effects of easier ingest processes. I have a few ideas around that (something like index tags) but I'll save it for a future thread!

@ghudgins Interesting feature. I've captured it in #96836. We'll discuss it and see if it's something we want to support and determine level of difficulty for implementation. Thanks!

@kindsun kindsun closed this Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants