Skip to content

[Telemetry] Use staging if it's not a distributable release #63875

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 20, 2020
Merged

[Telemetry] Use staging if it's not a distributable release #63875

merged 3 commits into from
Apr 20, 2020

Conversation

@afharo
Copy link
Member

@afharo afharo commented Apr 17, 2020

Summary

At the moment we rely on the dev flag to decide whether to push the telemetry to staging or production.
We can use the dist flag instead. This flag is true only when running from a distributable (either built from source or downloaded), but not when running as node scripts/kibana start.

Maybe we can get confirmation this is the expected behaviour from the @elastic/kibana-operations team.

For maintainers

@afharo afharo added Team:Operations Kibana-Operations Team Feature:Telemetry v8.0.0 release_note:skip Skip the PR/issue when compiling release notes :Telemetry v7.8.0 labels Apr 17, 2020
@afharo afharo requested a review from a team as a code owner April 17, 2020 16:45
@elasticmachine
Copy link
Contributor

elasticmachine commented Apr 17, 2020

Pinging @elastic/pulse (Team:Pulse)

@afharo
Copy link
Member Author

afharo commented Apr 17, 2020

During the build process, another flag is set: release. It is false for snapshots and true only for final releases. I don't think we want to use that flag at the moment (we may want customers running nightly builds to push data to our production cluster), or do we?
@alexfrancoeur what are your thoughts on this?

spalger
spalger reviewed Apr 17, 2020
View reviewed changes
src/plugins/telemetry_collection_manager/server/plugin.ts Outdated
constructor(initializerContext: PluginInitializerContext) {
this.logger = initializerContext.logger.get();
this.isDev = initializerContext.env.mode.dev;
this.isProd = initializerContext.env.packageInfo.dist; // Is it a distributable release? => isProd
Copy link
Contributor

@spalger spalger Apr 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

"prod" and "dev" mean something different than "dist" and "source", this probably shouldn't be called isProd

@Bamieh
Copy link
Member

Bamieh commented Apr 20, 2020

What is the difference between prod and dist?

@afharo
Copy link
Member Author

afharo commented Apr 20, 2020

What is the difference between prod and dist?

@Bamieh from here:
https://github.com/elastic/kibana/blob/master/src/core/server/config/env.ts#L122-L136

prod and dev are uniquely based on whether Kibana is run in --dev mode (either via env var or via cli flag).
dist is based on the flag package.json>build.distributable, only set when running node scripts/build. Either if we release it or it's compiled from source, dist: true, but when running Kibana as node scripts/kbn or yarn kbn, dist: false.

Anyway, I'll update in a sec this PR about @spalger 's comment. He is right: we shouldn't mix their names :)

@afharo
Copy link
Member Author

afharo commented Apr 20, 2020

I'm wondering if these changes mean we can revert the ones from this PR: #63356

On one hand, I'd rather keep it so we don't populate staging with (potentially broken) garbage, as PRs might contain malformed documents.
On the flip side, I think we should report telemetry during the builds (to staging, of course) because that could bring up potential crashes.

Actually, the potentially broken garbage reason has never been a problem when pushing the data to Live and during the dev time, those documents are likely to be pushed to staging anyway (when the dev is running Kibana locally).

Before pushing a commit reverting those changes, I'd like to know your thoughts @elastic/pulse ?

@kibanamachine
Copy link
Contributor

kibanamachine commented Apr 20, 2020

💚 Build Succeeded

History

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

Bamieh
Bamieh approved these changes Apr 20, 2020
View reviewed changes
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. I still think we should keep the enforced staging urls for testing in any case.

@afharo afharo merged commit 9226d4c into elastic:master Apr 20, 2020
@afharo afharo deleted the telemetry/isProd-based-on-distributable branch April 20, 2020 11:43
@afharo afharo mentioned this pull request Apr 20, 2020
Merged
afharo added a commit to afharo/kibana that referenced this pull request Apr 20, 2020
@afharo
[Telemetry] Use staging if it's not a distributable release (elastic#…
4af7b73 
…63875)

* [Telemetry] Use staging if it's not a distributable release (instead of 'dev' vs. 'prod' approach)

* Rename isProd to isDistributable and useProdKey
afharo added a commit that referenced this pull request Apr 20, 2020
@afharo
[Telemetry] Use staging if it's not a distributable release (#63875) (#…
4bde4a8 
…63955)

* [Telemetry] Use staging if it's not a distributable release (instead of 'dev' vs. 'prod' approach)

* Rename isProd to isDistributable and useProdKey
@lukeelmers lukeelmers added the Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// label Oct 1, 2021
@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 1, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Bamieh Bamieh Bamieh approved these changes

+1 more reviewer

@spalger spalger spalger left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// Team:Operations Kibana-Operations Team v7.8.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@afharo @elasticmachine @Bamieh @kibanamachine @spalger @lukeelmers