Report request and error

[jmfederico]

Rollbar knows how to report extra request data and extract information from errors if it is explicitly told to do so.

This PR takes inspiration from the official rollbar-go client and its Log function to do so.

Changes in this PR:

• Implement a default ReplaceAttr to remove *http.Request attributes (as rollbar breaks if they are passed to it)
• Extract *http.Request and error objects from slog.Record
• Dispatch the correct reporter method following the same logic as the official rollbar-go client

[samber]

if the developer uses the ReplaceAttr parameter, this function will be skipped

do you confirm there are no unintended outcomes?

[jmfederico]

If they are sending logging a "request" as part of the log message, and it is not removed here, the logging might break.

This is true for most logging handlers though, so if the developer is logging it, then they most probably are aware of it.

Maybe this can be made into a public function that developers can wrap in their own ReplaceAttr if needed?

func defaultReplaceAttr(groups []string, a slog.Attr) slog.Attr {
    return RemoveRequestAttr(groups []string, a slog.Attr)
}

func RemoveRequestAttr(groups []string, a slog.Attr) slog.Attr {
    // The existing code goes here.
}

[samber]

2 questions:

• is it only http.Request or any struct ?
• did you have this issue with nested attributes ?

[jmfederico]

1. *http.Request breaks the rollbar client. Some other structs do so as well.
2. Yes, even if *http.Request is nested within some other struct, it will break.

Each handler behaves differently, I have found that some logger are able to handle *http.Request and some do not, but this is the case for many structs and depends on the data that they carry and how Handlers marshal the data.

My thought process in this implementation was:

• Without this PR, if one passes a *http.Request as an attribute to slog, slog-rollbar will break
• which means that no one should be passing it right now, unless they have their own way of dealing with this
• But, it is good to have the request as part of the rollbar data, if available
• one way to handle it is to remove it from the attr list (which is the way I went for), and pass it to rollbar as a request (using their API, which is what I went for)
• But other alternatives are for instance to marshal it instead of removing it.

Now, back to your original question:

1. If a developer uses their own ReplaceAttr
2. And are passing a *http.Request as an attribute
3. And do not handle the *http.Request, either by removing or by making it marshable by Rollbar
4. Then their data will not make it into rollbar

But, I would not force removing it, as some other developer might want to Marshal it differently instead of removing it.

So my idea is:

• Make it safe by default (by removing it)
• But let developer decide what they want to do, and not force removing it.

[jmfederico]

I have renamed the default function to make its purpose clearer, and documented the default

[samber]

Ok, thanks for your feedback.

Can you open an issue to the rollbar go library? IMO, they should be able to receive any data.

As soon as they improve the package, we will do a new fix to the slog-rollbar to remove removeRequestAttr.

[samber]

Thanks for this contribution!

a few feedbacks and I'm ready to merge ;)