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

fix: reduce use of length/1 whenever not needed #1770

Merged
merged 1 commit into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 lib/logflare/account_email.ex
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ defmodule Logflare.AccountEmail do

diff = diff_schema(schema_to_list(formatted_new), schema_to_list(formatted_old))

if diff == [] do
if Enum.empty?(diff) do
# Something generates BOOL and something else generates BOOLEAN which causes this
Logger.error("Schema update email send with no new fields.",
source_id: source.token,
Expand Down
2 changes: 1 addition & 1 deletion lib/logflare/auth.ex
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ defmodule Logflare.Auth do
defp check_scopes(token_scopes, required) do
cond do
"private" in token_scopes -> :ok
required == [] -> :ok
Enum.empty?(required) -> :ok
Enum.any?(token_scopes, fn scope -> scope in required end) -> :ok
true -> {:error, :unauthorized}
end
Expand Down
2 changes: 1 addition & 1 deletion lib/logflare/endpoints/cache.ex
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ defmodule Logflare.Endpoints.Cache do
refresh(proactive_querying_ms(state))
{:noreply, %{state | query_tasks: tasks}}

tasks == [] ->
Enum.empty?(tasks) ->
{:stop, :normal, state}

true ->
Expand Down
10 changes: 5 additions & 5 deletions lib/logflare/logs/log_events.ex
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ defmodule Logflare.Logs.LogEvents do
le = LogEvent.make_from_db(row, %{source: source})
{:ok, le}

%{rows: rows} when length(rows) >= 1 ->
%{rows: rows} ->
row = Enum.find(rows, &(&1.id == id))
le = LogEvent.make_from_db(row, %{source: source})
{:ok, le}
Expand Down Expand Up @@ -150,14 +150,14 @@ defmodule Logflare.Logs.LogEvents do

defp process(result) do
case result do
{:ok, %{rows: rows}} when length(rows) > 1 ->
{:error, "Multiple rows returned, expected one"}
{:ok, %{rows: []}} ->
nil

{:ok, %{rows: [row]}} ->
row

{:ok, %{rows: []}} ->
nil
{:ok, %{rows: _rows}} ->
{:error, "Multiple rows returned, expected one"}

{:error, error} ->
{:error, error}
Expand Down
3 changes: 2 additions & 1 deletion lib/logflare/logs/logs.ex
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ defmodule Logflare.Logs do
)
when is_binary(drop_lql_string) do
cond do
length(filters) >= 1 && SourceRouting.route_with_lql_rules?(le, %Rule{lql_filters: filters}) ->
not Enum.empty?(filters) and
SourceRouting.route_with_lql_rules?(le, %Rule{lql_filters: filters}) ->
Map.put(le, :drop, true)

true ->
Expand Down
3 changes: 2 additions & 1 deletion lib/logflare/logs/lql/lql_encoder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ defmodule Logflare.Lql.Encoder do
|> Enum.map_join(" ", &to_fragment/1)
|> String.replace("chart:timestamp", "")

{"m.level", filter_rules} when length(filter_rules) >= 2 ->
# `filter_rules` has at least 2 entries
{"m.level", [_, _ | _] = filter_rules} ->
{min_level, max_level} =
Enum.min_max_by(filter_rules, &Parser.Helpers.get_level_order(&1.value))

Expand Down
10 changes: 6 additions & 4 deletions lib/logflare/logs/lql/lql_parser.ex
Original file line number Diff line number Diff line change
Expand Up @@ -123,14 +123,16 @@ defmodule Logflare.Lql.Parser do

defp maybe_cast_value(c, {:list, type}), do: maybe_cast_value(c, type)

defp maybe_cast_value(%{values: values, value: nil} = c, type) when length(values) >= 1 do
defp maybe_cast_value(%{values: values, value: nil} = c, type) when values != [] do
%{
c
| values:
values
|> Enum.map(&%{value: &1, path: c.path})
|> Enum.map(&maybe_cast_value(&1, type))
|> Enum.map(& &1.value)
|> Enum.map(fn data ->
%{value: data, path: c.path}
|> maybe_cast_value(type)
|> Map.fetch!(:value)
end)
}
end

Expand Down
2 changes: 1 addition & 1 deletion lib/logflare/logs/search/logs_search_operations.ex
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ defmodule Logflare.Logs.SearchOperations do
so.tailing? and not Enum.empty?(so.lql_ts_filters) ->
Utils.halt(so, @timestamp_filter_with_tailing)

length(so.chart_rules) > 1 ->
Logflare.Utils.List.at_least?(so.chart_rules, 2) ->
Utils.halt(so, "Only one chart rule can be used in a query")

match?([_], so.chart_rules) and
Expand Down
2 changes: 1 addition & 1 deletion lib/logflare/logs/search/logs_search_operations_helpers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ defmodule Logflare.Logs.SearchOperations.Helpers do
%{min: min, max: max, message: message}
end

def get_min_max_filter_timestamps(ts_filters, _chart_period) when length(ts_filters) > 1 do
def get_min_max_filter_timestamps(ts_filters, _chart_period) when is_list(ts_filters) do
{min, max} =
ts_filters
|> Enum.map(& &1.value)
Expand Down
6 changes: 4 additions & 2 deletions lib/logflare/logs/search/lql_validator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ defmodule Logflare.Lql.Validator do
@moduledoc false
import Logflare.Logs.SearchOperations.Helpers

alias Logflare.Utils.List, as: ListH

@timestamp_filter_with_tailing "Timestamp filters can't be used if live tail search is active"
@default_max_n_chart_ticks 250

Expand All @@ -20,10 +22,10 @@ defmodule Logflare.Lql.Validator do
tailing? and not Enum.empty?(lql_ts_filters) ->
@timestamp_filter_with_tailing

length(chart_rules) > 1 ->
ListH.at_least?(chart_rules, 2) ->
"Only one chart rule can be used in a LQL query"

match?([_], chart_rules) and
ListH.exactly?(chart_rules, 1) and
hd(chart_rules).value_type not in ~w[integer float]a and
hd(chart_rules).path != "timestamp" ->
chart_rule = hd(chart_rules)
Expand Down
2 changes: 1 addition & 1 deletion lib/logflare/logs/source_parsers/syslog_parser.ex
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ defmodule Logflare.Logs.SyslogParser do
defp merge_syslog_sd(tokens) when is_list(tokens) do
{sd_element_values, new_tokens} = Keyword.pop_values(tokens, :sd_element)

if length(sd_element_values) > 0 do
if not Enum.empty?(sd_element_values) do
sd =
sd_element_values
|> Enum.map(fn sd_element ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ defmodule Logflare.Logs.Validators.EqDeepFieldTypes do
is_list_of_enums(merged) ->
deep_merge_enums(merged)

merged == [] ->
Enum.empty?(merged) ->
{:list, :empty}

is_homogenous_list(merged) ->
Expand Down
10 changes: 5 additions & 5 deletions lib/logflare/single_tenant.ex
Original file line number Diff line number Diff line change
Expand Up @@ -261,13 +261,13 @@ defmodule Logflare.SingleTenant do
seed_plan = if default_plan, do: :ok

seed_sources =
if default_user do
if Sources.list_sources_by_user(default_user) |> length() > 0, do: :ok
if default_user && not Enum.empty?(Sources.list_sources_by_user(default_user)) do
:ok
end

seed_endpoints =
if default_user do
if Endpoints.list_endpoints_by(user_id: default_user.id) |> length() > 0, do: :ok
if default_user && not Enum.empty?(Endpoints.list_endpoints_by(user_id: default_user.id)) do
:ok
end

source_schemas_updated =
Expand Down Expand Up @@ -310,7 +310,7 @@ defmodule Logflare.SingleTenant do
state.field_count > 3
end

Enum.all?(checks) and length(sources) > 0
Enum.all?(checks) and not Enum.empty?(sources)
else
false
end
Expand Down
24 changes: 13 additions & 11 deletions lib/logflare/source/rate_counter_server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -140,18 +140,12 @@ defmodule Logflare.Source.RateCounterServer do
# TODO: optimize by not recalculating total sum and average
new_queue = LQueue.push(bucket.queue, state.last_rate)

average =
stats =
new_queue
|> Enum.to_list()
|> average()
|> round()
|> stats()

sum =
new_queue
|> Enum.to_list()
|> Enum.sum()

{length, %{bucket | queue: new_queue, average: average, sum: sum}}
{length, %{bucket | queue: new_queue, average: stats.avg, sum: stats.sum}}
end
end)
end
Expand Down Expand Up @@ -274,8 +268,16 @@ defmodule Logflare.Source.RateCounterServer do
Sources.Counters.get_inserts(source_id)
end

def average(xs) when is_list(xs) do
Enum.sum(xs) / length(xs)
def stats(xs) when is_list(xs) do
{total, count} =
Enum.reduce(xs, {0, 0}, fn v, {total, count} ->
{total + v, count + 1}
end)

%{
avg: total / count,
sum: total
}
end

defp init_counters(source_id, bigquery_project_id) when is_atom(source_id) do
Expand Down
11 changes: 3 additions & 8 deletions lib/logflare/sources.ex
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ defmodule Logflare.Sources do
rules =
for rule <- rules do
case rule do
%Rule{lql_filters: lql_filters} when length(lql_filters) >= 1 ->
%Rule{lql_filters: lql_filters} when lql_filters != [] ->
rule

%Rule{regex_struct: rs} when not is_nil(rs) ->
Expand Down Expand Up @@ -341,15 +341,10 @@ defmodule Logflare.Sources do
%{source | metrics: new_metrics}
end

def valid_source_token_param?(string) when is_binary(string) do
case String.length(string) === 36 && Ecto.UUID.cast(string) do
{:ok, _} -> true
_ -> false
end
def valid_source_token_param?(string) do
hauleth marked this conversation as resolved.
Show resolved Hide resolved
match?({:ok, _}, Ecto.UUID.dump(string))
end

def valid_source_token_param?(_), do: false

def delete_slack_hook_url(source) do
source
|> Source.changeset(%{slack_hook_url: nil})
Expand Down
6 changes: 3 additions & 3 deletions lib/logflare/sources/rules.ex
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ defmodule Logflare.Rules do
%Rule{regex_struct: rs}, _ when is_nil(rs) ->
{:cont, false}

%Rule{regex_struct: rs, lql_filters: lf}, _ when is_map(rs) and length(lf) >= 1 ->
{:cont, false}

%Rule{regex_struct: rs, lql_filters: []}, _ when is_map(rs) ->
{:halt, true}

%Rule{regex_struct: rs}, _ when is_map(rs) ->
{:cont, false}
end)
end

Expand Down
4 changes: 2 additions & 2 deletions lib/logflare/sources/source_routing.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ defmodule Logflare.Logs.SourceRouting do
rule.regex_struct || if(rule.regex != nil, do: Regex.compile!(rule.regex), else: nil)

cond do
length(rule.lql_filters) >= 1 and route_with_lql_rules?(le, rule) ->
not Enum.empty?(rule.lql_filters) and route_with_lql_rules?(le, rule) ->
do_route(le, rule)

regex_struct != nil and Regex.match?(regex_struct, body["event_message"]) ->
Expand Down Expand Up @@ -53,7 +53,7 @@ defmodule Logflare.Logs.SourceRouting do

@spec route_with_lql_rules?(LE.t(), Rule.t()) :: boolean()
def route_with_lql_rules?(%LE{body: le_body}, %Rule{lql_filters: lql_filters})
when length(lql_filters) >= 1 do
when lql_filters != [] do
lql_rules_match? =
Enum.reduce_while(lql_filters, true, fn lql_filter, _acc ->
%Lql.FilterRule{path: path, value: value, operator: operator, modifiers: mds} = lql_filter
Expand Down
54 changes: 54 additions & 0 deletions lib/logflare/utils/list.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
defmodule Logflare.Utils.List do
hauleth marked this conversation as resolved.
Show resolved Hide resolved
@doc """
Check if `list`'s lenght is *exactly* `n`

It does the same check as `length(list) == n`, but it will do at most `n`
steps through the list, while `length/1` will always step through whole `list`.
This can have negative performance impact if `lenght(list)` is much larger
than `n`.

## Example

```elixir
iex> #{__MODULE__}.exactly?([1, 2, 3], 2)
false
iex> #{__MODULE__}.exactly?([1, 2, 3], 3)
true
iex> #{__MODULE__}.exactly?([1, 2, 3], 4)
false
```
"""
@spec exactly?(list :: list(), n :: non_neg_integer()) :: boolean()
def exactly?([], 0), do: true
def exactly?([], _), do: false
def exactly?([_ | _], 0), do: false

def exactly?([_ | rest], n) when is_integer(n) and n > 0,
do: exactly?(rest, n - 1)

@doc """
Check if `list`'s length is *at least* of `n`

It does the same check as `length(list) >= n`, but it will do at most `n`
steps through the list, while `length/1` will always step through whole `list`.
This can have negative performance impact if `lenght(list)` is much larger
than `n`.

## Example

```elixir
iex> #{__MODULE__}.at_least?([1, 2, 3], 2)
true
iex> #{__MODULE__}.at_least?([1, 2, 3], 3)
true
iex> #{__MODULE__}.at_least?([1, 2, 3], 4)
false
```
"""
@spec at_least?(list :: list(), n :: non_neg_integer()) :: boolean()
def at_least?([], n) when n > 0, do: false
def at_least?(list, 0) when is_list(list), do: true

def at_least?([_ | rest], n) when is_integer(n) and n > 0,
do: at_least?(rest, n - 1)
end
20 changes: 12 additions & 8 deletions lib/logflare_web/live/endpoints/actions/show.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,25 @@
curl "<%= url(~p"/api/endpoints/query/#{@show_endpoint.token}") %>" \
-H 'X-API-KEY: YOUR-ACCESS-TOKEN' \
-H 'Content-Type: application/json; charset=utf-8'
<span :if={length(@declared_params) > 0}>\</span>
<span :if={length(@declared_params) > 0}>
-G <%= Enum.map(@declared_params, fn p -> "-d \"#{p}=VALUE\"" end) |> Enum.join(" ") %>
</span>
<%= if not Enum.empty?(@declared_params) do %>
<span>\</span>
<span>
-G <%= Enum.map_join(@declared_params, " ", fn p -> "-d \"#{p}=VALUE\"" end) %>
</span>
<% end %>
</code>

<code :if={@show_endpoint.enable_auth} class="tw-whitespace-pre-wrap">
# By name
curl "<%= url(~p"/api/endpoints/query/#{@show_endpoint.name}") %>" \
-H 'X-API-KEY: YOUR-ACCESS-TOKEN' \
-H 'Content-Type: application/json; charset=utf-8'
<span :if={length(@declared_params) > 0}>\</span>
<span :if={length(@declared_params) > 0}>
-G <%= Enum.map(@declared_params, fn p -> "-d \"#{p}=VALUE\"" end) |> Enum.join(" ") %>
</span>
<%= if not Enum.empty?(@declared_params) do %>
<span>\</span>
<span>
-G <%= Enum.map_join(@declared_params, " ", fn p -> "-d \"#{p}=VALUE\"" end) %>
</span>
<% end %>
</code>
</div>
</section>
2 changes: 1 addition & 1 deletion lib/logflare_web/live/search_live/logs_search_lv.ex
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ defmodule LogflareWeb.Source.SearchLV do
defp warning_message(assigns, search_op) do
tailing? = assigns.tailing?
querystring = assigns.querystring
log_events_empty? = search_op.events.rows == []
log_events_empty? = Enum.empty?(search_op.events.rows)

cond do
log_events_empty? and not tailing? ->
Expand Down
5 changes: 2 additions & 3 deletions lib/logflare_web/live/source_backends_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ defmodule LogflareWeb.SourceBackendsLive do
<div class="my-4">
<%= if !@show_create_form do %>
<button class="btn btn-primary" phx-click="toggle-create-form">Add a backend</button>
<% end %>
<%= if @show_create_form do %>
<% else %>
<.form
:let={f}
for={%{}}
Expand Down Expand Up @@ -60,7 +59,7 @@ defmodule LogflareWeb.SourceBackendsLive do
<%= submit("Add", class: "btn btn-primary") %>
</.form>
<% end %>
<div :if={@source_backends != []}>
<div :if={not Enum.empty?(@source_backends)}>
Backends:
<ul>
<li :for={sb <- @source_backends}>
Expand Down
Loading
Loading