-
Notifications
You must be signed in to change notification settings - Fork 254
telemetry: add no-op functionality to SegmentClient to make sure errors in initialization are not fatal #4100
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just not sure about the isInitProperly
name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would like to understand what it takes not to keep state. The NoopClient
should work without specific knowledge to what happened... and if needed, a typecheck would be preferred.
I think something went wrong during rebase, as I see some stuff about snyk and the However, the rest looks good to me, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, I'd squash the 2 commits though, we don't need to have the isInitProperly
version of the code in the upstream git history.
The shortlog for both commits overflows on github, it's supposed to be kept short, with a longer description in the longer commit log.
telemetry: add no-op functionality
Currently, when telemetry initialization fails, this is a hard failure for the `crc` command and daemon, and they exit.
This commit handles this more gracefully, when there's a telemetry initialization failure, an empty `NoOp` `Client` instance is returned, and all `Client` methods check for this `NoOp` instance and return early if needed.
About 20 to 30 characters. |
Currently, when telemetry initialization fails, this is a hard failure for the `crc` command and daemon, and they exit. This commit handles this more gracefully, when there's a telemetry initialization failure, an empty `Noop Client` instance is returned, and all `Client` methods check for this `Noop` instance and return early if needed.
Fixes: Issue #4085
A new field has been added to Client struct signalling whether the client has been initialized properly. Default value is
false
and it is set totrue
only if the initialization went as expected.Methods on Client check if this field is true before progressing further.
In case of any error during initialization, an empty Client is returned (and its methods do nothing).