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] Swallow errors in opt-in remote notification from the server #75641

Merged

Conversation

afharo
Copy link
Member

@afharo afharo commented Aug 21, 2020

Summary

This PR catches any errors when the server tries to notify the remote telemetry service about Opt-In/Opt-Out events but it can't reach it for whatever reason.

We are not awaiting the promise anymore to avoid timing out the Kibana request when the connection to the remote service times out.

Fixes #75637

For maintainers

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-telemetry (Team:KibanaTelemetry)

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

Can we log these somewhere to track how many times we encounter them? I'd prefer us knowing about their frequency rather than blinding swallowing them.

@afharo afharo force-pushed the telemetry/swallow-opt-in-remote-notification branch from dcee693 to 0bc6b96 Compare August 24, 2020 08:52
@afharo
Copy link
Member Author

afharo commented Aug 24, 2020

@TinaHeiligers fair point! I've added the log entry.

I've tested it by setting telemetry.optInStatusUrl: https://localhost in the file kibana.yml (to make sure it fails to connect) and I can see this log entry:

server    log   [09:48:40.436] [warning][plugins][telemetry] Failed to notify "https://localhost" from the server about the opt-in selection. Possibly blocked by a firewall? - Error: request to https://localhost failed, reason: connect ECONNREFUSED 127.0.0.1:443

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

Copy link
Contributor

@TinaHeiligers TinaHeiligers 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 f49f010 into elastic:master Aug 24, 2020
@afharo afharo deleted the telemetry/swallow-opt-in-remote-notification branch August 24, 2020 16:28
afharo added a commit to afharo/kibana that referenced this pull request Aug 24, 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
Feature:Telemetry release_note:fix Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.9.1 v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Telemetry] Opt-in/out notifications from server should not crash the Kibana request
5 participants