Support arbitrary value type in fctx metadata

[nghialm269]

Hi, thanks for the library.

While testing the library, I see that fctx only supports string values. If I want to add a struct to the metadata, I'll have to either stringify it (but the log will be hard to read when logged using a structured logger) or add each field manually (which is painful if the struct has a lot of fields).

So I want to ask if there is any reason why fctx only supports string values, and if it is possible to make it support arbitrary-type (interface{}) values instead.

[Southclaws]

Hey, thanks for opening an issue! It's good to see some feedback about the design here as I went back and forth about this for a while with user testing and eventually settled on strings.

The reason for this is that string values are just easier to serialise and play nicer with the kinds of tooling this library was built to integrate with (structured logging, tracing and context-rich HTTP responses for errors)

The issue with complex structures and nested data is it's difficult to represent and index. In production, this library is often wired up to Graylog, Datadog, Kibana etc. as well as Jaeger tracing and other otel tracing tools. They usually prefer flattened structures for indexing and rendering (though this isn't a requirement on every system.)

There's definitely a question to be made around where this responsibility should live, as it can often be annoying to turn the data you have into strings at the call site of WithMeta or just allow the values to bubble up and deal with them in whatever way seems fit when you actually handle the error (which, in a web server, is usually an error handling middleware which logs out the error and responds with some information) I've often written code like this:

fctx.WithMeta(ctx, "key", fmt.Sprint(value))

I think it would make sense to change this to interface{} though at some point and deal with the complexity at the other end. Which would allow you to more easily pass whatever data in and flatten it (or not) as you want.

However, one benefit of enforcing strings only at the call site is that your error handling code is much simpler, there doesn't need to be any reflection or encoding/serialisation of these values which may interrupt a very important area of the application: error reporting.

[nghialm269]

The issue with complex structures and nested data is it's difficult to represent and index. In production, this library is often wired up to Graylog, Datadog, Kibana etc. as well as Jaeger tracing and other otel tracing tools. They usually prefer flattened structures for indexing and rendering (though this isn't a requirement on every system.)

one benefit of enforcing strings only at the call site is that your error handling code is much simpler, there doesn't need to be any reflection or encoding/serialisation of these values which may interrupt a very important area of the application: error reporting.

Thanks for the insights. Since I've always worked in teams with DevOps, they usually take care of these things, I was able to filter the logs no matter what structures of the data I logged (one exception was when I wanted to filter a number field using greater than or less than operators), so I didn't think of indexing at all when asking.

I think it would make sense to change this to interface{} though at some point and deal with the complexity at the other end. Which would allow you to more easily pass whatever data in and flatten it (or not) as you want.

Another way is to add a new function like WithData that accepts interface{}, and its corresponding GetData function to get the data. WithMeta will remain there for indexing (and related) purposes, and WithData can be used in case users just want to quickly dump the struct without worrying about indexing. But perhaps this will make error wrapping too complicated.

[Southclaws]

WithData could be a nice non-breaking approach, I like it!

I would likely stick with a design like https://pkg.go.dev/log/slog#Debug where it's just ...any though I'm curious to hear if anyone has opinions about a more structured/safer approach:

fctx.WithData(
  fctx.String("key1", "value"),
  fctx.Int("key2", 123),
  fctx.Any("key3", arbitraryValue),
)

[nghialm269]

WithData could be a nice non-breaking approach, I like it!

I would likely stick with a design like pkg.go.dev/log/slog#Debug where it's just ...any though I'm curious to hear if anyone has opinions about a more structured/safer approach:

fctx.WithData(
  fctx.String("key1", "value"),
  fctx.Int("key2", 123),
  fctx.Any("key3", arbitraryValue),
)

I always prefer to have type-safe, but since this is for debugging/logging purposes, I think ...any should be good enough.

[Southclaws]

I hear you, type safety is usually my top priority, but ergonomics of the API is also important - if it's awkward to use, fewer people will be inclined to use it. I find this with Zap, it's a great logger but typing zap.String several times can be annoying!