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.

Sign up for GitHub

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

feat: Add telemetry middleware. #93

Merged
merged 2 commits into from
Feb 28, 2024
Merged

feat: Add telemetry middleware. #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
baeed5a
improvement: don't incur compile-time dependencies on middleware.
jimsynz Feb 28, 2024
b626c87
feat: Add a middleware which emits telemetry events about Reactor.
jimsynz Feb 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion documentation/dsls/DSL:-Reactor.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ Call functions before and after a group of steps.
| Name | Type | Default | Docs |
|------|------|---------|------|
| [`before_all`](#reactor-group-before_all){: #reactor-group-before_all .spark-required} | `(any, any, any -> any) \| mfa` | | The before function. See `Reactor.Step.Group` for more information. |
| [`after_all`](#reactor-group-after_all){: #reactor-group-after_all .spark-required} | `(any, any, any -> any) \| mfa` | | The after function. See `Reactor.Step.Group` for more information. |
| [`after_all`](#reactor-group-after_all){: #reactor-group-after_all .spark-required} | `(any -> any) \| mfa` | | The after function. See `Reactor.Step.Group` for more information. |
| [`allow_async?`](#reactor-group-allow_async?){: #reactor-group-allow_async? } | `boolean` | `true` | Whether the emitted steps should be allowed to run asynchronously. |


Expand Down
1 change: 1 addition & 0 deletions lib/reactor/dsl/middleware.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ defmodule Reactor.Dsl.Middleware do
target: __MODULE__,
args: [:module],
identifier: :module,
modules: [:module],
schema: [
module: [
type: {:behaviour, Middleware},
Expand Down
16 changes: 16 additions & 0 deletions lib/reactor/executor/hooks.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ defmodule Reactor.Executor.Hooks do
@doc "Run the init hooks collecting the new context as it goes"
@spec init(Reactor.t(), Reactor.context()) :: {:ok, Reactor.context()} | {:error, any}
def init(reactor, context) do
context =
Map.put(context, :__reactor__, %{
id: reactor.id,
inputs: reactor.inputs,
middleware: reactor.middleware,
step_count: step_count(reactor),
initial_state: reactor.state
})

Utils.reduce_while_ok(reactor.middleware, context, fn middleware, context ->
if function_exported?(middleware, :init, 1) do
middleware.init(context)
Expand Down Expand Up @@ -94,4 +103,11 @@ defmodule Reactor.Executor.Hooks do

:ok
end

defp step_count(reactor) when is_nil(reactor.plan), do: length(reactor.steps)

defp step_count(reactor) do
vertices = Graph.num_vertices(reactor.plan)
length(reactor.steps) + vertices
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why, but this seems strange to me. The number of steps is the vertices in the plan plus the steps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct. Any steps that have already been planned will be in the graph, whereas any unplanned steps are stored in the steps list until the next loop through the reactor.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we store a step count in the state as steps are added so that we don't have to do lots of counting? I'm not sure if num_vertices/1 is constant time or not, but if not we might want to consider keeping both counts in the state. Probably not big lists, but also if we count every time for every invocation of every middleware, it could end up being lots of calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pretty sure it just calls map_size behind the scenes. we could cache it, but since step_count is only called by the init callback, it's not happening very often. 🤷

end
end
Loading
Loading