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

Migrate CSP usage collector to kibana_usage_collection plugin #75536

Merged

Conversation

pgayvallet
Copy link
Contributor

Summary

Fix #50511

move the csp usage collector from legacy to the kibana_usage_collection plugin

Checklist

@pgayvallet pgayvallet added Feature:Legacy Removal Issues related to removing legacy Kibana release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.10.0 labels Aug 20, 2020
Comment on lines +63 to 65
const collectorOptions = createCspCollector(http);
const collector = usageCollection.makeUsageCollector<Usage>(collectorOptions);
usageCollection.registerCollector(collector);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The scripts/telemetry_check.js was kinda picky on the syntax I had to use here: I was not allowed to directly call usageCollection.makeUsageCollector(createCspCollector(http)) and I was forced to manually specify the <Usage> type that is already implicitly defined when calling makeUsageCollector

Copy link
Member

Choose a reason for hiding this comment

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

Yeah... we are working on improving it to be more developer-friendly. Thank you for sharing your frustration about this 🙂
cc/ @Bamieh

Comment on lines -42 to -45
const kibanaPlugin = body.status.statuses.find((s) => {
return s.id.indexOf('plugin:kibana') === 0;
});
expect(kibanaPlugin.state).to.be('green');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing the init function from the legacy kibana plugin removed the plugin entry from the status, so I had to adapt two tests relying on that. As we are very close to remove the plugin totally, I felt like it was better to remove that than to preserve an empty init function.

It's also likely we'll adapt and enhance these tests once we finish the status API migration anyway.

@pgayvallet pgayvallet marked this pull request as ready for review August 20, 2020 13:52
@pgayvallet pgayvallet requested a review from a team as a code owner August 20, 2020 13:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@pgayvallet pgayvallet requested a review from a team August 20, 2020 14:47
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

Would you mind adding csp to the README in https://github.com/elastic/kibana/blob/master/src/plugins/kibana_usage_collection/README.md please?
Otherwise LGMT

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@pgayvallet pgayvallet merged commit fd459de into elastic:master Aug 21, 2020
pgayvallet added a commit to pgayvallet/kibana that referenced this pull request Aug 21, 2020
…tic#75536)

* move csp usage collector from legacy kibana plugin to kibana_usage_collection

* make scripts/telemetry_check happy.

* remove assertion on legacy kibana plugin

* remove test on legacy kibana plugin

* update README
pgayvallet added a commit that referenced this pull request Aug 21, 2020
…) (#75643)

* move csp usage collector from legacy kibana plugin to kibana_usage_collection

* make scripts/telemetry_check happy.

* remove assertion on legacy kibana plugin

* remove test on legacy kibana plugin

* update README
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 21, 2020
* master: (71 commits)
  [Lens] Show 'No data for this field' for empty field in accordion (elastic#73772)
  Skip failing lens test
  Configure ScopedHistory consistenty regardless of URL used to mount app (elastic#75074)
  Fix returned payload by "search" usage collector (elastic#75340)
  [Security Solution] Fix missing key error (elastic#75576)
  Upgrade EUI to v27.4.1 (elastic#75240)
  Update datasets UI copy to data streams (elastic#75618)
  [Lens] Register saved object references (elastic#74523)
  [DOCS] Update links to Beats documentation (elastic#70380)
  [Enterprise Search] Convert our `public_url` route to `config_data` and collect initialAppData (elastic#75616)
  [Usage Collection Schemas] Remove Legacy entries (elastic#75652)
  [Dashboard First] Lens Originating App Breadcrumb (elastic#75470)
  Improve login UI error message. (elastic#75642)
  [Security Solution] modify circular deps checker to output images of circular deps graphs (elastic#75579)
  [Data Telemetry] Add index pattern to identify "meow" attacks (elastic#75163)
  Migrate CSP usage collector to `kibana_usage_collection` plugin (elastic#75536)
  [Console] Get ES Config from core (elastic#75406)
  [Uptime] Add delay in telemetry test (elastic#75162)
  [Lens] Use index pattern service instead saved object client (elastic#74654)
  Embeddable input (elastic#73033)
  ...
thomasneirynck pushed a commit to thomasneirynck/kibana that referenced this pull request Aug 21, 2020
…tic#75536)

* move csp usage collector from legacy kibana plugin to kibana_usage_collection

* make scripts/telemetry_check happy.

* remove assertion on legacy kibana plugin

* remove test on legacy kibana plugin

* update README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Legacy Removal Issues related to removing legacy Kibana release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate CSP usage collector to New Platform
5 participants