Skip to content

Commit 23d947b

Browse files
varsillmat-hek
andauthored
Use Process.label() instead of Process.register(). (#1071)
* Use Process.label() instead of Process.register(). Deprecate :components option for :unsafely_name_processes_for_stalker * Add report_links_to_observer config option in favour of deprecated unsafely_name_processes_for_observer: [:links] * Update required Elixir version * Remove quote --------- Co-authored-by: Mateusz Front <mateusz.front@swmansion.com>
1 parent a5c43db commit 23d947b

11 files changed

Lines changed: 59 additions & 54 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## Latest
44
* Improve remove_link action docs
5+
* Deprecate `:components` option for `:unsafely_name_processes_for_observer`
6+
* Deprecate `:links` option for `:unsafely_name_processes_for_observer` in favour of `:report_links_to_observer` configuration entry
7+
* [Set label](https://hexdocs.pm/elixir/Process.html#set_label/1) for all the Membrane processes (i.e. for both Membrane components like pipelines, elements and bins and for utility components like subprocess supervisors)
8+
* Update required Elixir version to `~> 1.17`
59

610
## 1.2.6
711
* Add tutorials [#1007](https://github.com/membraneframework/membrane_core/pull/1007), plugins [#1012](https://github.com/membraneframework/membrane_core/pull/1012) and demos [#1013](https://github.com/membraneframework/membrane_core/pull/1013) to the docs.

assets/images/observer_graph.png

286 KB
Loading

lib/membrane/component_path.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ defmodule Membrane.ComponentPath do
2828
"""
2929
@spec format(path()) :: String.t()
3030
def format(path) do
31-
Enum.join(path)
31+
Enum.join(path, "/")
3232
end
3333

3434
@doc """

lib/membrane/core/bin.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,12 @@ defmodule Membrane.Core.Bin do
106106
observability_config = %{
107107
name: name,
108108
component_type: :bin,
109-
pid: self(),
110109
parent_path: options.parent_path,
111110
log_metadata: options.log_metadata
112111
}
113112

114113
Membrane.Core.Stalker.register_component(options.stalker, observability_config)
114+
115115
SubprocessSupervisor.set_parent_component(options.subprocess_supervisor, observability_config)
116116

117117
{:ok, clock_proxy} =

lib/membrane/core/element.ex

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ defmodule Membrane.Core.Element do
112112
observability_config = %{
113113
name: options.name,
114114
component_type: :element,
115-
pid: self(),
116115
parent_path: options.parent_path,
117116
log_metadata: options.log_metadata
118117
}

lib/membrane/core/pipeline.ex

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,14 @@ defmodule Membrane.Core.Pipeline do
3131

3232
defp do_init(params) do
3333
observability_config = %{
34-
name: params.name,
35-
component_type: :pipeline,
36-
pid: self()
34+
name: params.name || "Pipeline#{:erlang.pid_to_list(self())}",
35+
component_type: :pipeline
3736
}
3837

3938
%{subprocess_supervisor: subprocess_supervisor} = params
39+
4040
SubprocessSupervisor.set_parent_component(subprocess_supervisor, observability_config)
41+
4142
stalker = Stalker.new(observability_config, subprocess_supervisor)
4243

4344
{:ok, resource_guard} =

lib/membrane/core/pipeline/supervisor.ex

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,10 @@ defmodule Membrane.Core.Pipeline.Supervisor do
4242
subprocess_supervisor = SubprocessSupervisor.start_link!()
4343

4444
with {:ok, pipeline} <- start_fun.(subprocess_supervisor) do
45+
name = name || "Pipeline#{:erlang.pid_to_list(pipeline)}"
46+
4547
Membrane.Core.Stalker.setup_component_utility(
46-
%{name: name, component_type: :pipeline, pid: pipeline},
48+
%{name: name, component_type: :pipeline},
4749
"Pipeline supervisor"
4850
)
4951

lib/membrane/core/stalker.ex

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ defmodule Membrane.Core.Stalker do
1212
[]
1313
)
1414

15+
@report_links_to_observer Application.compile_env(
16+
:membrane_core,
17+
:report_links_to_observer,
18+
false
19+
)
20+
1521
@metrics_enabled Application.compile_env(:membrane_core, :enable_metrics, true)
1622

1723
@scrape_interval 1000
@@ -25,8 +31,7 @@ defmodule Membrane.Core.Stalker do
2531
optional(:parent_path) => ComponentPath.path(),
2632
optional(:log_metadata) => Logger.metadata(),
2733
name: Membrane.Child.name(),
28-
component_type: :element | :bin | :pipeline,
29-
pid: pid()
34+
component_type: :element | :bin | :pipeline
3035
}
3136

3237
@type link_observability_data :: %{
@@ -117,9 +122,7 @@ defmodule Membrane.Core.Stalker do
117122
end
118123

119124
# Sets component path, logger metadata and adds necessary entries to the process dictionary
120-
# Also registers the process with a meaningful name for easier introspection with
121-
# stalker if enabled by setting `unsafely_name_processes_for_observer: :components`
122-
# in config.exs.
125+
# and labels the process with a meaningful name for easier introspection with stalker
123126
defp setup_process_local_observability(config, opts) do
124127
config = parse_observability_config(config, opts)
125128

@@ -132,7 +135,7 @@ defmodule Membrane.Core.Stalker do
132135
do: Process.put(:__membrane_pipeline__, true)
133136

134137
Logger.metadata(config.log_metadata)
135-
register_name_for_stalker(config)
138+
set_label(config)
136139

137140
ComponentPath.set(config.component_path)
138141

@@ -144,54 +147,36 @@ defmodule Membrane.Core.Stalker do
144147
defp parse_observability_config(config, opts) do
145148
utility_name = Map.get(opts, :utility_name)
146149
stalker = Map.get(opts, :stalker)
147-
%{name: name, component_type: component_type, pid: pid} = config
148-
is_name_provided = name != nil
149-
pid_string = pid |> :erlang.pid_to_list() |> to_string()
150-
name = if is_name_provided, do: name, else: pid_string
150+
%{name: name, component_type: component_type} = config
151151
is_utility = utility_name != nil
152152

153-
name_string = """
154-
#{if is_binary(name) and String.valid?(name), do: name, else: inspect(name)}\
155-
#{if component_type == :element, do: "", else: "/"}\
156-
"""
153+
name_string = if is_binary(name) and String.valid?(name), do: name, else: inspect(name)
157154

158155
%{
159156
stalker: stalker,
160157
name: name,
161158
name_string: name_string,
162-
pid_string: pid_string,
163159
component_type: component_type,
164-
is_name_provided: is_name_provided,
165160
is_utility: is_utility,
166161
utility_name: if(is_utility, do: " #{utility_name}", else: ""),
167162
component_path: Map.get(config, :parent_path, []) ++ [name_string],
168163
log_metadata: Map.get(config, :log_metadata, [])
169164
}
170165
end
171166

172-
if :components in @unsafely_name_processes_for_observer do
173-
defp register_name_for_stalker(config) do
174-
if Process.info(self(), :registered_name) == {:registered_name, []} do
175-
Process.register(
176-
self(),
177-
"""
178-
##{config.pid_string} #{if config.is_name_provided, do: config.name_string}\
179-
#{unless config.is_name_provided, do: " (#{config.component_type})"}#{config.utility_name}"\
180-
"""
181-
|> String.to_atom()
182-
)
183-
end
167+
defp set_label(config) do
168+
utility_str = if config.is_utility, do: "'s #{config.utility_name}", else: ""
169+
component_path_str = ComponentPath.format(config.component_path)
184170

185-
:ok
186-
end
187-
else
188-
defp register_name_for_stalker(_config), do: :ok
171+
label = "#{component_path_str}#{utility_str}"
172+
173+
Process.set_label(label)
189174
end
190175

191176
@doc """
192177
Generates observability data needed for reporting links and their metrics.
193178
194-
If optionally turned on by setting `unsafely_name_processes_for_observer: :links` in
179+
If optionally turned on by setting `report_links_to_observer: true` in
195180
config.exs, starts processes to reflect pads structure in the process tree for visibility
196181
in Erlang observer.
197182
"""
@@ -204,12 +189,29 @@ defmodule Membrane.Core.Stalker do
204189
}
205190
end
206191

192+
if :components in @unsafely_name_processes_for_observer do
193+
IO.warn("""
194+
Deprecated `:components` value for :unsafely_name_processes_for_observer` configuration.
195+
Now the processes are always labeled so there is no need to use this option anymore.
196+
""")
197+
end
198+
207199
if :links in @unsafely_name_processes_for_observer do
200+
IO.warn("""
201+
Deprecated `:links` value for `:unsafely_name_processes_for_observer` configuration.
202+
Instead please use the following configuration:
203+
```
204+
config :membrane_core, report_links_to_observer: true
205+
```
206+
""")
207+
end
208+
209+
if @report_links_to_observer do
208210
defp run_link_dbg_process(pad_ref, observability_data) do
209211
{:ok, observer_dbg_process} =
210212
Task.start_link(fn ->
211213
Process.flag(:trap_exit, true)
212-
Process.register(self(), :"pad #{inspect(pad_ref)} #{:erlang.pid_to_list(self())}")
214+
Process.set_label(self(), "pad #{inspect(pad_ref)} #{:erlang.pid_to_list(self())}")
213215
process_to_link = Map.get(observability_data, :observer_dbg_process)
214216
if process_to_link, do: Process.link(process_to_link)
215217

lib/membrane/pipeline.ex

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,7 @@ defmodule Membrane.Pipeline do
4545
### Visualizing the supervision tree
4646
4747
Use the [Applications tab](https://www.erlang.org/doc/apps/observer/observer_ug#applications-tab) in Erlang's Observer GUI
48-
(or the `Kino` library in Livebook) to visualize a pipeline's internal supervision tree. Use the following configuration for debugging purposes only:
49-
50-
config :membrane_core, unsafely_name_processes_for_observer: [:components]
51-
52-
This improves the readability of the Observer's process tree graph by naming the pipeline descendants, as demonstrated here:
48+
(or the `Kino` library in Livebook) to visualize a pipeline's internal supervision tree.
5349
5450
![Observer graph](assets/images/observer_graph.png).
5551
"""

mix.exs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ defmodule Membrane.Mixfile do
99
[
1010
app: :membrane_core,
1111
version: @version,
12-
elixir: "~> 1.12",
12+
elixir: "~> 1.17",
1313
elixirc_paths: elixirc_paths(Mix.env()),
1414
description: "Membrane Multimedia Framework (Core)",
1515
dialyzer: dialyzer(),

0 commit comments

Comments
 (0)