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

✨ [RUMF-1207] collect concurrent actions #1421

Closed
wants to merge 7 commits into from

Conversation

BenoitZugmeyer
Copy link
Member

Motivation

Ignore less click actions by sending concurrent automatic actions.

Changes

Please refer to the commit messages

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

Let's start with a light refactoring: this commit splits the
`parentContexts` logic in two to make it easier to follow view and
action parent context collection independently.
* Simplify the test context from `{ value: string }` to `string`. As
  we'll write a few more tests, it will help to have more concise
  values.

* `startClocks` is in fact called `startTime` in the code

* Add a test to make sure that calling `find()` without argument does
  not return the last closed context.
@BenoitZugmeyer BenoitZugmeyer requested a review from a team as a code owner March 17, 2022 18:28
Because we'll have multiple concurrent action entries, we need to change
the `ContextHistory` to support this. With this commit:

* instead of keeping a reference to the current active context, the
  context history now returns the reference with associated methods to
  "close" and "remove" it. It is now up to the code using context
  history to keep one or multiple references to the active contexts.

* a new `findAll` method is introduced returning a list of contexts
  instead of a single context that we'll use to get multiple action ids.
This commit changes the `action.id` typings to support either a single
string or an array. It impacts:
* any event that can be an action child
* the internal monitoring context
* the internal context, so logs will also inherit from this change
This changes `parentContexts.findAction` to track multiple pending
actions that would be sent through the LifeCycle. Instead of using a
single variable to track the current history entry, it uses a map
associating the action id to the history entry.
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/concurrent-actions branch from dcf18df to 41f110a Compare March 17, 2022 18:29
We need to call `waitIdlePage` for every created action, similarly to
`trackEventCounts`. This commit moves it closer to `trackEventCounts`,
so the action lifecycle is contained at a single place. It will be
easier to manage multiple concurrent actions.
Finally, remove the "one action at a time" limitation and collect a new
action for every click, even if another one is ongoing.

To do this, we still need to keep a reference to pending actions because
we need to discard them:
* when a view is created (this will be removed in a future PR)
* when stopping action collections (for tests)

This is somewhat cumbersome, but not a huge deal either.
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/concurrent-actions branch from 41f110a to 9d508dd Compare March 17, 2022 18:31
@BenoitZugmeyer
Copy link
Member Author

Superseded by #1432

@BenoitZugmeyer BenoitZugmeyer deleted the benoit/concurrent-actions branch March 21, 2022 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant