Add: the ability to redact values from Stringer and Error interface.

[moisesvega]

Currently, there's no feature to exclude sensitive information from outputs generated by the Stringer and Error interface implementations. However, we use the nolog annotation to exclude specific properties from being logged by the zap.objectMarshal method. This is particularly useful for omitting sensitive data in zap logger outputs. Despite this, there are scenarios where sensitive information might still be exposed. For instance, consider the following example:

exception DoesNotExistException {
   /** Key that was missing. */
   1: required string key
   2: optional string Error (go.name="Error2")
   3: optional string userName (go.nolog)
}

The code snippet above leads to the following implementation for the MarshalLogObject method:

// MarshalLogObject allows DoesNotExistException to be logged quickly by zap.
func (v *DoesNotExistException) MarshalLogObject(enc zapcore.ObjectEncoder) error {
   if v == nil {
      return nil
   }
   enc.AddString("key", v.Key)
   if v.Error2 != nil {
      enc.AddString("Error", *v.Error2)
   }
   return nil
}

However, our implementation of the Stringer and Error interfaces inadvertently exposes properties marked with nolog:

func (v *DoesNotExistException) Error() string {
   return v.String()
}

// String provides a human-readable representation of DoesNotExistException.
func (v *DoesNotExistException) String() string {
   if v == nil {
      return "<nil>"
   }

   var fields [3]string
   i := 0
   fields[i] = fmt.Sprintf("Key: %v", v.Key); i++
   if v.Error2 != nil {
      fields[i] = fmt.Sprintf("Error2: %v", *v.Error2); i++
   }
   if v.UserName != nil {
      fields[i] = fmt.Sprintf("UserName: %v", *v.UserName); i++
   }

   return fmt.Sprintf("DoesNotExistException{%s}", strings.Join(fields[:i], ", "))
}

This behavior can lead to unintended exposure of information annotated as nolog. For example, when this exception is utilized as an Error, or when logged using zap.Stringer("exception", e) or zap.Error(e) , the nolog-annotated property is inadvertently revealed:

playground link

func main() {
   name := "Jane Doe"
   e := &DoesNotExistException{
      Key:      "",
      Error2:   nil,
      UserName: &name,
   }
   logger, _ := zap.NewProduction()
   defer logger.Sync() // Ensures buffer is flushed
   logger.Info("hello",
      zap.Object("exception", e),
      zap.Error(e),
   )
     // output: {"level":"info","msg":"hello","exception":{"key":""},"error":"DoesNotExistException{Key: , UserName: Jane Doe"}
}

This PR introduces a new feature that enables the redaction of specified properties through a new Go annotation, go.redacted. When applied, this annotation ensures that the actual value of a property is replaced with in the outputs generated by our Stringer and Error interface implementations.

Output after this diff:

  // output: {"level":"info","msg":"hello","exception":{"key":""},"error":"DoesNotExistException{Key: , UserName: <redacted>}"}

[codecov]

Codecov Report

Attention: Patch coverage is 76.27119% with 28 lines in your changes are missing coverage. Please review.

Project coverage is 69.03%. Comparing base (12ffd74) to head (6296da8).

Files	Patch %	Lines
gen/internal/tests/exceptions/exceptions.go	64.10%	7 Missing and 7 partials ⚠️
gen/internal/tests/structs/structs.go	64.10%	7 Missing and 7 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #592      +/-   ##
==========================================
+ Coverage   69.01%   69.03%   +0.01%     
==========================================
  Files         140      140              
  Lines       23523    23625     +102     
==========================================
+ Hits        16235    16309      +74     
- Misses       4228     4242      +14     
- Partials     3060     3074      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[moisesvega]

on behalf of @r-hang

[r-hang]

Looks good on behalf of @tchung1118.

We can add the redacted feature designation to zap logs instead if skipping them outright.

[r-hang]

Is this a new test? Should we also be adding a the case here for a redacted field since we're adding this new feature here?

[moisesvega]

Yes, it is. I noticed that no tests have been conducted for ZapOut. Implementing a small test could help increase our code coverage. I updated the test and add a test case.