Skip to content

Conversation

@unflxw
Copy link
Contributor

@unflxw unflxw commented Sep 12, 2024

Part of https://github.com/appsignal/integration-guide/issues/169.

Ensure stubs are called exactly once

[@tombruijn could you take a look? I tried more reasonable approaches within webmock before reaching this count-requests-separately solution, but maybe I missed something in webmock that would solve this in a less awkward way]

Ensure that the stubs are called exactly once -- that is, that the
same stub is not used several times, potentially masking a bug where
several transmissions happen for the same events, and that no stubs
are declared which are left unused.

The strange way in which it is implemented is due to certain quirks
of how the webmock gem works. I could not find a way to have a
stub automatically disable itself when called, similar to what the
nock Node.js package provides -- so I implemented this by passing
a number of request bodies to expect to respond to when declaring
the mock, and erroring if more requests are made than the request
bodies provided.

That, however, does not cover the scenario where the stub is not
called at all. Somewhat bafflingly, the have_been_requested matcher
does not check a counter that has been incremented for each request.
Instead, it re-runs the requests it has stored by the stubs again,
and counts how many match. Given the implementation described above,
where the stub is exhausting a list of request bodies as it goes,
the matcher will fail when attempting to go a second time around.

So instead, we increment a counter when a request is processed
(without said request having exhausted the matcher) and we return
that counter from the stub. The tests store these counters in a
let binding, and an after block checks that each stored counter
has been increased to one. This is done in an after block not
only for convenience, but because some tests are mocking a call
that will only be performed in that same after block, when the
scheduler is stopped.

Implement heartbeat check-ins

Add an Appsignal::CheckIn.heartbeat helper that emits a single
heartbeat for the check-in identifier given.

When given continuous: true as the second argument, it spawns a
separate thread that emits a heartbeat every thirty seconds. This
is a convenience method for the use case where the heartbeat is
only meant as a check that the process is alive.

Split functions that deal with different event kinds out of the
scheduler and test them independently.

Ensure that the stubs are called exactly once -- that is, that the
same stub is not used several times, potentially masking a bug where
several transmissions happen for the same events, and that no stubs
are declared which are left unused.

The strange way in which it is implemented is due to certain quirks
of how the `webmock` gem works. I could not find a way to have a
stub automatically disable itself when called, similar to what the
`nock` Node.js package provides -- so I implemented this by passing
a number of request bodies to expect to respond to when declaring
the mock, and erroring if more requests are made than the request
bodies provided.

That, however, does not cover the scenario where the stub is not
called at all. Somewhat bafflingly, the `have_been_requested` matcher
does not check a counter that has been incremented for each request.
Instead, it re-runs the requests it has stored by the stubs _again_,
and counts how many match. Given the implementation described above,
where the stub is exhausting a list of request bodies as it goes,
the matcher will fail when attempting to go a second time around.

So instead, we increment a counter when a request is processed
(without said request having exhausted the matcher) and we return
that counter from the stub. The tests store these counters in a
`let` binding, and an `after` block checks that each stored counter
has been increased to one. This is done in an `after` block not
only for convenience, but because some tests are mocking a call
that will only be performed in that same `after` block, when the
scheduler is stopped.
@unflxw unflxw added chore A small task that takes a day or two at the most. feature A new feature for this component. labels Sep 12, 2024
@unflxw unflxw self-assigned this Sep 12, 2024
@unflxw unflxw removed the chore A small task that takes a day or two at the most. label Sep 12, 2024
@unflxw unflxw force-pushed the implement-heartbeat-checkins branch from 2abbd5e to 6077571 Compare September 12, 2024 12:08
Comment on lines +2 to +12
class RequestCounter
attr_reader :count

def initialize
@count = 0
end

def increase
@count += 1
end
end
Copy link
Member

Choose a reason for hiding this comment

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

@tombruijn could you take a look? I tried more reasonable approaches within webmock before reaching this count-requests-separately solution, but maybe I missed something in webmock that would solve this in a less awkward way

I'll take a look later. Your implementation seems to work, but a bit weird.

@unflxw unflxw force-pushed the implement-heartbeat-checkins branch 2 times, most recently from c5537ff to d20954b Compare September 13, 2024 14:12
@backlog-helper

This comment has been minimized.

@unflxw unflxw force-pushed the implement-heartbeat-checkins branch from d20954b to 96bd3f6 Compare September 16, 2024 12:21
Add an `Appsignal::CheckIn.heartbeat` helper that emits a single
heartbeat for the check-in identifier given.

When given `continuous: true` as the second argument, it spawns a
separate thread that emits a heartbeat every thirty seconds. This
is a convenience method for the use case where the heartbeat is
only meant as a check that the process is alive.

Split functions that deal with different event kinds out of the
scheduler and test them independently.
@unflxw unflxw force-pushed the implement-heartbeat-checkins branch from 96bd3f6 to 7ae7152 Compare September 16, 2024 13:40
@backlog-helper

This comment has been minimized.

7 similar comments
@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper
Copy link


This is a message from the daily scheduled checks.

New issue guide | Backlog management | Rules | Feedback

@unflxw unflxw merged commit 2bc8175 into main Sep 26, 2024
122 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature A new feature for this component.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants