Replace via_rule with via_rule_id#3109
Conversation
bblaszkow06
force-pushed
the
bb/via-rule-removal
branch
from
c71eb72 to
4875601
Compare
lib/logflare/rules/cache.ex
Outdated
| def get_lql_rules(ids) do | ||
| Cachex.execute!(__MODULE__, fn cache -> | ||
| for id <- Enum.uniq(ids) do | ||
| ContextCache.fetch(cache, {:get_lql_rule, [id]}, fn -> Rules.get_lql_rule(id) end) |
There was a problem hiding this comment.
Perhaps filtering out those that need to be fetched and then fetching all in one pass with a Repo.all would be more efficient... it would definitely reduce db load.
There was a problem hiding this comment.
It makes sense for the DB fetching, but not quite for caching, as we wouldn't want to duplicate entries for fetching e.g. [1, 2, 3] and [2, 3], and there's no straightforward way to fetch multiple ids from ETS cache. We could split the list, and group fetch the rest (that is missing from cache). Since get_lql_rules is removed, I'll consider it in a following PR for using in get_rules replacement
| lql_rule = | ||
| case le do | ||
| %{via_rule_id: nil} -> nil | ||
| %{via_rule_id: rule_id} -> Rules.Cache.get_lql_rule(rule_id) | ||
| end | ||
|
|
||
| maybe_broadcast("source:#{source.token}", "source:#{source.token}:new", %{ | ||
| body: body, | ||
| via_rule: le.via_rule && Map.take(le.via_rule, [:regex]), | ||
| via_lql_rule: lql_rule, |
There was a problem hiding this comment.
we can get rid of the lql rule broadcasting, it was effectively broken (regex rules was removed a long time ago) so there is no benefit in keeping it here.
| encoded_le = | ||
| le | ||
| |> Map.take([:body, :origin_source_uuid]) | ||
| |> Map.put(:via_lql_rule, lql_rule) | ||
| |> case do | ||
| %{body: %{"metadata" => %{"level" => level}}} = le | ||
| when level in ~W(debug info warning error alert critical notice emergency) -> | ||
| body = Map.put(le.body, "level", level) | ||
| %{le | body: body} | ||
|
|
||
| le -> | ||
| le | ||
| end |
There was a problem hiding this comment.
never been a fan the body manipulation logic here, lets take the opportunity to rip it out
| <%= if log.via_rule do %> | ||
| <span data-toggle="tooltip" data-placement="top" title={"Matching #{ log.via_rule.lql_string } routing from #{log.origin_source_uuid }"} style="color: ##5eeb8f;"> | ||
| <%= if log.via_lql_rule do %> | ||
| <span data-toggle="tooltip" data-placement="top" title={"Matching #{ log.via_lql_rule } routing from #{ log.origin_source_uuid }"} style="color: #5eeb8f;"> |
There was a problem hiding this comment.
i think we can just show a "Routed" icon if the log.via_rule_id is present, save us the trouble of fetching the data.
lib/logflare/rules.ex
Outdated
| @doc "Retrieves LQL representation of a rule" | ||
| @spec get_rule(non_neg_integer()) :: String.t() | nil | ||
| def get_lql_rule(id) do | ||
| Repo.one(from(r in Rule, where: r.id == ^id, select: r.lql_string)) | ||
| end | ||
|
|
There was a problem hiding this comment.
if we remove the lql string rendering then we don't really need this.
|
we probably can simplify a lot of legacy code here. |
2 participants
Uh oh!
There was an error while loading. Please reload this page.