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.

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

Propagate parent process logger level to preloader #4406

Closed
wants to merge 3 commits into from

Conversation

v0idpwn
Copy link
Member

@v0idpwn v0idpwn commented Apr 23, 2024

@v0idpwn
Copy link
Member Author

v0idpwn commented Apr 23, 2024

Problem: Logger.get_process_level and Logger.put_process_level functions were added on Elixir 1.15. What should we do here? A few options:

  • checking elixir version in runtime, adding a small overhead
  • wait until we can require elixir >= 1.15
  • write a different implementation for elixir >= 1.15

Edit: I decided for the last option, but am open to change if you disagree

@josevalim
Copy link
Member

What if we fix the metadata initially and we address the level once we require Elixir v1.15?

@v0idpwn
Copy link
Member Author

v0idpwn commented Apr 24, 2024

I like the idea of propagating logger metadata too, certainly can add that!
With regards to the level, I assume we want to support Elixir <1.15 for at least one year more (1.15.0 is not even one year old yet), and delaying this fix until then doesn't seem to be worth it. The two variants of pmap seem relatively low cost to me. WDYT?

@greg-rychlewski
Copy link
Member

is everything inside of Logger.get/put_process_level also dependent on Elixir 1.15 or is there anything we can inline?

@v0idpwn
Copy link
Member Author

v0idpwn commented Apr 24, 2024

It's very Elixir-version dependent, I'd say.

Before 1.15 there was Logger.enable/1, Logger.disable/1 and Logger.enabled?/1. We could use that, but it's a more limited feature (and is now deprecated in favor of the new APIs).

@josevalim
Copy link
Member

On the flip side, we did not have this feature for years, we can wait one or two more before adding it. :)

@v0idpwn
Copy link
Member Author

v0idpwn commented Apr 26, 2024

That's fair enough, @josevalim. I'll close this one.

@v0idpwn v0idpwn closed this Apr 26, 2024
@v0idpwn v0idpwn deleted the propagate-log branch April 26, 2024 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logger.put_module_level does not silence preloads
3 participants