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] collect xpack.cloud details #31180

Merged
merged 6 commits into from
Feb 21, 2019
Merged

Conversation

Bamieh
Copy link
Member

@Bamieh Bamieh commented Feb 14, 2019

Removal of found plugin in cloud will break the ability in telemetry to determine if the metrics are coming from Cloud or not.

This Change adds the following to the telemetry metric under plugins:

{
   plugins: {
      cloud: {
        isCloudEnabled: true | false,
      }
   }
}

if cloud is not enabled, the telemetry will report isCloudEnabled: false. I favored this over not reporting at all when cloud is not enabled with the motivation to keep the telemetry object consistent in all cases.

@epixa
Copy link
Contributor

epixa commented Feb 14, 2019

I think the cloud id includes the uuid in plaintext. If so, that's probably the value we'd want to send rather than the whole cloud id.

The term id in this case is a little ambiguous, but it's really more than just a unique identifier.

Someone from the cloud team might be able to clarify here, and you can see cloud ids in action via the beats modules getting started instructions from the Kibana home page of a cluster deployed in Elastic Cloud.

@epixa epixa added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Feb 14, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@elasticmachine

This comment has been minimized.

@Bamieh
Copy link
Member Author

Bamieh commented Feb 15, 2019

@epixa i added the elasticsearch cluster uuid as esUUID. I kept the whole cloud ID because it is small and we can derive other information from it. Removed after Uri's message.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@epixa epixa left a comment

Choose a reason for hiding this comment

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

LGTM

const { id } = server.config().get(`xpack.cloud`);

return {
isCloudEnabled: !!id,
Copy link
Member

Choose a reason for hiding this comment

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

any reason to not check that the type is a string?

Copy link
Member Author

@Bamieh Bamieh Feb 21, 2019

Choose a reason for hiding this comment

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

Both should work for our case.

Checking for 'falesy' ids might protect us from unpredicted scenarios (empty string ids etc). More importantly; Checks for isCloudEnabled in other places in our code use !!id so this also ensures consistency (I'd favor consistency personally).

Copy link
Member

Choose a reason for hiding this comment

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

++ for consistency

@Bamieh Bamieh merged commit b60119e into elastic:master Feb 21, 2019
@Bamieh Bamieh deleted the cloud-telemetery branch February 21, 2019 15:29
Bamieh added a commit to Bamieh/kibana that referenced this pull request Feb 21, 2019
* collect xpack.cloud details

* add elasticsearch cluster uuid

* remove cloud ID from telemetry stats

* remove esUUID from telemtry stats
Bamieh added a commit to Bamieh/kibana that referenced this pull request Feb 21, 2019
* collect xpack.cloud details

* add elasticsearch cluster uuid

* remove cloud ID from telemetry stats

* remove esUUID from telemtry stats
Bamieh added a commit to Bamieh/kibana that referenced this pull request Feb 21, 2019
* collect xpack.cloud details

* add elasticsearch cluster uuid

* remove cloud ID from telemetry stats

* remove esUUID from telemtry stats
Bamieh added a commit that referenced this pull request Feb 21, 2019
* collect xpack.cloud details

* add elasticsearch cluster uuid

* remove cloud ID from telemetry stats

* remove esUUID from telemtry stats
Bamieh added a commit that referenced this pull request Feb 21, 2019
Backports the following commits to 7.x:
 - [Telemetry] collect xpack.cloud details  (#31180)
Bamieh added a commit that referenced this pull request Feb 24, 2019
* [Telemetry] collect xpack.cloud details (#31180)

* collect xpack.cloud details

* add elasticsearch cluster uuid

* remove cloud ID from telemetry stats

* remove esUUID from telemtry stats

* add stack_stats.kibana.plugins.cloud.isCloudEnabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Telemetry Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v6.7.0 v7.0.0 v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants