From b138ae9764406339e6f99341231a592d08acde51 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Tue, 6 Sep 2022 12:10:02 +0200 Subject: [PATCH 1/5] Fix Dialyzer issues --- lib/telemetry_metrics.ex | 2 ++ lib/telemetry_metrics/utils.ex | 14 +++++++++++--- mix.exs | 1 + 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/telemetry_metrics.ex b/lib/telemetry_metrics.ex index bfd7c4b..3ee7f02 100644 --- a/lib/telemetry_metrics.ex +++ b/lib/telemetry_metrics.ex @@ -9,6 +9,8 @@ defmodule Membrane.TelemetryMetrics do @type label() :: list() + @dialyzer [{:nowarn_function, emit_event?: 1}] + @doc """ Evaluates to conditional call to `:telemetry.execute/3` or to nothing, depending on if specific event is enabled in config file. If event is enabled, `:telemetry.execute/3` will be executed only if value returned by call to `func` will be truthly. diff --git a/lib/telemetry_metrics/utils.ex b/lib/telemetry_metrics/utils.ex index a6f8107..34585ad 100644 --- a/lib/telemetry_metrics/utils.ex +++ b/lib/telemetry_metrics/utils.ex @@ -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) diff --git a/mix.exs b/mix.exs index d9bce86..ae0b2c9 100644 --- a/mix.exs +++ b/mix.exs @@ -52,6 +52,7 @@ defmodule Membrane.TelemetryMetrics.Mixfile do if System.get_env("CI") == "true" do # Store PLTs in cacheable directory for CI + File.mkdir_p!(Path.join([__DIR__, "priv", "plts"])) [plt_local_path: "priv/plts", plt_core_path: "priv/plts"] ++ opts else opts From 33436e8ff97c7dbdd1db94e24277c1f5bac7ce22 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Tue, 6 Sep 2022 15:28:06 +0200 Subject: [PATCH 2/5] Add comment about supressing dialyzer errors --- lib/telemetry_metrics.ex | 4 +++- mix.exs | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/telemetry_metrics.ex b/lib/telemetry_metrics.ex index 3ee7f02..3fb7c3b 100644 --- a/lib/telemetry_metrics.ex +++ b/lib/telemetry_metrics.ex @@ -9,7 +9,9 @@ defmodule Membrane.TelemetryMetrics do @type label() :: list() - @dialyzer [{:nowarn_function, emit_event?: 1}] + # Suppresses Dialyzer false-positive error occurring due to fact, that function contains match on module attribute known at compile time but can be overridden in the config file. + # Dialyzer returns an error saying, that pattern can never match the type, but it might be not true, in the case when somebody will change its value in the custom config file. + @dialyzer [{:no_match, emit_event?: 1}] @doc """ Evaluates to conditional call to `:telemetry.execute/3` or to nothing, depending on if specific event is enabled in config file. diff --git a/mix.exs b/mix.exs index ae0b2c9..d9bce86 100644 --- a/mix.exs +++ b/mix.exs @@ -52,7 +52,6 @@ defmodule Membrane.TelemetryMetrics.Mixfile do if System.get_env("CI") == "true" do # Store PLTs in cacheable directory for CI - File.mkdir_p!(Path.join([__DIR__, "priv", "plts"])) [plt_local_path: "priv/plts", plt_core_path: "priv/plts"] ++ opts else opts From f0e208a6f2241920f509c7142d0a9eeca9160cdc Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Thu, 8 Sep 2022 18:38:27 +0200 Subject: [PATCH 3/5] Implement suggestion from CR --- lib/telemetry_metrics.ex | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/telemetry_metrics.ex b/lib/telemetry_metrics.ex index 3fb7c3b..1a65a84 100644 --- a/lib/telemetry_metrics.ex +++ b/lib/telemetry_metrics.ex @@ -9,10 +9,6 @@ defmodule Membrane.TelemetryMetrics do @type label() :: list() - # Suppresses Dialyzer false-positive error occurring due to fact, that function contains match on module attribute known at compile time but can be overridden in the config file. - # Dialyzer returns an error saying, that pattern can never match the type, but it might be not true, in the case when somebody will change its value in the custom config file. - @dialyzer [{:no_match, emit_event?: 1}] - @doc """ Evaluates to conditional call to `:telemetry.execute/3` or to nothing, depending on if specific event is enabled in config file. If event is enabled, `:telemetry.execute/3` will be executed only if value returned by call to `func` will be truthly. @@ -88,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 end end From 9a8e0d296d61064ac05793decd7b73cbbf1fca8b Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Mon, 12 Sep 2022 17:06:25 +0200 Subject: [PATCH 4/5] Implement suggestion from CR --- lib/telemetry_metrics.ex | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/telemetry_metrics.ex b/lib/telemetry_metrics.ex index 1a65a84..86797ac 100644 --- a/lib/telemetry_metrics.ex +++ b/lib/telemetry_metrics.ex @@ -4,8 +4,8 @@ defmodule Membrane.TelemetryMetrics do Provided macros evalueates to meaningful code or to nothing, depending on config values, in order to achieve performance boost, when specific event or whole telemetry is not in use. """ - @enabled Application.compile_env(:membrane_telemetry_metrics, :enabled, false) - @events Application.compile_env(:membrane_telemetry_metrics, :events, :all) + # @enabled Application.compile_env(:membrane_telemetry_metrics, :enabled, false) + # @events Application.compile_env(:membrane_telemetry_metrics, :events, :all) @type label() :: list() @@ -84,10 +84,13 @@ defmodule Membrane.TelemetryMetrics do end end + 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 + 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 From 04c16ac86d7f928dd2abae1285c92d3e0a110a59 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Mon, 26 Sep 2022 13:24:35 +0200 Subject: [PATCH 5/5] Remove commented code lines --- lib/telemetry_metrics.ex | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/telemetry_metrics.ex b/lib/telemetry_metrics.ex index 86797ac..594ccad 100644 --- a/lib/telemetry_metrics.ex +++ b/lib/telemetry_metrics.ex @@ -4,9 +4,6 @@ defmodule Membrane.TelemetryMetrics do Provided macros evalueates to meaningful code or to nothing, depending on config values, in order to achieve performance boost, when specific event or whole telemetry is not in use. """ - # @enabled Application.compile_env(:membrane_telemetry_metrics, :enabled, false) - # @events Application.compile_env(:membrane_telemetry_metrics, :events, :all) - @type label() :: list() @doc """