rrrene / credo

A static code analysis tool for the Elixir language with a focus on code consistency and teaching.
http://credo-ci.org/
MIT License
4.94k stars 419 forks source link

Credo.Check.Warning.MissedMetadataKeyInLoggerConfig reporting false errors since 1.7.2 upgrade #1101

Closed Billzabob closed 10 months ago

Billzabob commented 11 months ago

Precheck

Environment

What were you trying to do?

Run mix credo --strict

Expected outcome

No Credo errors

Actual outcome

Since going from 1.7.1 to 1.7.2 we are now getting a ton of errors from Credo.Check.Warning.MissedMetadataKeyInLoggerConfig that look like this:

┃ [W] ↗ Logger metadata key aid not found in Logger config
┃       lib/libs/quiltt.ex:85 #(Libs.Quiltt.upsert_account)

We have all these specified in our config/config.exs like so:

config :logger, :console,
  format: "$time $metadata[$level] $message\n",
  metadata: [
    :account,
    :admin,
    :aid,
    ...
  ]
tcitworld commented 11 months ago

Same issue here.

:logger |> Application.get_env(:default_formatter) |> Keyword.get(:metadata) returns the correct metadata list in the same environment.

rrrene commented 11 months ago

@Billzabob Ah, we changed the key that is read to :default_formatter for newer versions of Elixir. Does it work when you change your logger config to the new key?

@tcitworld When you are writing "in that environment", is that the same env Credo is run in?

ViseLuca commented 11 months ago

@rrrene yeah, I tried on my project to add the metadata in every environment file but nothing is changing. Same error of @Billzabob

rhammer1 commented 11 months ago

Changing :console to :default_formatter worked for me on a clean phoenix install

ie. `$ mix credo -v 1.7.2

$ elixir -v Erlang/OTP 26 [erts-14.2.1] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [jit:ns]

Elixir 1.16.0 (compiled with Erlang/OTP 24)

$ uname -a Linux russ-76 6.6.8-200.fc39.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Dec 21 04:01:49 UTC 2023 x86_64 GNU/Linux

$ mkdir /tmp/temp-phoenix $ cd /tmp/temp-phoenix $ mix archive.install hex phx_new $ mix phx.new hello

finish install

$ sed -i -e '/:dns_cluster/a \ {:credo, "~> 1.7.2", only: [:dev, :test], runtime: false},' mix.exs $ mix deps.get

finish install

$ mix credo

all good

$ echo 'defmodule Hello.Foo do @moduledoc false

require Logger

defp foo do Logger.metadata(request_id: request_id) end end' >lib/hello/foo.ex

$ mix credo Checking 22 source files ...

Warnings - please take a look ┃ ┃ [W] ↗ Logger metadata key request_id not found in Logger config ┃ lib/hello/foo.ex:7 #(Hello.Foo.foo)

Please report incorrect results: https://github.com/rrrene/credo/issues

Analysis took 0.2 seconds (0.04s to load, 0.2s running 55 checks on 22 files) 66 mods/funs, found 1 warning.

Showing priority issues: ↑ ↗ → (use mix credo explain to explain issues, mix credo --help for options).

$ sed -i -e 's/:console/:default_formatter/' config/{config,dev}.exs

$ mix credo

all good

`

Question? is this the correct way to clear the 'issue'? I don't know the impact to the rest of the application when changing :console to :default_formatter

rrrene commented 10 months ago

@Billzabob @tcitworld @ViseLuca Thanks for reporting this 😀 It 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! 👍

@rhammer1 No, we don't want people to change their config.exs for Credo to work. We f'ed up on this one 🤔

ViseLuca commented 10 months ago
{:credo, github: "rrrene/credo"}

Yeah issue is solved! I tried right now, thank you very much! 😄

rrrene commented 10 months ago

@Billzabob @tcitworld @ViseLuca @rhammer1 Fix is live as part of 1.7.3. Sorry for the inconvenience. ✌️

Please feel free to re-open if there are any problems :+1:

rhammer1 commented 10 months ago

Credo 1.7.3 fixes the problem, thanks!