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

Fix Dialyzer issues #4

Merged
merged 5 commits into from
Sep 26, 2022
Merged

Fix Dialyzer issues #4

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b138ae9
Fix Dialyzer issues
FelonEkonom Sep 6, 2022
33436e8
Add comment about supressing dialyzer errors
FelonEkonom Sep 6, 2022
f0e208a
Implement suggestion from CR
FelonEkonom Sep 8, 2022
9a8e0d2
Implement suggestion from CR
FelonEkonom Sep 12, 2022
04c16ac
Remove commented code lines
FelonEkonom Sep 26, 2022
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
12 changes: 5 additions & 7 deletions lib/telemetry_metrics.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,10 @@ defmodule Membrane.TelemetryMetrics do
end
end

defp emit_event?(event) do
cond do
not @enabled -> false
@events == :all -> true
is_list(@events) -> event in @events
true -> false
end
cond do
not @enabled -> defp emit_event?(_event), do: false
@events == :all -> defp emit_event?(_event), do: true
is_list(@events) -> defp emit_event?(event), do: event in @events
true -> defp emit_event?(_event), do: false
Copy link
Member

Choose a reason for hiding this comment

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

I've told you to check for warnings 😛
When MIX_ENV=test it's fine (thus CI passes) but in dev when @enabled is false, there's a warning about unused module attribute.

I've found a way to solve it - use variables for code executed at compile time:

Suggested change
cond do
not @enabled -> defp emit_event?(_event), do: false
@events == :all -> defp emit_event?(_event), do: true
is_list(@events) -> defp emit_event?(event), do: event in @events
true -> defp emit_event?(_event), do: false
enabled = Application.compile_env(:membrane_telemetry_metrics, :enabled, false)
events = Application.compile_env(:membrane_telemetry_metrics, :events, :all)
cond do
not enabled -> defp emit_event?(_event), do: false
events == :all -> defp emit_event?(_event), do: true
is_list(events) -> defp emit_event?(event), do: event in events
true -> defp emit_event?(_event), do: false

end
end
14 changes: 11 additions & 3 deletions lib/telemetry_metrics/utils.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
defmodule Membrane.TelemetryMetrics.Utils do
@moduledoc false

@type handler_config :: %{
:ets => :ets.tid() | atom(),
optional(:measurement) => Telemetry.Metrics.measurement()
}

@cleanup_event_prefix :__membrane_telemetrymetrics_cleanup__

@spec attach_metric_handler(:telemetry.event_name(), :telemetry.handler_function(), %{
ets: :ets.tid() | atom()
}) :: [reference()]
@spec attach_metric_handler(
:telemetry.event_name(),
:telemetry.handler_function(),
handler_config()
) ::
[reference()]
def attach_metric_handler(event_name, handler_function, %{ets: ets} = config) do
handler_id = make_ref()
:telemetry.attach(handler_id, event_name, handler_function, config)
Expand Down