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] Server side fetcher #50015

Merged
merged 64 commits into from
Nov 13, 2019
Merged

Conversation

Bamieh
Copy link
Member

@Bamieh Bamieh commented Nov 4, 2019

Details:

This PR mainly adds server side fetcher. A few refactoring + features where needed to fully accommodate this change.

  • Adds server fetcher feature (Closes [Telemetry] server side fetcher #49711)
    Allows telemetry to be fetched from the server instead of the browser if this kibana.yml config is set:

     telemetry.usageFetcher: server
    

    Currently this config accepts server and browser; in the future we might be adding a beat option.

    This telemetry config, like optIn can be overriden via saved objects. The reason behind this is to allow configuring this without restaring kibana.

  • Adds telemetry for telemetry (Closes Add telemetry for telemetry #50013) This currently reports:

     export interface TelemetryUsageStats {
       opt_in_status?: boolean | null;
       usage_fetcher?: 'browser' | 'server';
       last_reported?: number;
     }
    
  • Added user-agent tracking under ui metrics:

     "kibana-user_agent": [
       {
         "key": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.87 Safari/537.36",
         "value": 1
       }
     ]
    
  • tests for all telemetry configs

  • ui metric use kfetch instead of $http to hide the loading bar when sending ui metrics usage to kibana server and to solve https://github.com/elastic/kibana/pull/49671 (CC @justinkambic @epixa ).

  • refactor configs overriding in the telemetry saved object by adding a repository file to get/update saved objetcts. (Helps resolving *yml* based setting to ship telemetry (and disable the UI to opt out) #49751 (comment))


Notes

Added server fetcher feature to send telemetry from kibana server instead of browser under the flag telemetry.usageFetcher: server.

Introduced a new telemetry collector for the telemetry plugin which includes opt-in status, last report, and the type of fetcher the user is using.

Introduced user-agent tracking in ui-metrics to prevent losing this information if telemetry is sent from the server.

Replaced $http fetcher with kfetch as it $http shows a loader whenever we are sending telemetry from the browser. Finally Added a repository feature in telemetry to allow overriding configs via saved objects.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@Bamieh Bamieh marked this pull request as ready for review November 6, 2019 12:12
@Bamieh Bamieh requested review from a team and justinkambic November 6, 2019 12:12
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-stack-services (Team:Stack Services)

Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

From my side, this doesn't break the functionality my previous changes to some of this code fixed, so I am OK. I'm not going to approve because I didn't perform a full code review, but I am happy to help in that regard if needed.

Thank you for keeping me in the loop.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@Bamieh Bamieh requested a review from a team as a code owner November 6, 2019 15:33
@Bamieh
Copy link
Member Author

Bamieh commented Nov 12, 2019

@elasticmachine merge upstream

@Bamieh Bamieh requested a review from gmmorris November 12, 2019 22:00
Copy link
Contributor

@gmmorris gmmorris left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for running through the changes with me

@elasticmachine
Copy link
Contributor

💔 Build Failed

@Bamieh
Copy link
Member Author

Bamieh commented Nov 12, 2019

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@Bamieh Bamieh merged commit 35a5b77 into elastic:master Nov 13, 2019
@Bamieh Bamieh deleted the telemetry/server_fetcher branch November 13, 2019 07:19
Bamieh added a commit to Bamieh/kibana that referenced this pull request Nov 13, 2019
* initial push

* self code review

* ignore node-fetch type

* usageFetcher api

* user agent metric

* telemetry plugin collector

* remove extra unused method

* remove unused import

* type check

* fix collections tests

* pass kfetch as dep

* add ui metrics integration test for user agent

* dont start ui metrics when not authenticated

* user agent count always 1

* fix broken ui-metric integration tests

* try using config.get

* avoid fetching configs if sending

* type unknown -> string

* check if fetcher is causing the issue

* disable ui_metric from functional tests

* enable ui_metric back again

* ignore keyword above 256

* check requesting app first

* clean up after all the debugging :)

* fix tests

* always return 200 for ui metric reporting

* remove boom import

* logout after removing role/user

* undo some changes in tests

* inside try catch

* prevent potential race conditions in priorities with =

* use snake_case for telemetry plugin collection

* usageFetcher -> sendUsageFrom

* more replacements

* remove extra unused route

* config() -> config

* Update src/legacy/core_plugins/telemetry/index.ts

Co-Authored-By: Mike Côté <[email protected]>

* Update src/legacy/core_plugins/ui_metric/server/routes/api/ui_metric.ts

Co-Authored-By: Mike Côté <[email protected]>

* config() -> config

* fix SO update logic given the current changes

* fix opt in check

* triple check

* check for non boolean

* take into account older settings

* import TelemetryOptInProvider

* update test case
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 13, 2019
* upstream/master:
  Remove internal platform types exports (elastic#50427)
  [APM] Document `apm_oss.metricsIndices` and `apm_oss.sourcemap… (elastic#50312)
  [Telemetry] Server side fetcher (elastic#50015)
  [SIEM] Detection engine placeholders (elastic#50220)
  [Uptime] Donut chart loader position centered vertically  (elastic#50219)
  update telemetry banner notice text (elastic#50403)
  Fix aborting when searching without batching (elastic#49966)
  [Telemetry] Remove telemetry splash page and add conditional messaging (elastic#50189)
  Revert chromedriver update (elastic#50324)
  Remove deprecated argument include_type_name from ES calls (elastic#50285)
  [Maps] add settings to maps telemetry (elastic#50161)
  remove visualize loader (elastic#46910)
  Fix misuse of react-router and react-router-dom (elastic#50120)
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 13, 2019
…-fallback

* 'master' of github.com:elastic/kibana:
  Remove internal platform types exports (elastic#50427)
  [APM] Document `apm_oss.metricsIndices` and `apm_oss.sourcemap… (elastic#50312)
  [Telemetry] Server side fetcher (elastic#50015)
  [SIEM] Detection engine placeholders (elastic#50220)
  [Uptime] Donut chart loader position centered vertically  (elastic#50219)
  update telemetry banner notice text (elastic#50403)
  Fix aborting when searching without batching (elastic#49966)
  [Telemetry] Remove telemetry splash page and add conditional messaging (elastic#50189)
  Revert chromedriver update (elastic#50324)
  Remove deprecated argument include_type_name from ES calls (elastic#50285)
  [Maps] add settings to maps telemetry (elastic#50161)
  remove visualize loader (elastic#46910)
  Fix misuse of react-router and react-router-dom (elastic#50120)
  Move table-list-view to kibana-react (elastic#50046)
  [ML] Stats bar for data frame analytics (elastic#49464)
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 13, 2019
…ger-ace-theme

* 'master' of github.com:elastic/kibana: (56 commits)
  [ML] Server info service refactor (elastic#50302)
  Remove internal platform types exports (elastic#50427)
  [APM] Document `apm_oss.metricsIndices` and `apm_oss.sourcemap… (elastic#50312)
  [Telemetry] Server side fetcher (elastic#50015)
  [SIEM] Detection engine placeholders (elastic#50220)
  [Uptime] Donut chart loader position centered vertically  (elastic#50219)
  update telemetry banner notice text (elastic#50403)
  Fix aborting when searching without batching (elastic#49966)
  [Telemetry] Remove telemetry splash page and add conditional messaging (elastic#50189)
  Revert chromedriver update (elastic#50324)
  Remove deprecated argument include_type_name from ES calls (elastic#50285)
  [Maps] add settings to maps telemetry (elastic#50161)
  remove visualize loader (elastic#46910)
  Fix misuse of react-router and react-router-dom (elastic#50120)
  Move table-list-view to kibana-react (elastic#50046)
  [ML] Stats bar for data frame analytics (elastic#49464)
  [ML] Make navigation in tests more stable (elastic#50132)
  Migrate authorization subsystem to the new platform.  (elastic#46145)
  Bugfix: Interpreter conversion of string to number should throw on NaN elastic#27788 (elastic#50063)
  Update dependency @elastic/charts to v14 (elastic#49947)
  ...
mikecote pushed a commit that referenced this pull request Nov 13, 2019
* initial push

* self code review

* ignore node-fetch type

* usageFetcher api

* user agent metric

* telemetry plugin collector

* remove extra unused method

* remove unused import

* type check

* fix collections tests

* pass kfetch as dep

* add ui metrics integration test for user agent

* dont start ui metrics when not authenticated

* user agent count always 1

* fix broken ui-metric integration tests

* try using config.get

* avoid fetching configs if sending

* type unknown -> string

* check if fetcher is causing the issue

* disable ui_metric from functional tests

* enable ui_metric back again

* ignore keyword above 256

* check requesting app first

* clean up after all the debugging :)

* fix tests

* always return 200 for ui metric reporting

* remove boom import

* logout after removing role/user

* undo some changes in tests

* inside try catch

* prevent potential race conditions in priorities with =

* use snake_case for telemetry plugin collection

* usageFetcher -> sendUsageFrom

* more replacements

* remove extra unused route

* config() -> config

* Update src/legacy/core_plugins/telemetry/index.ts

Co-Authored-By: Mike Côté <[email protected]>

* Update src/legacy/core_plugins/ui_metric/server/routes/api/ui_metric.ts

Co-Authored-By: Mike Côté <[email protected]>

* config() -> config

* fix SO update logic given the current changes

* fix opt in check

* triple check

* check for non boolean

* take into account older settings

* import TelemetryOptInProvider

* update test case
mikecote pushed a commit that referenced this pull request Nov 13, 2019
* initial push

* self code review

* ignore node-fetch type

* usageFetcher api

* user agent metric

* telemetry plugin collector

* remove extra unused method

* remove unused import

* type check

* fix collections tests

* pass kfetch as dep

* add ui metrics integration test for user agent

* dont start ui metrics when not authenticated

* user agent count always 1

* fix broken ui-metric integration tests

* try using config.get

* avoid fetching configs if sending

* type unknown -> string

* check if fetcher is causing the issue

* disable ui_metric from functional tests

* enable ui_metric back again

* ignore keyword above 256

* check requesting app first

* clean up after all the debugging :)

* fix tests

* always return 200 for ui metric reporting

* remove boom import

* logout after removing role/user

* undo some changes in tests

* inside try catch

* prevent potential race conditions in priorities with =

* use snake_case for telemetry plugin collection

* usageFetcher -> sendUsageFrom

* more replacements

* remove extra unused route

* config() -> config

* Update src/legacy/core_plugins/telemetry/index.ts

Co-Authored-By: Mike Côté <[email protected]>

* Update src/legacy/core_plugins/ui_metric/server/routes/api/ui_metric.ts

Co-Authored-By: Mike Côté <[email protected]>

* config() -> config

* fix SO update logic given the current changes

* fix opt in check

* triple check

* check for non boolean

* take into account older settings

* import TelemetryOptInProvider

* update test case
@@ -39,28 +41,36 @@ export const createUiStatsReporter = (appName: string) => (
}
};

export const trackUserAgent = (appName: string) => {
if (telemetryReporter) {
return telemetryReporter.reportUserAgent(appName);
Copy link
Member

Choose a reason for hiding this comment

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

This popped up from someone's dev environment:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

As a note, this was caused by someone not running bootstrap

@alexfrancoeur alexfrancoeur mentioned this pull request Nov 15, 2019
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants