otelzap: Skeleton for zap encoder

[khushijain21]

Part of #5191

Pre-work #5279

Why we might need a custom encoder -
zap code forces you to implement your own object encoder to intercept field types. We cannot use their default console and json encoder - as they write to the provided io.Writer in JSON format.

MemoryEncoder from zap would have been a suitable solution for our use case - but it is not recommended to be used in production.

A custom encoder will also ensure support for all compatible types

[codecov]

Codecov Report

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

Project coverage is 63.0%. Comparing base (c54129c) to head (967d5c5).

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #5566     +/-   ##
=======================================
- Coverage   63.2%   63.0%   -0.2%     
=======================================
  Files        193     194      +1     
  Lines      11917   11949     +32     
=======================================
  Hits        7534    7534             
- Misses      4166    4198     +32     
  Partials     217     217             

Files	Coverage Δ
bridges/otelzap/encoder.go	0.0% <0.0%> (ø)

[pellared]

From https://pkg.go.dev/go.uber.org/zap#hdr-Extending_Zap:

requires implementing the zapcore.Encoder, zapcore.WriteSyncer, or zapcore.Core interfaces

Can we have proof we need to implement the encoder? AFAIK we just need to implement zapcore.Core. Zap encoder implementation would be needed only if we would like the users to create the zapcore.Core using https://pkg.go.dev/go.uber.org/zap/zapcore#NewCore. What are the benefits of supporting such use-case?

EDIT: AFAIK implementing zap encoders makes sense if the destination is io.Writer-like (basically steams logs to https://pkg.go.dev/go.uber.org/zap/zapcore#WriteSyncer).

PS. I already mentioned it as the first bullet in #5191 (comment)

[khushijain21]

opentelemetry-go-contrib/bridges/zap/zap_logger.go

Line 103 in ba244b3

func convertValue(f zapcore.Field) log.Value {

I did use switch statements at the beginning and this is the problem I ran into -

When a user chooses to implement MarshalLogObject/MarshalLogArray - we need an encoder to unmarshal its contents since MarshalLogObject uses methods defined on the encoder interface - in this case enc.AddString and enc.AddInt. A switch statement cannot recursively unmarshall our data as we see in other bridges.

// Person is a custom type that holds information about a person.
type Person struct {
    Name string
    Age  int
}

// MarshalLogObject implements the zapcore.ObjectMarshaler interface.
func (p Person) MarshalLogObject(enc zapcore.ObjectEncoder) error {
    enc.AddString("name", p.Name)
    enc.AddInt("age", p.Age)
    return nil
}

func main() {
    // Create a zap logger.
    logger, _ := zap.NewProduction()
    defer logger.Sync()

    // Create an instance of Person.
    person := Person{
        Name: "Jane Doe",
        Age:  25,
    }

    // Log the Person struct using the Object field.
    logger.Info("Logging custom object", zap.Object("person", person))
}

The only other way to do this w/o an enoder would be to read from the destination that zap writes to and convert the data to OTel values (cannot vouch if this may be a good solution tho)

[pellared]

When a user chooses to implement MarshalLogObject/MarshalLogArray

Now, I get it. Thanks for the explanation 🙏 I missed that these use the encoder API.

To sum up, this is needed for handling ArrayMarshalerType and ObjectMarshalerType, right?

EDIT: I pushed 1d91526 to your other PR.

Encoders are indeed needed.

[khushijain21]

To sum up, this is needed for handling ArrayMarshalerType and ObjectMarshalerType, right?

yes correct

[pellared]

golangci-lint ./bridges/otelzap
Error: encoder.go:19:2: field `kv` is unused (unused)
	kv []log.KeyValue
	^
Error: encoder.go:22:6: func `newObjectEncoder` is unused (unused)
func newObjectEncoder(len int) *objectEncoder {
     ^

You can force the linter to ignore the error by adding a comment looking like //nolint:unused TODO.