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

MissedMetadataKeyInLoggerConfig is incorrect #1070

Closed
Coffei opened this issue Sep 1, 2023 · 5 comments
Closed

MissedMetadataKeyInLoggerConfig is incorrect #1070

Coffei opened this issue Sep 1, 2023 · 5 comments

Comments

@Coffei
Copy link

Coffei commented Sep 1, 2023

Environment

  • Credo version (mix credo -v): 1.7.0-ref.initial.292d461+uncommittedchanges
  • Erlang/Elixir version (elixir -v): Elixir 1.15.4, Erlang 26.0.2
  • Operating system: Fedora 38

Using default credo config, just installed as a dependency.

The issue

I have this in my code

    Logger.metadata(job: job.id)

which produces this credo warning

┃       Logger metadata key job will be ignored in production

I have this in my config/config.exs

config :logger, :default_formatter,
  format: "$time [$level] $metadata $message\n",
  metadata: [:job]

and my config/prod.exs is empty.

So I'd say the metadata should work fine in prod too. I tried running the app with MIX_ENV=prod and surely I can see the metadata in the log.

Expected outcome

Credo shouldn't generate a warning in this case.

@rrrene
Copy link
Owner

rrrene commented Sep 3, 2023

Credo uses the current console logger config for its defaults: https://hexdocs.pm/credo/Credo.Check.Warning.MissedMetadataKeyInLoggerConfig.html#module-metadata_keys

If the defaults don't suffice, you unfortunately need to configure them (by hand or by script).

@Coffei
Copy link
Author

Coffei commented Sep 4, 2023

Hi, the logger configuration changed a bit in Elixir 15. You no longer specify a console logger but rather use :default_formatter as in my example above, see https://hexdocs.pm/logger/Logger.html#module-configuration. The old configuration is still supported as noted here https://hexdocs.pm/logger/Logger.html#module-backends-and-backwards-compatibility. My config is just whatever the docs say I should use, I just added the metadata.

I think this check should support the new config too, since it's the default in Elixir 15 and I suppose new projects will use the new configuration more and more often.

@elishagreenwald
Copy link

+1 to this. I encountered the same issue

@rrrene
Copy link
Owner

rrrene commented Dec 13, 2023

@Coffei @elishagreenwald Thanks for reporting this 😀 This should now be fixed on master.

You can try this by setting the Credo dep to

{:credo, github: "rrrene/credo"}

Please report back if your issue is solved! 👍

@Coffei
Copy link
Author

Coffei commented Dec 15, 2023

Yes, this is fixed on master, thanks!

@Coffei Coffei closed this as completed Dec 15, 2023
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 a pull request may close this issue.

3 participants