Wrap :proc_lib label functionality

[nathanl]

I'm open to reworking this completely, but I wanted to propose making the OTP 27.0 :proc_lib.set_label/1 functionality easy to use.

I think we can make a lot of observability and error reporting clearer if we can start using these labels in a lot of places.

Example of effect: mtrudel/bandit#319

[nathanl]

@mtrudel 👀

[josevalim]

Generally speaking, we only add a new Erlang/OTP feature to Elixir once it is the minimum support Erlang/OTP 27 version. However, given this is implemented simply as a process dictionary, I think we can implement the set_label natively in Elixir, which is the most important one:

https://github.com/erlang/otp/blob/02273700b205acf853ce8bc9f58ad8aee91cd665/lib/stdlib/src/proc_lib.erl#L853-L867

So my suggestion:

1. Add only set_label (by directly writing to the pdict). get_label requires a feature that exists in OTP 26.2+
2. We need to decide if it will be called set_label or put_label

[mtrudel]

1. Add only set_label (by directly writing to the pdict).

Minor concern that this being in the pdict is really an implementation detail on OTP's part; nothing in the corresponding workup mandates that it can be accessed / changed via the pdict, or says anything about the key name. Maybe if we mark it with a TODO to update it to the public :proc_lib.set_label/1 API once we're wholly on OTP27+?

[josevalim]

Maybe if we mark it with a TODO to update it to the public :proc_lib.set_label/1 API once we're wholly on OTP27+?

Sounds good to me. We should also add a test, that runs only on Erlang/OTP 27+, that we can read the value with proc_lib.get_label.

[nathanl]

Add only set_label (by directly writing to the pdict). get_label requires a feature that exists in OTP 26.2+

If we're implementing set_label for now using Process.put/2, why not also implement get_label for now using Process.get/1? Setting a label will allow it to be shown in Observer, but if we can also get a label in Elixir, tools like Phoenix LiveDashboard can start showing labels, too.

Also, would it make sense to do this for now?

    if function_exported?(:proc_lib, :set_label, 1) do
      apply(:proc_lib, :set_label, [label])
    else
      # Use Process.put/2
    end

[josevalim]

If we're implementing set_label for now using Process.put/2, why not also implement get_label for now using Process.get/1?

Because get_label uses a feature in Erlang/OTP 26.2+ for reading a particular key of the process dictionary instead of copying the whole dictionary.

[josevalim]

Also, would it make sense to do this for now?

I wouldn't say so. Conditionals mean it will require more complicated testing to guarantee both branches work as expected. I prefer a temporary non-branching code.

[josevalim]

Looks good to me.

Last question is: put_label/1 or set_label/1? We use set_ prefix in a single place in Elixir, which is Node.set_cookie. Storing in the process dictionary itself is called Process.put.

[nathanl]

🤔 If we use set_label/1, that matches the name of the (eventual) underlying Erlang function (like put/2 does). If we use put_label/1, might that imply "uses the process dictionary" by its resemblance to put/2, even though that's an implementation detail that could change?

I would lean toward set_label/1, but I'm happy with whatever you (and/or other core members) think. I'm a happy user of many of your design decisions. 😄

[whatyouhide]

I’m for put_label for the name as we don't use set_ much in Elixir.

[josevalim]

Note: we are still deciding if it should be put_label or set_label.

[nathanl]

FWIW, "put label" to me sounds more like usages in maps, keywords, application env, and the process dictionary - putting a value at a key. Whereas "set label" sounds like "the process can have exactly one label", which is the case.

That's not a very strong point because the function signature and docs would immediately make that clear, but it does seem to me like a stronger point than "we don't use the word 'set' very much".

[josevalim]

Yes, that was the last topic of our discussion. There is effectively no collection here, so put feels awkward (albeit it is consistent).

[nathanl]

I revised the doc string based on @josevalim's helpful comment here.

I'd like to revise it again to clarify where the label may be seen "in crash logs"; it doesn't show up in the exception in my Phoenix application logs. Can anybody explain to me so that we can explain it here?

[josevalim]

I'd like to revise it again to clarify where the label may be seen "in crash logs"; it doesn't show up in the exception in my Phoenix application logs. Can anybody explain to me so that we can explain it here?

It is not going to show until we change Elixir to handle it, but it will be changed before v1.17 is out.

[sabiwara]

It is not going to show until we change Elixir to handle it, but it will be changed before v1.17 is out.

I think we did? Or is there more to be done? #13396

[josevalim]

Does that show up only for regular processes but also GenServer? What about our own, such as Task and DynamicSupervisor?

[sabiwara]

Does that show up only for regular processes but also GenServer? What about our own, such as Task and DynamicSupervisor?

Oh you're right, these have separate formatters. Sorry still unfamiliar with the logger codebase.
Will send a PR.

[josevalim]

💚 💙 💜 💛 ❤️

[nathanl]

Yay! I just published https://nathanmlong.com/2024/03/beam-process-labels/ discussing this

[mtrudel]

I'll be adding support for this to Bandit and Thousand Island shortly!