Implement stop method for probes #612
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tombruijn
requested changes
Feb 11, 2022
unflxw
force-pushed
the
implement-probes-stop-method
branch
from
1a92d5f to
9b4f269
Compare
tombruijn
approved these changes
Feb 11, 2022
packages/nodejs/.changesets/stop-minutely-probes-when-stopping-appsignal.md
Outdated
Show resolved
Hide resolved
unflxw
force-pushed
the
implement-probes-stop-method
branch
2 times, most recently
from
6b47eb5 to
58a597e
Compare
unflxw
added a commit
that referenced
this pull request
Feb 11, 2022
These tests would assert the opposite behaviour of that which they claimed to test. The test itself was broken, as it was spying on a previously created client object, then creating a new client object and performing actions on it, [as pointed out by @tombruijn in #612][comment]. This commit fixes the existing tests so they perform the right assertions, and adds a new one for the behaviour implied by the tests. [comment]: #612 (comment)
luismiramirez
approved these changes
Feb 14, 2022
jeffkreeftmeijer
approved these changes
Feb 14, 2022
Implement a `.stop` method on the `BaseProbes` class and on the
`Probes` interface, along with a read-only `.isRunning` property.
The new `BaseProbes` object handles only the stopping logic. The
actual behaviour of the probes is delegated to two private classes:
`BaseProbeRunner` (which used to be `BaseProbes`) and
`NoopProbeRunner` (which used to be `NoopProbes`). Both of these
implement the `ProbeRunner` private interface. Stopping the probes
permanently switches the implementation used.
`NoopProbes` no longer exists, but `BaseProbes({ run: false })`
can be used to initialise it in the stopped state, which is
functionally equivalent.
Call `metrics().probes().stop()` from the `BaseClient.stop()`
method, ensuring the probes system is stopped when AppSignal is
stopped.
Fixes #418 and closes #569.
unflxw
force-pushed
the
implement-probes-stop-method
branch
from
58a597e to
afe221e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement a
.stopmethod on theBaseProbesclass and on theProbesinterface, along with a read-only.isRunningproperty.The new
BaseProbesobject handles only the stopping logic. The actual behaviour of the probes is delegated to two private classes:BaseProbeRunner(which used to beBaseProbes) andNoopProbeRunner(which used to beNoopProbes). Both of these implement theProbeRunnerprivate interface. Stopping the probes permanently switches the implementation used.NoopProbesno longer exists, butBaseProbes({ run: false })can be used to initialise it in the stopped state, which is functionally equivalent.Call
metrics().probes().stop()from theBaseClient.stop()method, ensuring the probes system is stopped when AppSignal is stopped.Fixes #418 and closes #569.