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

[Telemetry] Migrate public to NP #56285

Merged
merged 61 commits into from
Feb 13, 2020
Merged

Conversation

Bamieh
Copy link
Member

@Bamieh Bamieh commented Jan 29, 2020

Fully migrates telemetry public to NP (Closes #45428)

Important Notes:

  • I've improved the tests we have for telemetry/public a lot in the PR. This PR mainly moves the current code from old platform to NP so we can open an issue to add more test coverage in future PRs.

  • Management section wrapper is not moved to NP since this management plugin feature is still in legacy. The management component and everything inside the wrapper is fully migrated to NP in the telemetry plugin.

  • Moved handling of old settings from public to server. This way we reduce fetching 3 requests on every page load. This will happen once on kibana init and moves old flags to the current opt In saved object, copying the behavior we had in previous kibana releases.

  • Preivously we used to hide the banner in the status page. We hide the banner in that page since it can be accessed without a login under a kibana configuration. This is no longer needed since anonymousPaths.isAnonymous will add the status page if that config is set.

  • Opting in/out under advanced settings no longer requires a page refresh to work properly after a settings change.

  • Welcome screen in homepage will not show any telemetry related information when telemetry.enabled is set to false:
    image

  • Split the previous monolith telemetryProvider implementation into two main services under NP:

    1. telemetryService for fetching configurations and sending telemetry
    2. telemetryNotifications for handling all telemetry notifications and banners.
  • Internationalized Opted in banner title when migrating the component (closes [i18n] Missing i18n for telemetry banner  #55907)

  • Adds missing field settings to usage statistics advanced settings (closes data-test-subj's and other attributes undefined in Usage Data advanced setting #54968)

  • Fixed flaky tests for browser telemetry sender (closes telemetry class _checkReportStatus returns false if last report is too recent #27922)

@elasticmachine
Copy link
Contributor

Pinging @elastic/pulse (Team:Pulse)

@pgayvallet
Copy link
Contributor

Please ping the platform team again when the PR is ready for review.

@Bamieh Bamieh marked this pull request as ready for review January 31, 2020 14:35
@Bamieh Bamieh requested a review from a team January 31, 2020 14:35
@Bamieh Bamieh added the release_note:skip Skip the PR/issue when compiling release notes label Jan 31, 2020
@@ -135,7 +135,8 @@ export default function({ getService, getPageObjects }) {
it('matches baseline report', async function() {
this.timeout(300000);

await PageObjects.dashboard.switchToEditMode();
await PageObjects.common.navigateToApp('dashboard');
await PageObjects.dashboard.gotoDashboardEditMode('My PNG Dash');
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for catching this, and sorry you had to go through it. There definitely is an issue with the Reporting functional tests in that they have overcomplicated steps that could be worked around by using smarter archives of the objects being reported on.

Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

Platform integration points LGTM

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

Approved from the side reporting functional test changes. Thanks for not just skipping the test!

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Kibana app changes LGTM, thanks a lot for this. We are so close to migrate home to the new platform now 🎉

Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

It looks great to me! Just 2 NITs. Please, feel free to merge if they are not valid comments.

@Bamieh
Copy link
Member Author

Bamieh commented Feb 12, 2020

@elasticmachine merge upstream

@Bamieh
Copy link
Member Author

Bamieh commented Feb 13, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@Bamieh Bamieh merged commit 06df2b0 into elastic:master Feb 13, 2020
@Bamieh Bamieh deleted the telemetry/migrate_public branch February 13, 2020 07:44
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 13, 2020
* master:
  add `absolute` option to `getUrlForApp` (elastic#57193)
  [Telemetry] Migrate public to NP (elastic#56285)
  address flaky test where instances might have different start… (elastic#57506)
  fix(NA): support legacy plugins path in plugins (elastic#57472)
  build immutable bundles for new platform plugins (elastic#53976)
  [SIEM] [Detection Engine] Reject if duplicate rule_id in request payload (elastic#57057)
  Add autocomplete="off" for input type="password" to appease the scanners (elastic#56922)
  Use default spaces suffix for signals index if spaces disabled (elastic#57244)
  [Alerting] Create alert design cleanup (elastic#56929)
mbondyra added a commit to mbondyra/kibana that referenced this pull request Feb 13, 2020
* master: (22 commits)
  Use log4j pattern syntax (elastic#57433)
  [ML] Categorization field example endpoint tests (elastic#57471)
  [Lens] Filter out pinned filters from saved object of Lens (elastic#57197)
  Lens client side shim cleanup (elastic#56976)
  [Maps] do not show border color for icon in legend when border width is zero (elastic#57501)
  refactors 'data-providers' tests (elastic#57474)
  add `absolute` option to `getUrlForApp` (elastic#57193)
  [Telemetry] Migrate public to NP (elastic#56285)
  address flaky test where instances might have different start… (elastic#57506)
  fix(NA): support legacy plugins path in plugins (elastic#57472)
  build immutable bundles for new platform plugins (elastic#53976)
  [SIEM] [Detection Engine] Reject if duplicate rule_id in request payload (elastic#57057)
  Add autocomplete="off" for input type="password" to appease the scanners (elastic#56922)
  Use default spaces suffix for signals index if spaces disabled (elastic#57244)
  [Alerting] Create alert design cleanup (elastic#56929)
  Management Api - add to migration guide (elastic#56892)
  fixing maps (elastic#56706)
  [Maps] Autocomplete for custom color palettes and custom icon palettes (elastic#56446)
  [Alerting] make actionGroup name's i18n-able (elastic#57404)
  fixed flaky test (elastic#57490)
  ...

# Conflicts:
#	src/legacy/core_plugins/telemetry/public/components/__snapshots__/telemetry_form.test.js.snap
#	src/plugins/telemetry/public/components/telemetry_management_section.tsx
@lukeelmers lukeelmers added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Oct 1, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform Feature:Telemetry 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.7.0 v8.0.0
Projects
None yet