Configure default sensitive fields #24

mahdiridho · 2025-03-27T11:55:44Z

Through the discussion issue below:

We are able to redefining the default sensitive fields when Logger initialization

bboure · 2025-03-27T11:57:08Z

src/index.ts


-    constructor(serviceName: string, applicationName: string, correlationId: string | null = null) {
+    constructor(serviceName: string, applicationName: string, options: LoggerOptions = {}) {


⚠️ This is a breaking change.
We need to decide what to do with it. I see 3 options:

We accept the breaking change. As this project is quite new and not widely used, we go and fix the issue everywhere we are using it (which is not a lot of places)
We do a MAJOR version bump
We do backward compatible change. (i.e. if options is a string, we consider it's correlationId).
I would personally probably go with 3 since it's not a big change and it avoids breaking anything. Maybe 1 if we are sure we are only using it internally.
It's still early for numbing to v2 already 😄

Yes, I was thinking multiple times to move in the correlationId arg to new options. We define it with setCorrelationId() instead in our projects. I agree since it's still very early, we can simplify it from now.

It supports both old/new constructor style now

bboure · 2025-03-27T11:57:36Z

src/index.ts

+        this.defaultSensitiveAttributes = [...Logger.DEFAULT_SENSITIVE_ATTRIBUTES];
+
+        // Handle custom sensitive attributes
+        if (options.overrideSensitiveAttributes) {
+            this.defaultSensitiveAttributes = options.overrideSensitiveAttributes;
+        } else if (options.additionalSensitiveAttributes) {
+            this.defaultSensitiveAttributes = [...this.defaultSensitiveAttributes, ...options.additionalSensitiveAttributes];
+        }


Nit-pick

Suggested change

this.defaultSensitiveAttributes = [...Logger.DEFAULT_SENSITIVE_ATTRIBUTES];

// Handle custom sensitive attributes

if (options.overrideSensitiveAttributes) {

this.defaultSensitiveAttributes = options.overrideSensitiveAttributes;

} else if (options.additionalSensitiveAttributes) {

this.defaultSensitiveAttributes = [...this.defaultSensitiveAttributes, ...options.additionalSensitiveAttributes];

}

// Handle custom sensitive attributes

if (options.overrideSensitiveAttributes) {

this.defaultSensitiveAttributes = options.overrideSensitiveAttributes;

} else {

this.defaultSensitiveAttributes = [

...Logger.DEFAULT_SENSITIVE_ATTRIBUTES,

...(options.additionalSensitiveAttributes || [])

];

}

bboure · 2025-03-27T11:58:08Z

src/index.ts

@@ -99,7 +114,7 @@ class Logger {
            };

            // Merge default sensitive attributes with custom ones
-            const attributesToMask = new Set([...defaultSensitiveAttributes, ...arrayToLowerCase(sensitiveAttributes)]);
+            const attributesToMask = new Set([...this.defaultSensitiveAttributes, ...arrayToLowerCase(sensitiveAttributes)]);


as we discussed offline, it would be great if Logger.log was also able to override.
e.g.

// Let's log all the things 🪄 logger.log('info', 'My data', { secret: 'superseretthing', password: 'm1Pa$$w0rd' }, { overrideSensitiveAttributes: [] });

// Print user but hide sensible things logger.log('info', 'My datauser', user, { additionalSensitiveAttributes: ['firstName', 'lastname', 'email'] });

it supports both style old and new plan now

bboure · 2025-03-27T11:59:17Z

src/index.ts

+        this.defaultSensitiveAttributes = [...Logger.DEFAULT_SENSITIVE_ATTRIBUTES];
+        // Clear any custom sensitive attributes that were added through log methods
+        this.log("info", "Sensitive attributes have been reset to defaults", {}, {}, []);
+    }


Is this this.log necessary? I would not log things inside the logger, lol.

Also, let's discuss what "reset" this should really do.
Should it reset to the "default" list (Logger.DEFAULT_SENSITIVE_ATTRIBUTES)
Or should it set it back to what we passed in the constructor??

I think reset only makes sense if you can "set"/change it outside of the constructor.
Because once you reset it you can't change it back to something else.

this.log is to reset "sensitiveAttributes". This reset method to clear out both default and sensitiveAttributes.

Yes, we can add "set" along with reset if we agree with this feature. It's just my bet to see if it's useful or not?

What I was asking in my first question i why do we need this line

this.log("info", "Sensitive attributes have been reset to defaults", {}, {}, []);

Regarding the second point. Wither we have a setSesitiveAttributes, or we remove this resetSensitiveAttributes. I think the second does not make sense without the first.
For now, I would simply remove it. I don't see it useful at all

bboure · 2025-03-27T13:11:03Z

src/index.ts

+        this.defaultSensitiveAttributes = [...Logger.DEFAULT_SENSITIVE_ATTRIBUTES];
+        // Clear any custom sensitive attributes that were added through log methods
+        this.log("info", "Sensitive attributes have been reset to defaults", {}, {}, []);
+    }


What I was asking in my first question i why do we need this line

this.log("info", "Sensitive attributes have been reset to defaults", {}, {}, []);

Regarding the second point. Wither we have a setSesitiveAttributes, or we remove this resetSensitiveAttributes. I think the second does not make sense without the first.
For now, I would simply remove it. I don't see it useful at all

bboure · 2025-03-27T14:19:12Z

src/index.ts

@@ -277,7 +312,7 @@ class Logger {
        message: string = "",
        payload: JSONValue | Error = {},
        context: JSONObject = {},
-        sensitiveAttributes: StringArray = []
+        sensitiveAttributes: StringArray | LoggerOptions = []


I would not re-use the constructor's LoggerOptions here.
The constructor might have some options that we don't want here.
Please use a separate type here

Separate it with inheritance interface

src/index.ts

bboure · 2025-03-27T16:26:53Z

src/index.ts

+                inputOptions = { additionalSensitiveAttributes: options };
+            } else {
+                inputOptions = options;
+            }


Please move this up all the way to the top of the method

bboure · 2025-04-07T10:51:19Z

README.md

 ```

 - **serviceName** [string, mandatory]: Added to each log output
 - **applicationName** [string, mandatory]: Defines the Namespace for metrics
- **correlationId** [string, optional]: A new UUIDv4 is generated when not defined. Added to each log output.
+- **options** [object | string | null, optional]: Configuration options


Don't mention string here, or mention that it is deprecated.
We keep it only for backward compatibility purpose.

bboure · 2025-04-07T10:52:03Z

README.md

+- **options** [object | string | null, optional]: Configuration options
+  - **correlationId** [string, optional]: A new UUIDv4 is generated when not defined. Added to each log output.
+  - **additionalSensitiveAttributes** [string[], optional]: Add new sensitive attributes to the pre-defined defaults
+  - **overrideSensitiveAttributes** [string[], optional]: Completely override the default sensitive attributes


These 3 are in option. We should make that clear.

Configure initial sensitive fields

8f53ded

bboure reviewed Mar 27, 2025

View reviewed changes

Mahdi Ridho added 2 commits March 27, 2025 19:32

Fixed to define default sensitive attr

03fb3f5

Constructor and sensitiveAttributes compatible with both old/new style

ca12abc

bboure reviewed Mar 27, 2025

View reviewed changes

Mahdi Ridho added 7 commits March 27, 2025 21:34

resetSensitiveAttributes no needed

17bec45

Separate interfaces for options and sensitive attr

6f05e19

Normalizes the log options

b2498ae

Normalize constructor options

5d55bac

Update doc for new sensitive fields features

09efea5

lower case the updated defaultSensitiveAttributes

2a44e21

Update unit test

c402ca2

bboure reviewed Apr 7, 2025

View reviewed changes

Minor fix review

c9beda6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Configure default sensitive fields #24

Configure default sensitive fields #24

mahdiridho commented Mar 27, 2025

bboure Mar 27, 2025

mahdiridho Mar 27, 2025

mahdiridho Mar 27, 2025

bboure Mar 27, 2025

mahdiridho Mar 27, 2025

bboure Mar 27, 2025

mahdiridho Mar 27, 2025

bboure Mar 27, 2025

mahdiridho Mar 27, 2025

bboure Mar 27, 2025

mahdiridho Mar 27, 2025

bboure Mar 27, 2025

bboure Mar 27, 2025

mahdiridho Mar 27, 2025

bboure Mar 27, 2025

bboure Apr 7, 2025

bboure Apr 7, 2025


		constructor(serviceName: string, applicationName: string, correlationId: string \| null = null) {
		constructor(serviceName: string, applicationName: string, options: LoggerOptions = {}) {

Configure default sensitive fields #24

Are you sure you want to change the base?

Configure default sensitive fields #24

Conversation

mahdiridho commented Mar 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment