Refactor log level configuration #7695

jiv-e · 2020-09-02T09:32:21Z

Remove STRAPI_LOG_LEVEL environment variable related redundant code.
Add helpful error message to the current log level configuration
system.

- Remove STRAPI_LOG_LEVEL environment variable related redundant code. - Add helpful error message to the current log level configuration system. Signed-off-by: Juho Viitasalo <juho.viitasalo@lildrop.com>

Signed-off-by: Juho Viitasalo <juho.viitasalo@lildrop.com>

codecov · 2020-09-02T09:53:12Z

Codecov Report

Merging #7695 into master will increase coverage by 5.78%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #7695      +/-   ##
==========================================
+ Coverage   27.23%   33.01%   +5.78%     
==========================================
  Files        1163     1219      +56     
  Lines       15532    13571    -1961     
  Branches     2411     1349    -1062     
==========================================
+ Hits         4230     4481     +251     
+ Misses       9534     8207    -1327     
+ Partials     1768      883     -885

Flag	Coverage Δ
#front	`24.70% <ø> (+5.28%)`	⬆️
#unit	`54.05% <ø> (+0.20%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...admin/src/containers/LanguageProvider/selectors.js	`50.00% <0.00%> (-21.43%)`	⬇️
...rapi-admin/admin/src/hooks/useFetchRole/reducer.js	`80.00% <0.00%> (-20.00%)`	⬇️
...min/admin/src/components/Roles/Permissions/init.js	`80.00% <0.00%> (-20.00%)`	⬇️
...n/src/components/Users/SelectRoles/utils/styles.js	`71.42% <0.00%> (-17.47%)`	⬇️
...-admin/admin/src/containers/HomePage/components.js	`40.00% <0.00%> (-17.15%)`	⬇️
...ents/Webhooks/HeadersInput/utils/getBorderColor.js	`50.00% <0.00%> (-16.67%)`	⬇️
packages/strapi-admin/services/permission.js	`77.57% <0.00%> (-14.00%)`	⬇️
.../admin/src/components/Roles/Permissions/reducer.js	`85.41% <0.00%> (-13.64%)`	⬇️
...t-type-builder/controllers/validation/relations.js	`50.00% <0.00%> (-12.50%)`	⬇️
.../admin/src/containers/Webhooks/EditView/reducer.js	`82.35% <0.00%> (-11.59%)`	⬇️
... and 1124 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 005db79...fba9abb. Read the comment docs.

alexandrebodin · 2020-09-02T15:20:48Z

Hello, I don't understand what this PR adds ? it is actually a breaking change

jiv-e · 2020-09-02T15:29:59Z

Have you read the issue?

PR removes some redundant and confusing code that doesn't do anything anymore. It is not a breaking change.
What it adds is a validation for the log level value and a helpful error message in case the validation fails.

jiv-e · 2020-09-02T15:41:04Z

Is it possible this pull request gets accepted? I might need some help passing the code coverage check. I'm not sure what to do with it.

alexandrebodin · 2020-09-02T16:49:03Z

That's actually not that simple :).

The logger can be and is used outside of the middleware and might be required (e.g in command line) without the middleware being instantiated. in those usecases the env var is usefull.

We need to make sure the standalone logger offers the previous features it has without have to depend on its log level being instantiate in the middleware.

I Hope that makes sense.

jiv-e · 2020-09-03T06:27:38Z

That makes perfect sense. Do you still think it's a good idea to still add the validation part? Do we need to add tests for these features?

In any case when this is settled I'll try to do another pull request to add some docs.

alexandrebodin · 2020-09-03T07:36:36Z

Sure doing some validation is alright :) And tests are always welcome 👍

alexandrebodin · 2020-09-29T12:48:18Z

Hi @jiv-e will you be able to finish this PR ?

jiv-e · 2020-09-30T11:17:53Z

Hi @alexandrebodin,
Yes. I made two more commits. Is this ok?

alexandrebodin · 2020-09-30T13:45:56Z

packages/strapi/lib/middlewares/logger/index.js

@@ -25,14 +26,31 @@ module.exports = strapi => {
    initialize() {
      const { level, exposeInContext, requests } = strapi.config.middleware.settings.logger;

-      if (level) {
-        strapi.log.level = level;
+      const logLevels = ['fatal', 'error', 'warn', 'info', 'debug', 'trace'];


Can you use the levels available at strapi.log.levels to avoid having to maintain this list :)

const logLevels = Object.keys(strapi.log.levels.values);

packages/strapi/lib/middlewares/logger/index.js

alexandrebodin · 2020-10-07T12:47:42Z

Hey @jiv-e almost there :) WIll you be able to apply the last feedbacks to your PR ?

jiv-e · 2020-10-08T09:45:12Z

Done.. I also added validation for STRAPI_LOG_LEVEL.

alexandrebodin

LGTM 👍

jiv-e added 2 commits September 2, 2020 12:33

Refactor log level configuration

60d878a

- Remove STRAPI_LOG_LEVEL environment variable related redundant code. - Add helpful error message to the current log level configuration system. Signed-off-by: Juho Viitasalo <juho.viitasalo@lildrop.com>

Fix error message

524370a

Signed-off-by: Juho Viitasalo <juho.viitasalo@lildrop.com>

jiv-e force-pushed the issue/7683 branch from c2cf41b to 524370a Compare September 2, 2020 09:34

jiv-e changed the title ~~Issue/7683~~ Refactor log level configuration Sep 2, 2020

Fix log level base value

17e0148

alexandrebodin self-requested a review September 3, 2020 07:28

alexandrebodin added source: core:strapi Source is core/strapi package issue: enhancement Issue suggesting an enhancement to an existing feature labels Sep 4, 2020

jiv-e added 2 commits September 30, 2020 14:15

Fix breaking change

2260bb6

Add debug message

5cabab7

alexandrebodin reviewed Sep 30, 2020

View reviewed changes

packages/strapi/lib/middlewares/logger/index.js Show resolved Hide resolved

Change logger code to use Pino's log level values directly

a2d4d84

Update index.js

fba9abb

alexandrebodin approved these changes Oct 8, 2020

View reviewed changes

alexandrebodin merged commit 6fb0a63 into strapi:master Oct 8, 2020

alexandrebodin added this to the 3.2.2 milestone Oct 8, 2020

This was referenced Jun 6, 2021

Update dependency strapi to v3.2.5 [SECURITY] - autoclosed Tejaswgupta/furniture-shop-strapi#31

Closed

🤖 Update dependency strapi to version 3.2.5 - autoclosed gongbaodd/query.gongbushang.com#18

Closed

This was referenced Jun 6, 2021

🤖 Update dependency strapi-plugin-content-manager to version 3.2.5 - autoclosed gongbaodd/query.gongbushang.com#16

Closed

🤖 Update dependency strapi-plugin-content-type-builder to version 3.2.5 - autoclosed gongbaodd/query.gongbushang.com#17

Closed

renovate bot mentioned this pull request Jun 14, 2021

Update strapi monorepo to v3.6.10 Tejaswgupta/furniture-shop-strapi#21

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor log level configuration #7695

Refactor log level configuration #7695

jiv-e commented Sep 2, 2020 •

edited

codecov bot commented Sep 2, 2020 •

edited

alexandrebodin commented Sep 2, 2020

jiv-e commented Sep 2, 2020

jiv-e commented Sep 2, 2020

alexandrebodin commented Sep 2, 2020

jiv-e commented Sep 3, 2020

alexandrebodin commented Sep 3, 2020

alexandrebodin commented Sep 29, 2020

jiv-e commented Sep 30, 2020

alexandrebodin Sep 30, 2020 •

edited

jiv-e Sep 30, 2020

alexandrebodin commented Oct 7, 2020

jiv-e commented Oct 8, 2020

alexandrebodin left a comment

Refactor log level configuration #7695

Refactor log level configuration #7695

Conversation

jiv-e commented Sep 2, 2020 • edited

codecov bot commented Sep 2, 2020 • edited

Codecov Report

alexandrebodin commented Sep 2, 2020

jiv-e commented Sep 2, 2020

jiv-e commented Sep 2, 2020

alexandrebodin commented Sep 2, 2020

jiv-e commented Sep 3, 2020

alexandrebodin commented Sep 3, 2020

alexandrebodin commented Sep 29, 2020

jiv-e commented Sep 30, 2020

alexandrebodin Sep 30, 2020 • edited

Choose a reason for hiding this comment

jiv-e Sep 30, 2020

Choose a reason for hiding this comment

alexandrebodin commented Oct 7, 2020

jiv-e commented Oct 8, 2020

alexandrebodin left a comment

Choose a reason for hiding this comment

jiv-e commented Sep 2, 2020 •

edited

codecov bot commented Sep 2, 2020 •

edited

alexandrebodin Sep 30, 2020 •

edited