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

Jump to bottom

Provide some more pointers for logger overload protection #13855

Merged
merged 2 commits into from
Sep 24, 2024
Merged

Provide some more pointers for logger overload protection #13855

merged 2 commits into from
Sep 24, 2024

Conversation

PragTob
Copy link
Contributor

@PragTob PragTob commented Sep 24, 2024

The previous docs didn't mention that logger may drop messages entirely, which I think is important to call out (and not just because I finished a long debugging journey to find these :) ).

This should give a fair overview of the built-in overload protections, similar to what I tried to do in erlang/otp#8855

I think this can really help folks be aware of some of these.

As always, thank you a lot for all your work 💚

@PragTob
Provide some more pointers for logger overload protection
c5eef5c 
The previous docs didn't mention that logger may drop messages
entirely, which I think is important to call out (and not just
because I finished a long debugging journey to find these :) ).

This should give a fair overview of the built-in overload
protections, similar to what I tried to do in erlang/otp#8855

I think this can really help folks be aware of some of these.
PragTob
PragTob commented Sep 24, 2024
View reviewed changes
lib/logger/lib/logger.ex
* Alternates between sync and async modes to remain
performant when required but also apply back-pressure
when under stress.
* Provides multiple forms of [overload protection](https://www.erlang.org/doc/apps/kernel/logger_chapter.html#protecting-the-handler-from-overload):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The line is too long, unsure how to break it up or if there's a better way to link to it - but I found calling out the specific chapter for details valuable.

Copy link
Member

Choose a reason for hiding this comment

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

It's fine, no worries!

PragTob
PragTob commented Sep 24, 2024
View reviewed changes
lib/logger/lib/logger.ex
* Provides multiple forms of [overload protection](https://www.erlang.org/doc/apps/kernel/logger_chapter.html#protecting-the-handler-from-overload):
* keeps track of its message queue and switches to sync mode to apply
back pressure or even drop messages
* limits the number of logs emitted defaulting to 500 per second
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm unsure what level of detail to provide as defaults may change, however I found it surprising how compartively low this limit is and so I found it worth calling out

@PragTob
Copy link
Contributor Author

PragTob commented Sep 24, 2024

CI failed on something unrelated, I can't restart it - I could push an empty commit but that probably wouldn't be great 🤔

   1) test purges consolidation path if asked (Mix.Tasks.Compile.ElixirTest)
     /home/runner/work/elixir/elixir/lib/mix/test/mix/tasks/compile.elixir_test.exs:723
     ** (EXIT from #PID<0.8822.0>) an exception was raised:
         ** (UndefinedFunctionError) function List.Chars.to_charlist/1 is undefined or private
             (elixir 1.18.0-dev) List.Chars.to_charlist(~c"/home/runner/work/elixir/elixir/bin/../lib/ex_unit/ebin")
             (elixir 1.18.0-dev) lib/protocol.ex:501: anonymous fn/4 in Protocol.extract_matching_by_attribute/3
             (elixir 1.18.0-dev) lib/enum.ex:2546: Enum."-reduce/3-lists^foldl/2-0-"/3
             (elixir 1.18.0-dev) lib/protocol.ex:500: Protocol.extract_matching_by_attribute/3
             (mix 1.18.0-dev) lib/mix/tasks/compile.protocols.ex:140: Mix.Tasks.Compile.Protocols.consolidate/4
             (elixir 1.18.0-dev) lib/task/supervised.ex:101: Task.Supervised.invoke_mfa/2
             (elixir 1.18.0-dev) lib/task/supervised.ex:36: Task.Supervised.reply/4

@josevalim
Copy link
Member

That was a race, it should be fixed here: #13856

@PragTob
Copy link
Contributor Author

PragTob commented Sep 24, 2024

Neat!

whatyouhide
whatyouhide approved these changes Sep 24, 2024
View reviewed changes
lib/logger/lib/logger.ex
* Alternates between sync and async modes to remain
performant when required but also apply back-pressure
when under stress.
* Provides multiple forms of [overload protection](https://www.erlang.org/doc/apps/kernel/logger_chapter.html#protecting-the-handler-from-overload):
Copy link
Member

Choose a reason for hiding this comment

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

It's fine, no worries!

lib/logger/lib/logger.ex Outdated Show resolved Hide resolved
@whatyouhide whatyouhide merged commit 6cd46d7 into elixir-lang:main Sep 24, 2024
9 checks passed
@whatyouhide
Copy link
Member

Thanks @PragTob! 💟

@PragTob PragTob deleted the improve-logger-docs branch September 24, 2024 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josevalim josevalim josevalim approved these changes

@whatyouhide whatyouhide whatyouhide approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PragTob @josevalim @whatyouhide