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] add spacesEnabled config back to xpack_main #43312

Merged
merged 4 commits into from
Aug 14, 2019

Conversation

Bamieh
Copy link
Member

@Bamieh Bamieh commented Aug 14, 2019

Closes #43208

This bug was introduced when I moved telemetry out of xpack_main. I did search the xpack_main plugin for spaces variables usage and the only place this variable was used was within telemetry code so I assumed it would be safe to move it with the plugin.

@Bamieh Bamieh requested a review from bmcconaghy August 14, 2019 19:58
@Bamieh Bamieh added the release_note:skip Skip the PR/issue when compiling release notes label Aug 14, 2019
Copy link
Contributor

@bmcconaghy bmcconaghy left a comment

Choose a reason for hiding this comment

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

Code LGTM

@kobelb
Copy link
Contributor

kobelb commented Aug 14, 2019

Should we remove the duplicate definition from here?

return {
telemetryEnabled: getXpackConfigWithDeprecated(config, 'telemetry.enabled'),
telemetryUrl: getXpackConfigWithDeprecated(config, 'telemetry.url'),
spacesEnabled: config.get('xpack.spaces.enabled'),
telemetryBanner: config.get('xpack.telemetry.banner'),
telemetryOptedIn: null,
activeSpace: null,

@Bamieh
Copy link
Member Author

Bamieh commented Aug 14, 2019

@kobelb these variables are being used in the telemetry code.

@kobelb
Copy link
Contributor

kobelb commented Aug 14, 2019

So... injected vars are available "globally". Any plugin can define an injected var which can then be consumed anywhere using chrome.getInjected('foo') and a bunch of other ways.

@Bamieh Bamieh requested a review from a team August 14, 2019 20:44
Copy link
Contributor

@kobelb kobelb left a comment

Choose a reason for hiding this comment

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

LGTM on green CI

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@Bamieh Bamieh requested a review from bmcconaghy August 14, 2019 23:04
Copy link
Contributor

@bmcconaghy bmcconaghy left a comment

Choose a reason for hiding this comment

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

Code LGTM

@Bamieh Bamieh merged commit 09196aa into elastic:master Aug 14, 2019
@Bamieh Bamieh deleted the telemetry/fix_bug#43208 branch August 14, 2019 23:08
@Bamieh Bamieh added the v7.4.0 label Aug 14, 2019
Bamieh added a commit to Bamieh/kibana that referenced this pull request Aug 14, 2019
* add spacesEnabled config back to xpack

* move some variables back to xpack_main

* fix import path
Bamieh added a commit to Bamieh/kibana that referenced this pull request Aug 14, 2019
* add spacesEnabled config back to xpack

* move some variables back to xpack_main

* fix import path
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 15, 2019
…p-metrics-selectall

* 'master' of github.com:elastic/kibana: (50 commits)
  [Uptime] update monitor list configs for mobile view (elastic#43218)
  [APM] Local UI filters (elastic#41588)
  [Code] Upgrade ctags langserver (elastic#43252)
  [Code] show multiple definition results in panel (elastic#43249)
  Adds Metric Type to full screen launch tracking (elastic#42692)
  [Canvas] Convert Autocomplete to Typescript (elastic#42502)
  [telemetry] add spacesEnabled config back to xpack_main (elastic#43312)
  [ML] Adds DF Transform Analytics list to Kibana management (elastic#43151)
  Add TLS client authentication support. (elastic#43090)
  [csp] Telemetry for csp configuration (elastic#43223)
  [SIEM] Run Cypress Tests Against Elastic Cloud & Cypress Command Line / Reporting (elastic#42804)
  docs: add tip on agent config in a dt (elastic#43301)
  [ML] Adding bucket span estimator to new wizards (elastic#43288)
  disable flaky tests (elastic#43017)
  Fix percy target branch for PRs (elastic#43160)
  [ML] Adding post create job options (elastic#43205)
  Restore discover histogram selection triggering fetch (elastic#43097)
  Per panel time range (elastic#43153)
  [Infra UI] Add APM to Metadata Endpoint (elastic#42197)
  Sentence case copy changes (elastic#43215)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes v7.3.1 v7.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting xpack.telemetry.enabled to false prevents access to roles ui
4 participants