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

[EBT] ElasticV3-Server - Check connectivity as soon as optIn(true) is called #136936

Conversation

afharo
Copy link
Member

@afharo afharo commented Jul 22, 2022

Summary

Resolves #135647.

The shipper ElasticV3-Server now reacts to OptIn value changes to check connectivity as soon as it's set. Instead of waiting 1 minute for the next tick of the timer.

While I was at it, I fixed a couple of gotchas:

  • We used concatMap in the connectivity checks. It could trigger multiple requests at the same. For a connectivity check, it makes sense to trigger only one at a time, so I switched it to exhaustMap.
  • I split a very long test into smaller chunks. I recommend using the Hide whitespace changes toggle when reviewing.

Checklist

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
If Kibana sends optIn(true) too many times, we might trigger requests more often. Low Medium That call is triggered by user interaction. I would not expect them to call it that often to hurt their own cluster. However, if we really think it could be an issue, we can add .pipe(distinct()) to only react to changes in the value.

For maintainers

@afharo afharo added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.4.0 labels Jul 22, 2022
@afharo afharo requested a review from a team as a code owner July 22, 2022 09:57
@elasticmachine
Copy link
Contributor

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

@afharo afharo added v8.4.0 and removed v8.4.0 labels Jul 22, 2022
@TinaHeiligers
Copy link
Contributor

@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

@TinaHeiligers TinaHeiligers enabled auto-merge (squash) July 25, 2022 16:38
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
@kbn/analytics-shippers-elastic-v3-server 15 18 +3

Total ESLint disabled count

id before after diff
@kbn/analytics-shippers-elastic-v3-server 15 18 +3

History

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

@TinaHeiligers TinaHeiligers merged commit 0824234 into elastic:main Jul 25, 2022
@afharo afharo deleted the ebt/v3_server_shipper/check_connectivity_on_opt-in_change branch October 17, 2022 14:41
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:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EBT] Kibana Server won't confirm it's online until 1 minute has passed since optIn(true)
5 participants