Skip to content

[EPM] Store map visualizations from the package registry and use saved object ID#62059

Merged
jonathan-buttner merged 7 commits intoelastic:masterfrom
jonathan-buttner:saved-objects-maps-ids
Apr 7, 2020
Merged

[EPM] Store map visualizations from the package registry and use saved object ID#62059
jonathan-buttner merged 7 commits intoelastic:masterfrom
jonathan-buttner:saved-objects-maps-ids

Conversation

@jonathan-buttner
Copy link
Contributor

@jonathan-buttner jonathan-buttner commented Mar 31, 2020

This PR adds maps to the types of kibana assets that can be created from a package.

Example map working

image

Testing

To test this change:

  1. Open the Ingest Manager plugin
  2. Install the Okta package

Kibana may crash because of the ingest pipeline within the okta package (this is happing on master without my change as well), this is what was happening to me

{ Error: Internal Server Error
    at respond (/Users/jbuttner/proj/es/kibana/node_modules/elasticsearch/src/lib/transport.js:349:15)
    at checkRespForFailure (/Users/jbuttner/proj/es/kibana/node_modules/elasticsearch/src/lib/transport.js:306:7)
    at HttpConnector.<anonymous> (/Users/jbuttner/proj/es/kibana/node_modules/elasticsearch/src/lib/connectors/http.js:173:7)
    at IncomingMessage.wrapper (/Users/jbuttner/proj/es/kibana/node_modules/elasticsearch/node_modules/lodash/lodash.js:4929:19)
    at IncomingMessage.emit (events.js:203:15)
    at endReadableNT (_stream_readable.js:1145:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)
  status: 500,
  displayName: 'InternalServerError',
  message: 'Internal Server Error',
  path: '/_ingest/pipeline/logs-okta.system-0.0.1',
  query: undefined,
  body:
   '---\nerror:\n  root_cause:\n  - type: "not_x_content_exception"\n    reason: "Compressor detection can only be called on some xcontent bytes or compressed\\\n      \\ xcontent bytes"\n  type: "not_x_content_exception"\n  reason: "Compressor detection can only be called on some xcontent bytes or compressed\\\n    \\ xcontent bytes"\nstatus: 500\n',
  statusCode: 500,
  response:
   '---\nerror:\n  root_cause:\n  - type: "not_x_content_exception"\n    reason: "Compressor detection can only be called on some xcontent bytes or compressed\\\n      \\ xcontent bytes"\n  type: "not_x_content_exception"\n  reason: "Compressor detection can only be called on some xcontent bytes or compressed\\\n    \\ xcontent bytes"\nstatus: 500\n',
  toString: [Function],
  toJSON: [Function] }

Terminating process...
 server crashed  with status code 1

The dashboard and map should still be installed even though kibana crashed.
3. Restart Kibana but not ES
4. Go to the dashboard page
5. Open the [Filebeat Okta] Overview dashboard
6. See that the map is now in the dashboard

With the change

image

Without this change

image

@jonathan-buttner jonathan-buttner added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project v7.8.0 labels Mar 31, 2020
@jonathan-buttner jonathan-buttner requested a review from a team March 31, 2020 19:38
@jonathan-buttner jonathan-buttner self-assigned this Mar 31, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Feature:EPM)

'index-pattern': 'indexPatternApp',
search: 'searchProfilerApp',
visualization: 'visualizeApp',
map: 'mapApp',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if these need to be actual names of icons or something? @jen-huang @neptunian @jfsiii

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is only used in the AssetsFacetGroup component to get the icon. We have a prototype of an assets section in the package detail page, but its been pushed back for a while as its not really important. so you can keep this and when we bring it back, we'll fix it if its not working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok, thanks Sandra!

@jfsiii
Copy link
Contributor

jfsiii commented Mar 31, 2020

@jonathan-buttner Can you include instructions for how to test this?

const savedObject: SavedObjectToBe = {
type,
id: file.replace('.json', ''),
id: asset.id,
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC, EPM initially stored the ID given by the registry/package. However, IIRC, it was removed because we couldn't count on it(upgrades?). I believe we were going to have EPM maintain a mapping(?).

/cc @ruflin @skh @neptunian

I guess we can add this now and fix the broken case you show, then deal with the other issues if/when they show up.

Copy link
Contributor

Choose a reason for hiding this comment

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

My long term preference would be that we use our "own" ids to make sure we don't conflict with other assets and keep a list.

+1 on adding this now to make it work and figure out the exact renaming.

@skh @neptunian Your input on where we are standing here would be great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jfsiii I think I can actually revert this change based on a recommendation from @ruflin. The dashboard I was exporting was relying on a custom index pattern I created instead of the one that the ingest manager creates so I should be able to get around needing to change this line.

@ruflin
Copy link
Contributor

ruflin commented Apr 1, 2020

@jonathan-buttner Thanks for taking this on. This will close #60353

@jonathan-buttner
Copy link
Contributor Author

@jonathan-buttner Can you include instructions for how to test this?

Yeah sorry about that I'll add some instructions.

@jonathan-buttner
Copy link
Contributor Author

@elasticmachine merge upstream

@jonathan-buttner jonathan-buttner added the Feature:Endpoint Elastic Endpoint feature label Apr 3, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

@jonathan-buttner
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

'index-pattern': 'indexPatternApp',
search: 'searchProfilerApp',
visualization: 'visualizeApp',
map: 'mapApp',
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is only used in the AssetsFacetGroup component to get the icon. We have a prototype of an assets section in the package detail page, but its been pushed back for a while as its not really important. so you can keep this and when we bring it back, we'll fix it if its not working.

@jonathan-buttner jonathan-buttner merged commit 994aa63 into elastic:master Apr 7, 2020
@jonathan-buttner jonathan-buttner deleted the saved-objects-maps-ids branch April 7, 2020 12:50
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 7, 2020
* master:
  [APM] Change custom link from EuiListGroupItem to EuiLink (elastic#62742)
  [Remote Clusters] Update callout and move server_name field (elastic#62352)
  Removes Pitch Presentation Template from Canvas (elastic#62688)
  FTR: Enable w3c for chromedriver (elastic#62542)
  [ML] Disable functional tests
  [ILM] Skip failing API integration test (elastic#62779)
  [SIEM] Update beat doc (elastic#61902)
  [Search] Properly add slash preceding path in async search (elastic#62722)
  [APM] make sure environment query is correct for service maps… (elastic#62764)
  Add service map icon for rum-js agent type (elastic#62721)
  [APM] Service map - fixes irrelevant services on data refresh (elastic#62750)
  [APM] Service map - Fix taxi edge arrow orientation (elastic#62741)
  [APM] Prevent error rate alert trigger from rendering NaN (elastic#62754)
  [EPM] Store map visualizations from the package registry and use saved object ID (elastic#62059)
  [Alerting] for email action, set tls.rejectUnauthorized: false when secure: false (elastic#62380)
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 9, 2020
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 62059 or prevent reminders by adding the backport:skip label.

@jen-huang jen-huang added the Team:Fleet Team label for Observability Data Collection Fleet team label Apr 9, 2020
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 9, 2020
jonathan-buttner added a commit that referenced this pull request Apr 9, 2020
…d object ID (#62059) (#63146)

* Using saved objects id instead of creating one

* Adding map to list of types

* Fixing typing errors

* Reverting id change

* Reverting asset id

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
jfsiii pushed a commit that referenced this pull request Jan 14, 2021
## PR Summary
Show [`emsApp` icon from EUI](https://eui.elastic.co/#/display/icons#apps) if integration includes map assets.

<img width="229" alt="Screen Shot 2021-01-12 at 3 14 14 PM" src="https://user-images.githubusercontent.com/57655/104367878-65628280-54e9-11eb-8d9c-fa838e2c15ae.png">

Looking at #62059 (comment), I believe it landed with the incorrect key at a time when that part of the page wasn't visible. When we enabled that section, we likely didn't notice the missing key since most integrations don't have Maps assets

## Bug: `master` doesn't have an icon for Map assets
 1. Visit an integration with map assets, like AWS http://localhost:5601/app/fleet#/integrations/detail/aws-0.3.16
 2. Observe missing icon for Map in right column summary & HTTP request for `/app/mapApp`
    <img width="1013" alt="Screen Shot 2021-01-12 at 3 10 44 PM" src="https://user-images.githubusercontent.com/57655/104367875-64c9ec00-54e9-11eb-81d8-982bec862f5f.png">

## This PR adds one
 1. Visit an integration with map assets, like AWS http://localhost:5601/app/fleet#/integrations/detail/aws-0.3.16
 2. Observe EMS icon is present in right column summary
    <img width="1087" alt="Screen Shot 2021-01-12 at 3 11 39 PM" src="https://user-images.githubusercontent.com/57655/104367877-65628280-54e9-11eb-91da-578fec6ff769.png">
jfsiii pushed a commit that referenced this pull request Jan 14, 2021
## PR Summary
Show [`emsApp` icon from EUI](https://eui.elastic.co/#/display/icons#apps) if integration includes map assets.

<img width="229" alt="Screen Shot 2021-01-12 at 3 14 14 PM" src="https://user-images.githubusercontent.com/57655/104367878-65628280-54e9-11eb-8d9c-fa838e2c15ae.png">

Looking at #62059 (comment), I believe it landed with the incorrect key at a time when that part of the page wasn't visible. When we enabled that section, we likely didn't notice the missing key since most integrations don't have Maps assets

## Bug: `master` doesn't have an icon for Map assets
 1. Visit an integration with map assets, like AWS http://localhost:5601/app/fleet#/integrations/detail/aws-0.3.16
 2. Observe missing icon for Map in right column summary & HTTP request for `/app/mapApp`
    <img width="1013" alt="Screen Shot 2021-01-12 at 3 10 44 PM" src="https://user-images.githubusercontent.com/57655/104367875-64c9ec00-54e9-11eb-81d8-982bec862f5f.png">

## This PR adds one
 1. Visit an integration with map assets, like AWS http://localhost:5601/app/fleet#/integrations/detail/aws-0.3.16
 2. Observe EMS icon is present in right column summary
    <img width="1087" alt="Screen Shot 2021-01-12 at 3 11 39 PM" src="https://user-images.githubusercontent.com/57655/104367877-65628280-54e9-11eb-91da-578fec6ff769.png">
jfsiii pushed a commit that referenced this pull request Jan 14, 2021
## PR Summary
Show [`emsApp` icon from EUI](https://eui.elastic.co/#/display/icons#apps) if integration includes map assets.

<img width="229" alt="Screen Shot 2021-01-12 at 3 14 14 PM" src="https://user-images.githubusercontent.com/57655/104367878-65628280-54e9-11eb-8d9c-fa838e2c15ae.png">

Looking at #62059 (comment), I believe it landed with the incorrect key at a time when that part of the page wasn't visible. When we enabled that section, we likely didn't notice the missing key since most integrations don't have Maps assets

## Bug: `master` doesn't have an icon for Map assets
 1. Visit an integration with map assets, like AWS http://localhost:5601/app/fleet#/integrations/detail/aws-0.3.16
 2. Observe missing icon for Map in right column summary & HTTP request for `/app/mapApp`
    <img width="1013" alt="Screen Shot 2021-01-12 at 3 10 44 PM" src="https://user-images.githubusercontent.com/57655/104367875-64c9ec00-54e9-11eb-81d8-982bec862f5f.png">

## This PR adds one
 1. Visit an integration with map assets, like AWS http://localhost:5601/app/fleet#/integrations/detail/aws-0.3.16
 2. Observe EMS icon is present in right column summary
    <img width="1087" alt="Screen Shot 2021-01-12 at 3 11 39 PM" src="https://user-images.githubusercontent.com/57655/104367877-65628280-54e9-11eb-91da-578fec6ff769.png">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Endpoint Elastic Endpoint feature Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.8.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants