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

Remove support for deprecated xpack.telemetry configurations #51142

Merged
merged 17 commits into from
Apr 2, 2020

Conversation

epixa
Copy link
Contributor

@epixa epixa commented Nov 20, 2019

In 7.5, we moved telemetry to OSS and dropped the xpack prefix for the
telemetry plugin configuration options. We deprecated the usage of the
xpack prefix so any existing usage would trigger a warning at startup.

In 8.0, we remove support for the deprecated xpack prefix configs for
telemetry.

Closes #51141

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-docs (Team:Docs)

@elasticmachine
Copy link
Contributor

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

@epixa epixa requested a review from a team November 20, 2019 01:33
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@Bamieh Bamieh self-requested a review November 20, 2019 10:26
Copy link
Member

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

We need to remove a few extra lines of code from the telemetry plugin and xpack_main before merging this on master.

@epixa
Copy link
Contributor Author

epixa commented Nov 20, 2019

@Bamieh Can you clarify? I'm not aware of the code you're referring to, but I'll go remove it if you point me in its direction.

@epixa
Copy link
Contributor Author

epixa commented Nov 20, 2019

@gchaps Can you help me out on this docs failure? I'm not sure how since this is simply adding a new section that doesn't link outward, but apparently it broke the stack upgrade docs?

The `xpack.` prefix has been removed for all telemetry configurations.

*Impact:*
For any configurations beginning with `xpack.telemetry.`, remove the `xpack.` prefix. For example, use <<telemetry.enabled, `telemetry.enabled`>> instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

Because this content gets pulled into the Stack docs, the link needs to be formatted as follows:

{kibana-ref}/monitoring-settings-kb.html#monitoring-general-settings[telemetry.enabled]

Note: This link goes to the section heading because I was unable to get the link to telemetry.enabled to work. Also, linking to the section heading gives the reader more context.

Copy link
Member

Choose a reason for hiding this comment

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

@afharo i believe we need to address this one before merging.

Copy link
Member

Choose a reason for hiding this comment

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

yup! It's even failing the build. I'll work on it today...

I was wondering why we have that config parameter in the monitoring file instead of a specific doc file for telemetry where we can explain all the configuration properties.

@Bamieh
Copy link
Member

Bamieh commented Nov 20, 2019

@epixa I believe your comment is before our sync today where we've discussed these changes. If you still need me to write them in a comment please let me know.

@elasticmachine
Copy link
Contributor

Pinging @elastic/pulse (Team:Pulse)

@TinaHeiligers
Copy link
Contributor

@Bamieh Would you mind taking ownership of this task please? It's not urgent.

@Bamieh Bamieh self-assigned this Mar 17, 2020
In 7.5, we moved telemetry to OSS and dropped the xpack prefix for the
telemetry plugin configuration options. We deprecated the usage of the
xpack prefix so any existing usage would trigger a warning at startup.

In 8.0, we remove support for the deprecated xpack prefix configs for
telemetry.
@afharo afharo force-pushed the remove-xpack-telemetry-config branch from 3e908f4 to 4996348 Compare March 23, 2020 18:21
@epixa epixa requested a review from a team as a code owner March 23, 2020 18:21
@afharo afharo assigned afharo and unassigned Bamieh Mar 23, 2020
@afharo afharo requested a review from Bamieh March 23, 2020 18:24
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

LGTM for platform changes

afharo and others added 9 commits April 1, 2020 18:57
Co-Authored-By: gchaps <33642766+gchaps@users.noreply.github.com>
Co-Authored-By: gchaps <33642766+gchaps@users.noreply.github.com>
Co-Authored-By: gchaps <33642766+gchaps@users.noreply.github.com>
Co-Authored-By: gchaps <33642766+gchaps@users.noreply.github.com>
Co-Authored-By: gchaps <33642766+gchaps@users.noreply.github.com>
Co-Authored-By: gchaps <33642766+gchaps@users.noreply.github.com>
Co-Authored-By: gchaps <33642766+gchaps@users.noreply.github.com>
Co-Authored-By: gchaps <33642766+gchaps@users.noreply.github.com>
Co-Authored-By: gchaps <33642766+gchaps@users.noreply.github.com>
Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

@afharo we need to remove the telemetry configs from xpack_main:
x-pack/legacy/plugins/xpack_main/index.js

@afharo
Copy link
Member

afharo commented Apr 2, 2020

@Bamieh good call! I'm surprised it was there anyway... since that file enforces configPrefix: 'xpack.xpack_main', so the specified telemetry config would have the accumulated prefix xpack.xpack_main.telemetry.*.

Anyway... removed!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@Bamieh
Copy link
Member

Bamieh commented Apr 2, 2020

@afharo it was needed to support the legacy config to be passed to the kibana configs.

Copy link
Member

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

LGTM

@afharo afharo merged commit a729b3b into elastic:master Apr 2, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 2, 2020
* master:
  Switch to embeddable factory interface with optional override (elastic#61165)
  fix text error in diagrams (elastic#62101)
  [Index management] Prepare support Index template V2 format (elastic#61588)
  Updates dashboard images (elastic#62011)
  [Maps] remove MapBounds type (elastic#62332)
  [Uptime] Convert anomaly Jobs name to lowercase to comply with… (elastic#62293)
  Make d3 place nicely with object values (elastic#62004)
  EMT-287: update schema with elastic agent id (elastic#62252)
  [Maps] fix replaceLayerList to handle case where map is not intialized (elastic#62202)
  Remove support for deprecated xpack.telemetry configurations (elastic#51142)
  [Uptime] Remove static constant for index name completely (elastic#62256)
  [APM] E2E: install dependencies for vanilla workspaces (elastic#62178)
  [backport] Bump to 5.1.3 (elastic#62286)
  Show server name in Remote Cluster detail panel (elastic#62250)
  Rename some alert types (elastic#61693)
  changing duration type to ms, s, m (elastic#62265)
  [ML] Clear Kibana index pattern cache on creation or form reset. (elastic#62184)
  Move `src/legacy/server/index_patterns` to data plugin (server) (Remove step) (elastic#61618)
  [NP] Remove IndexedArray usage in Schemas (elastic#61410)
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 6, 2020
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 51142 or prevent reminders by adding the backport:skip label.

@afharo
Copy link
Member

afharo commented Apr 7, 2020

This is for 8.x. No need to backport it

@afharo afharo added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Apr 7, 2020
@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
backport:skip This commit does not require backporting Feature:Telemetry release_note:breaking Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Docs v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove deprecated xpack prefix on telemetry configurations
9 participants