use private functions as report_cb#775
Conversation
|
@hauleth does it need to be a public function too? IIRC yes but double checking! |
|
Yeah, it needs to be public function and capture needs to use remote call format. Personally I use such approach most of the time: # Do not show in documentation
@doc false
# underscore functions aren't imported by default AFAIK
def __report_cb__(report) do
# ...
endThat way it is public function from VM viewpoint, but it is private API from documentation viewpoint. |
|
I'll make the change, but at this point I have to ask for more details. What is that quirk we're talking about and where can I read about it? It sounds like it'd be nice to document it somewhere if reports are to get any adoption. |
|
Ok I think I found where it's coming from. Telemetry recommends remote captures for performance reasons, but looking at (this thread)[https://github.com/elixir-lang/elixir/pull/13668] it looks like the eventual conclusion was that performance difference might not be noticeable with recent OTP versions. So the only justification that landed in the docs is hot code reload. Which is still a good enough reason to use remote captures! |
bbb2250 to
feb571a
Compare
|
Pushed the change! I didn't spot any difference from underscores (as tested with |
feb571a to
a43913a
Compare
|
oof i misread "import" for "export" 🤦 ok, thanks for bearing with me folks, this new push should finally be good! |
@hauleth raised a good point that anonymous functions shouldn't be used as report callbacks. This PR uses private functions instead. Embarrassingly, this isn't the first time I'm making this mistake.
Anyways, I went with a private function per log message, but open to other arrangements (separate module? a single function with multiple heads matching on
log_id?) if you think it's better.