[Telemetry] Separate the license retrieval from the stats in the usage collectors #57332

afharo · 2020-02-11T16:03:15Z

Summary

Separate the license retrieval from the stats fetchers in the usage collectors. This way we can allocate and understand where the logic fails and still have the right license information even when other info might be failing to be retrieved.

Fixes #56967

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-02-11T16:03:18Z

Pinging @elastic/pulse (Team:Pulse)

Bamieh

Direction looks good!

I have a few suggestions:

We cannot be calling xpack APIs from OSS. To solve this see (2).
Let us define getLicense as part of the collectionManager api just like getStats and cluster uuids. This way we can still have the implementation decoupled from where the data is fetched from (monitoring/oss/xpack/others).
I dont think we should really worry about the performance cost of calling the license api. Telemetry is called a few times per day at max.

One idea is to use the license API to register the license fetching instead of depending on our own implementation (in the license_manager plugin) We can open a separate issue/PR for this one though.

…ge-oss-and-xpack-collectors

src/legacy/core_plugins/telemetry/server/collection_manager.ts

…ge-oss-and-xpack-collectors

Bamieh

LGTM, thanks! i did not pull the code locally to test it

Bamieh · 2020-02-24T09:16:02Z

src/legacy/core_plugins/telemetry/server/collection_manager.ts

@@ -141,7 +169,17 @@ export class TelemetryCollectionManager {
      return;
    }

-    return await collection.statsGetter(clustersDetails, statsCollectionConfig);
+    const stats = await collection.statsGetter(clustersDetails, statsCollectionConfig);
+    const licenses = await collection.licenseGetter(clustersDetails, statsCollectionConfig);


can we group them in 1 promise.all so we dont have to wait for them sequentially?

Good call! Just pushed the update :)

Bamieh · 2020-02-24T09:16:14Z

src/legacy/core_plugins/telemetry/server/collection_manager.ts

+      return {
+        ...(license ? { license } : {}),
+        ...stat,
+        collectionSource: collection.title,


…ge-oss-and-xpack-collectors

afharo · 2020-02-24T10:37:12Z

Anyone in the @elastic/stack-monitoring team can have a quick look at these changes? Thank you :)

afharo · 2020-02-24T15:55:05Z

@elasticmachine merge upstream

kibanamachine · 2020-02-24T17:45:30Z

💚 Build Succeeded

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

History

💔 Build #28450 failed e28ea25
💔 Build #28393 failed 1891137
💔 Build #28200 failed 0ce1cad
💔 Build #28152 failed 25d3da5
💔 Build #27120 failed 35068e1

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

igoristic

Looks good 👍

…e collectors (elastic#57332) * [Telemetry] Merge OSS and XPack usage collectors * Create X-Pack collector again * Separate the license retrieval from the stats * Fix telemetry tests with new fields * Use Promise.all to retrieve license and stats at the same time * Fix moment mock Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

…e collectors (#57332) (#58450) * [Telemetry] Merge OSS and XPack usage collectors * Create X-Pack collector again * Separate the license retrieval from the stats * Fix telemetry tests with new fields * Use Promise.all to retrieve license and stats at the same time * Fix moment mock Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

…re/files-and-filetree * 'master' of github.com:elastic/kibana: (174 commits) [SIEM] Fix unnecessary re-renders on the Overview page (elastic#56587) Don't mutate error message (elastic#58452) Fix service map popover transaction duration (elastic#58422) [ML] Adding filebeat config to file dataviz (elastic#58152) [Uptime] Improve refresh handling when generating test data (elastic#58285) [Logs / Metrics UI] Remove path prefix from ViewSourceConfigur… (elastic#58238) [ML] Functional tests - adjust classification model memory (elastic#58445) [ML] Use event.timezone instead of beat.timezone in file upload (elastic#58447) [Logs UI] Unskip and stabilitize log column configuration tests (elastic#58392) [Telemetry] Separate the license retrieval from the stats in the usage collectors (elastic#57332) hide welcome screen for cloud (elastic#58371) Move src/legacy/ui/public/notify/app_redirect to kibana_legacy (elastic#58127) [ML] Functional tests - stabilize typing during df analytics creation (elastic#58227) fix short url in spaces (elastic#58313) [SIEM] Upgrades cypress to version 4.0.2 (elastic#58400) [Index management] Move to new platform "plugins" folder (elastic#58109) [kbn/optimizer] disable parallelization in terser plugin (elastic#58396) [Uptime] Delete useless try...catch blocks (elastic#58263) [Uptime] Use scripted metric for snapshot calculation (elastic#58247) (elastic#58389) [APM] Stabilize agent configuration API (elastic#57767) ... # Conflicts: # src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.tsx

elasticmachine · 2021-10-01T21:41:49Z

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

[Telemetry] Merge OSS and XPack usage collectors

d4a81ad

afharo added Feature:Telemetry v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Team:Pulse labels Feb 11, 2020

afharo requested a review from Bamieh February 11, 2020 16:03

Bamieh requested changes Feb 11, 2020

View reviewed changes

afharo added 5 commits February 13, 2020 14:20

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

122731b

…ge-oss-and-xpack-collectors

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

97f46f5

…ge-oss-and-xpack-collectors

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

35068e1

…ge-oss-and-xpack-collectors

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

3631357

…ge-oss-and-xpack-collectors

Create X-Pack collector again

25d3da5

afharo commented Feb 21, 2020

View reviewed changes

src/legacy/core_plugins/telemetry/server/collection_manager.ts Outdated Show resolved Hide resolved

afharo added 2 commits February 21, 2020 17:02

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

b573500

…ge-oss-and-xpack-collectors

Separate the license retrieval from the stats

0ce1cad

afharo requested a review from a team February 21, 2020 18:26

afharo changed the title ~~[Telemetry] Merge OSS and XPack usage collectors~~ [Telemetry] Separate the license retrieval from the stats in the usage collectors Feb 21, 2020

afharo requested a review from Bamieh February 21, 2020 18:33

afharo mentioned this pull request Feb 21, 2020

[Telemetry] Unify Monitoring and Local collectors payload structure #58249

Closed

Bamieh approved these changes Feb 24, 2020

View reviewed changes

afharo added 3 commits February 24, 2020 10:32

Fix telemetry tests with new fields

555afc2

Use Promise.all to retrieve license and stats at the same time

c8ffa9b

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

1891137

…ge-oss-and-xpack-collectors

Fix moment mock

e28ea25

Merge branch 'master' into telemetry/merge-oss-and-xpack-collectors

6b7393f

igoristic approved these changes Feb 24, 2020

View reviewed changes

afharo merged commit 506268c into elastic:master Feb 25, 2020

afharo deleted the telemetry/merge-oss-and-xpack-collectors branch February 25, 2020 10:31

afharo mentioned this pull request Feb 25, 2020

[7.x] [Telemetry] Separate the license retrieval from the stats in the usage collectors (#57332) #58450

Merged

lukeelmers added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Oct 1, 2021

lukeelmers removed the Team:KibanaTelemetry label Oct 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Telemetry] Separate the license retrieval from the stats in the usage collectors #57332

[Telemetry] Separate the license retrieval from the stats in the usage collectors #57332

afharo commented Feb 11, 2020 •

edited

Loading

elasticmachine commented Feb 11, 2020

Bamieh left a comment •

edited

Loading

Bamieh left a comment

Bamieh Feb 24, 2020

afharo Feb 24, 2020

Bamieh Feb 24, 2020

afharo commented Feb 24, 2020

afharo commented Feb 24, 2020

kibanamachine commented Feb 24, 2020

igoristic left a comment

elasticmachine commented Oct 1, 2021

[Telemetry] Separate the license retrieval from the stats in the usage collectors #57332

[Telemetry] Separate the license retrieval from the stats in the usage collectors #57332

Conversation

afharo commented Feb 11, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Feb 11, 2020

Bamieh left a comment • edited Loading

Choose a reason for hiding this comment

Bamieh left a comment

Choose a reason for hiding this comment

Bamieh Feb 24, 2020

Choose a reason for hiding this comment

afharo Feb 24, 2020

Choose a reason for hiding this comment

Bamieh Feb 24, 2020

Choose a reason for hiding this comment

afharo commented Feb 24, 2020

afharo commented Feb 24, 2020

kibanamachine commented Feb 24, 2020

💚 Build Succeeded

History

igoristic left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 1, 2021

afharo commented Feb 11, 2020 •

edited

Loading

Bamieh left a comment •

edited

Loading