newrelic / go-agent

New Relic Go Agent
Apache License 2.0
756 stars 296 forks source link

Attributes are not forwarded to NewRelic for logs #883

Open mariusgrigaitis opened 3 months ago

mariusgrigaitis commented 3 months ago

Logs in context integrations and newrelic-agent does not forward attributes from logs

https://docs.newrelic.com/docs/logs/logs-context/configure-logs-context-go/#installing-a-logs-in-context-plugin

https://github.com/newrelic/go-agent/blob/master/v3/integrations/logcontext-v2/nrslog/handler.go#L130

Expected Behavior

Attributes appear on NR.

Reproduction case

https://github.com/newrelic/go-agent/blob/master/v3/integrations/logcontext-v2/nrslog/example/main.go

Additional attributes can be added to prove the case in the linked example

Additional context

What's the point of integrating structured logging solutions into NewRelic if attributes (essential part of structured) are not being forwarded?

Seems to be the case for all logcontext-v2 integrations, since attributes are not part of logcontext-v2 design?

Related: #768

r--w commented 3 months ago

This functionality is quite critical. Good slog practice requires adding attributes with additional metadatada, right now all is lost, only message is displayed in NewRelic's console.

r--w commented 3 months ago

@mariusgrigaitis I managed to add a simple middleware to add all the attributes to the message and this way you can at least see meaningful logs in NewRelic's console, here's the part of the code using https://github.com/samber/slog-multi/

....
logger := slog.New(
        slogmulti.
            Pipe(slogmulti.NewHandleInlineMiddleware(concatAttrsMiddleware)).
            Handler(nrslog.JSONHandler(app, os.Stdout, opts)),
    )

....
func concatAttrsMiddleware(ctx context.Context, record slog.Record, next func(context.Context, slog.Record) error) error {
    flat := strings.Builder{}

    record.Attrs(func(attr slog.Attr) bool {
        flat.WriteString("key: " + attr.Key + " value: " + fmt.Sprintf("%v", attr.Value))
        return true
    })

    message := record.Message
    if flat.Len() > 0 {
        message += ", " + flat.String()
    }

    return next(ctx, slog.NewRecord(record.Time, record.Level, message, record.PC))
}
iamemilio commented 3 months ago

Hi all, thank you for your advocacy and your patience. I hear your concerns and have gotten the go ahead to prioritize this ASAP. Supporting attribute collection for all v2 logs in context packages is my top priority. I will communicate expectations and upcoming changes as they come. I will communicate about this primarily in https://github.com/newrelic/go-agent/issues/768 since that is where most people seem to be tracking this issue.

clarkmcc commented 2 months ago

Thank you! I fought with this for hours before I realized it was expected behavior. Looking forward to a fix.

miyamo2 commented 2 months ago

I am also developing on a library as a hobby to include attributes in the Logs in Context while also logging to stdout in JSON format.

I hope you can use this as a temporary solution.

https://github.com/miyamo2/altnrslog

iamemilio commented 2 months ago

Capturing attributes should already work for decorated logs captured by log forwarders, but the agent is not yet able to handle and emit log attributes. We will have that code settled and shipped soon, and then we can start plugging it into our logging tools: https://github.com/newrelic/go-agent/pull/900

iamemilio commented 2 months ago

https://github.com/newrelic/go-agent/issues/768#issuecomment-2065146652

iamemilio commented 2 months ago

https://github.com/newrelic/go-agent/releases/tag/v3.33.0 - Adds this feature to Zap. Upcoming releases will add this to zerolog and slog too

mirackara commented 2 months ago

911 PR for slog attribute support.

dselans commented 1 month ago

Same as @clarkmcc , spent hours troubleshooting this until I found this issue.

Using logrus with .WithField() and .WithFields().

conamu commented 1 month ago

911 PR for slog attribute support.

I saw it was approved a month ago. Any updates on when this is going to get released?

dselans commented 1 month ago

@conamu They added support for attributes for uber/zap in 3.33.0 - it works well but it doesn't include initial fields or fields added via WithField() :(

As a tmp bandaid, I added my own log wrapper that keeps track of the fields and just re-adds them every time a log.Debug/Info/Warn/etc is called (with or without extra attributes). Could do the same for logrus/whatever other logger you're using.

Gist: https://gist.github.com/dselans/b318d89158ad83b930221650db15c6ec

conamu commented 1 month ago

we are already using logrus in most of the codebase. Im more interested in a solution to work with slog, a native package, instead of having to fix another library.

iamemilio commented 4 weeks ago

The Slog package is out :) https://github.com/newrelic/go-agent/tree/master/v3/integrations/logcontext-v2/nrslog

iamemilio commented 4 weeks ago

Working on a fix for #915

iamemilio commented 3 weeks ago

See: https://github.com/newrelic/go-agent/pull/919