[Telemetry] collector set to np #51618

Bamieh · 2019-11-25T16:37:39Z

Fully migrate (server.usage.collectorSet) to New Platform under (UsageCollection) plugin.

The PR is huge but its mostly moving plugins to use this NP plugin instead.

Implement the new NP plugin
Fully migrate all plugins to use UsageCollection
Remove usage.collectorSet from core.
Update readme src/plugins/usage_collection/README.md

Testing

Reviewers:

The PR is huge as it is touching many-many plugins, if your team is required a review; a quick way to filter the files to see only the changes relevant to you is
by filtering the files through github:

Footer Notes

This PR does not change the current collectorSet behavior in any way.

Closes #46924
Closes #51371

Dev Docs

Fully migrate (server.usage.collectorSet) to New Platform under (UsageCollection) plugin. To use the UsageCollector plugin to collect server side stats with the NP follow the quick guide below (Note that all the plugins are already migrated to use this new plugin in this very same PR):

Make sure usageCollection is in your optional Plugins:

// plugin/kibana.json
{
  "id": "...",
  "optionalPlugins": ["usageCollection"]
}

Register Usage collector in the setup function:

// server/plugin.ts
class Plugin {
  setup(core, plugins) {
    registerMyPluginUsageCollector(plugins.usageCollection);
  }
}

Creating and registering a Usage Collector. Ideally collectors would be defined in a separate directory server/collectors/register.ts.

// server/collectors/register.ts
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { CallCluster } from 'src/legacy/core_plugins/elasticsearch';

export function registerMyPluginUsageCollector(usageCollection?: UsageCollectionSetup): void {
  // usageCollection is an optional dependency, so make sure to return if it is not registered.
  if (!usageCollection) {
    return;
  }

  // create usage collector
  const myCollector = usageCollection.makeUsageCollector({
    type: MY_USAGE_TYPE,
    fetch: async (callCluster: CallCluster) => {

    // query ES and get some data
    // summarize the data into a model
    // return the modeled object that includes whatever you want to track

      return {
        my_objects: {
          total: SOME_NUMBER
        }
      };
    },
  });

  // register usage collector
  usageCollection.registerCollector(myCollector);
}

…lector_set_to_np

elasticmachine · 2019-11-25T18:16:41Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: fb96edb

elasticmachine · 2019-11-25T18:50:34Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: de02775

elasticmachine · 2019-11-25T20:31:48Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: a8b276b

…lector_set_to_np

elasticmachine · 2019-11-25T22:24:10Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 6cec0a2

elasticmachine · 2019-11-25T23:00:02Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 4d5e9f7

…lector_set_to_np

crob611

Canvas changes look good 👍

chrisdavies

The lens stuff looks good to me. I didn't review the over-arching PR, nor did I pull it down and run it.

smith

APM changes look good.

tsullivan · 2019-11-26T15:49:55Z

This PR does not change the current collectorSet behavior in any way.

I tested that Kibana Monitoring still works, with the self-monitoring use case, so I agree there are no regressions there.

elasticmachine · 2019-11-26T15:50:03Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 23aedbe

elasticmachine · 2019-11-26T16:11:48Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: f0dd27a

walterra

ML changes LGTM

tsullivan

LGTM! Thanks for doing this. It looks like a lot of work! Thanks for helping me understand the current status of Telemetry Plugin (still in Legacy) and Usage Collection Plugin (New Platform)

legrego

Spaces related changes LGTM! Thanks for doing this!

kindsun

File Upload changes lgtm!

jasonrhodes · 2019-11-26T18:33:34Z

x-pack/legacy/plugins/infra/server/kibana.index.ts

-
-export const initServerWithKibana = (kbnServer: KbnServer) => {
+export const initServerWithKibana = (kbnServer: Server) => {
+  const usageCollection = kbnServer.newPlatform.setup.plugins.usageCollection as UsageCollection;


So ideally in the NP and if we were shimmed correctly here, this would all happen in the setup method right?

@jasonrhodes correct. you can check the (dev notes) for a full example.

joshdover

No fundamental problems I'm worried about, just some suggestions.

joshdover · 2019-11-26T19:58:38Z

src/plugins/usage_collection/server/plugin.ts

+      maximumWaitTimeForAllCollectorsInS: config.maximumWaitTimeForAllCollectorsInS,
+    });
+
+    return collectorSet;


You may want to expose this as a key on an object to make changes to this API in the future simpler:

Suggested change

return collectorSet;

return { collectorSet };

That way you can add additional keys to the exposed API without breaking existing usages.

@joshdover my aim is to keep the setup function only for initializing this class and passing configs to it. The actual contract is the CollectorSet class itself. This would prevent bloating the code inside the plugin itself, and restricting it inside the CollectorSet class.

src/plugins/usage_collection/server/plugin.ts

joshdover · 2019-11-26T20:03:22Z

src/plugins/usage_collection/README.md

+  });
+
+  // register usage collector
+  usageCollection.registerCollector(myCollector);


Is this step useful? Seems like it'd be nice if usageCollection.makeUsageCollector just automatically registered it as well.

I wanted to keep the changes to the collectorSet api to a minimal.

Initially the idea behind was that we might want to do other stuff to the collector other than registering it with the usage collector.

This might still be the case we'd want to have with statsCollector. which use the same collector but register it under statsCollection: used for monitoring ui stats and does not report it under usage.

src/plugins/usage_collection/README.md

src/legacy/core_plugins/kibana/index.js

…lector_set_to_np

sorenlouv · 2019-11-26T23:48:48Z

x-pack/legacy/plugins/apm/server/routes/typings.ts

-      register: (options: unknown) => unknown;
-    };
-  };
+export type APMLegacyServer = Pick<Server, 'savedObjects' | 'log'> & {


Yay, no more custom types. I think actually you can remove all of it so:

export type APMLegacyServer = Server;

(yes, we should just delete APMLegacyServer and use Server everywhere)

FWIW, this was per suggestion of @rudolf, to be very explicit about whatever functionality we are still using from Server, so we have better control over what we allow Server to be used for. I personally still think that that is a good idea

elasticmachine · 2019-11-26T23:55:42Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 19ea04c

* merge conflicts * fix upgrade_assistant code * fix upgrade_assistant code * checkout upgrade_assistant from master * checkout upgrade_assistant from master * redo upgrade_assistant work on 7.x branch

* upstream/7.x: Fix infinite redirect loop when multiple cookies are sent (elastic#50452) (elastic#51821) [Console] Proxy fallback (elastic#50185) (elastic#51814) Added endgame-* index and new heading 3 Elastic Endpoint SMP. (elastic#51071) (elastic#51828) [Maps] Added options to disable zoom, hide tool tips, widgets/overlays in embeddable maps (elastic#50663) (elastic#51811) Move errors and validate index pattern ⇒ NP (elastic#51805) (elastic#51831) [SIEM][Detection Engine] Adds ecs threat properties to rules (elastic#51782) (elastic#51827) [Lens] Remove client-side reference to server source code (elastic#51763) (elastic#51825) fixes drag and drop in tests (elastic#51806) (elastic#51813) [Uptime] Redesign/44541 new monitor list expanded row (elastic#46567) (elastic#51809) [7.x] [Telemetry] collector set to np (elastic#51618) (elastic#51787) [Uptime] added test for chart wrapper (elastic#50399) (elastic#51808) Expressions service fixes: better error and loading states handling (elastic#51183) (elastic#51800) Query String(Bar) Input - cleanup (elastic#51598) (elastic#51804) [ML] Adjust and re-enable categorization advanced wizard test (elastic#51005) (elastic#51017) fixes url state tests (elastic#51746) (elastic#51798) fixes browser field tests (elastic#51738) (elastic#51799) [Task Manager] Tests for the ability to run tasks of varying durations in parallel (elastic#51572) (elastic#51701) [ML] Fix anomaly detection test suite (elastic#51712) (elastic#51795) [SIEM] Fix Timeline drag and drop behavior (elastic#51558) (elastic#51793)

* first iteration * local collection ready * type check * fix collectorSet tests * unskip test * ordering * collectors as array in constructor * update README files * update README and canvas to check for optional dep * update README with more details * Add file path for README example * type UsageCollectionSetup * run type check after refactor

Bamieh added 3 commits November 25, 2019 17:54

first iteration

83e440a

local collection ready

c161fe6

Merge branch 'master' of github.com:elastic/kibana into telemetry/col…

fb96edb

…lector_set_to_np

Bamieh added Feature:NP Migration Feature:Telemetry v7.6.0 v8.0.0 release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. labels Nov 25, 2019

type check

de02775

fix collectorSet tests

3398c80

Bamieh added 2 commits November 25, 2019 21:05

merge master

5f20f4c

unskip test

a8b276b

Bamieh added 3 commits November 25, 2019 23:13

ordering

a3b5396

Merge branch 'master' of github.com:elastic/kibana into telemetry/col…

6cec0a2

…lector_set_to_np

collectors as array in constructor

4d5e9f7

Bamieh added 2 commits November 26, 2019 16:02

Merge branch 'master' of github.com:elastic/kibana into telemetry/col…

b017e11

…lector_set_to_np

update README files

23aedbe

Bamieh marked this pull request as ready for review November 26, 2019 14:39

Bamieh requested a review from a team as a code owner November 26, 2019 14:39

Bamieh requested a review from a team November 26, 2019 14:39

Bamieh requested review from a team as code owners November 26, 2019 14:39

update README and canvas to check for optional dep

a7da928

update README with more details

278be6d

crob611 approved these changes Nov 26, 2019

View reviewed changes

Add file path for README example

f0dd27a

chrisdavies approved these changes Nov 26, 2019

View reviewed changes

smith approved these changes Nov 26, 2019

View reviewed changes

walterra approved these changes Nov 26, 2019

View reviewed changes

tsullivan approved these changes Nov 26, 2019

View reviewed changes

legrego approved these changes Nov 26, 2019

View reviewed changes

kindsun approved these changes Nov 26, 2019

View reviewed changes

Bamieh requested a review from a team November 26, 2019 17:46

jasonrhodes reviewed Nov 26, 2019

View reviewed changes

jasonrhodes approved these changes Nov 26, 2019

View reviewed changes

jasonrhodes mentioned this pull request Nov 26, 2019

[APM] New Kibana platform migration plan #32894

Closed

joshdover approved these changes Nov 26, 2019

View reviewed changes

joshdover mentioned this pull request Nov 26, 2019

Migrate Cloud plugin to New Platform #51776

Closed

1 task

Bamieh added 3 commits November 27, 2019 00:21

type UsageCollectionSetup

7566a8a

run type check after refactor

cd85bf5

Merge branch 'master' of github.com:elastic/kibana into telemetry/col…

19ea04c

…lector_set_to_np

sorenlouv reviewed Nov 26, 2019

View reviewed changes

Bamieh merged commit 0039e97 into elastic:master Nov 26, 2019

Bamieh deleted the telemetry/collector_set_to_np branch November 26, 2019 23:55

Bamieh mentioned this pull request Nov 27, 2019

[7.x] [Telemetry] collector set to np (#51618) #51787

Merged

Kerry350 mentioned this pull request Nov 27, 2019

Infra server NP shim + config/routing API adoption #45299

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Telemetry] collector set to np #51618

[Telemetry] collector set to np #51618

Bamieh commented Nov 25, 2019 •

edited

Loading

elasticmachine commented Nov 25, 2019

elasticmachine commented Nov 25, 2019

elasticmachine commented Nov 25, 2019

elasticmachine commented Nov 25, 2019

elasticmachine commented Nov 25, 2019

crob611 left a comment

chrisdavies left a comment

smith left a comment

tsullivan commented Nov 26, 2019

elasticmachine commented Nov 26, 2019

elasticmachine commented Nov 26, 2019

walterra left a comment

tsullivan left a comment

legrego left a comment

kindsun left a comment

jasonrhodes Nov 26, 2019

Bamieh Nov 26, 2019

joshdover left a comment

joshdover Nov 26, 2019

Bamieh Nov 26, 2019

joshdover Nov 26, 2019

Bamieh Nov 26, 2019

sorenlouv Nov 26, 2019 •

edited

Loading

dgieselaar Nov 27, 2019

elasticmachine commented Nov 26, 2019

[Telemetry] collector set to np #51618

[Telemetry] collector set to np #51618

Conversation

Bamieh commented Nov 25, 2019 • edited Loading

Testing

Reviewers:

Footer Notes

Dev Docs

elasticmachine commented Nov 25, 2019

💔 Build Failed

elasticmachine commented Nov 25, 2019

💔 Build Failed

elasticmachine commented Nov 25, 2019

💔 Build Failed

elasticmachine commented Nov 25, 2019

💔 Build Failed

elasticmachine commented Nov 25, 2019

💚 Build Succeeded

crob611 left a comment

Choose a reason for hiding this comment

chrisdavies left a comment

Choose a reason for hiding this comment

smith left a comment

Choose a reason for hiding this comment

tsullivan commented Nov 26, 2019

elasticmachine commented Nov 26, 2019

💚 Build Succeeded

elasticmachine commented Nov 26, 2019

💚 Build Succeeded

walterra left a comment

Choose a reason for hiding this comment

tsullivan left a comment

Choose a reason for hiding this comment

legrego left a comment

Choose a reason for hiding this comment

kindsun left a comment

Choose a reason for hiding this comment

jasonrhodes Nov 26, 2019

Choose a reason for hiding this comment

Bamieh Nov 26, 2019

Choose a reason for hiding this comment

joshdover left a comment

Choose a reason for hiding this comment

joshdover Nov 26, 2019

Choose a reason for hiding this comment

Bamieh Nov 26, 2019

Choose a reason for hiding this comment

joshdover Nov 26, 2019

Choose a reason for hiding this comment

Bamieh Nov 26, 2019

Choose a reason for hiding this comment

sorenlouv Nov 26, 2019 • edited Loading

Choose a reason for hiding this comment

dgieselaar Nov 27, 2019

Choose a reason for hiding this comment

elasticmachine commented Nov 26, 2019

💚 Build Succeeded

Bamieh commented Nov 25, 2019 •

edited

Loading

sorenlouv Nov 26, 2019 •

edited

Loading