A Sprint function for services that use json logging

[dammitjim]

Hello,

In the services that we run we utilise json logging for all of our error logs, I had a peruse and it seems that the current print functions either require a writer or go straight to os.Stderr.

I think it would be very helpful for us, as well as for other users of this package, if there was a function that performs the same as fprint but returns a string for use.

I've created a proof of concept here: https://github.com/jimah/errors/blob/master/errors.go#L253

For example output please see the tests.

Thank you for your time!

[davecheney]

Can you use

var b bytes.Buffer
errors.Fprint(&b, err)

?

[jd3nn1s]

Similarly I'd like to be able to pass the print output to golang/glog in a single line of code like:

glog.Error(err.Sprint())

IMHO using multiple lines could mean a lot of boiler plate for something frequent like a log statement.

[davecheney]

If you want a different presentation I recommend copying the body of Fprint and altering to your specification, there is nothing in that function that requires knowledge of the types in the errors package.

On Sat, 7 May 2016, 17:34 jd3nn1s, notifications@github.com wrote:

Similarly I'd like to be able to pass the print output to golang/glog in a single line of code like:

glog.Error(err.Sprint())

IMHO using multiple lines could mean a lot of boiler plate for something frequent like a log statement.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/pkg/errors/issues/20#issuecomment-217613709

[dammitjim]

Hi guys,

var b bytes.Buffer errors.Fprint(&b, err)

Unless I'm mistaken this would involve processing the string after the fact to ensure that it is single line for JSON logging which is unideal for performance considering how many logs can be churned out.

Copying the body of Fprint and altering is essentially what this function is, however I believe this is not an edge case and is common enough to warrant being in the main package. If you disagree can we discuss the following alternatives?

• A function that returns the errors as a string array
• An additional parameter passed to fprint to specify the separator rather than \n

Thank you

[davecheney]

@jimah

Unless I'm mistaken this would involve processing the string after the fact to ensure that it is single line for JSON logging which is unideal for performance considering how many logs can be churned out.

If logging errors is such a bottleneck in your application, you may have other problems.

Copying the body of Fprint and altering is essentially what this function is, however I believe this is not an edge case and is common enough to warrant being in the main package. If you disagree can we discuss the following alternatives?

I agree, but this package will only support one string format. If you'd like to propose a single line format for this extended output, I'm happy to discuss changing it, but it will not be configurable.

[jd3nn1s]

I should clarify that I meant multiple lines of code.

I don't have a problem with multi-line log statements in my use-case, if there are more than a few levels of wrapped errors it could be pretty unreadable on a single line.

If the error message is used in JSON it should be encoded/escaped anyway in case something logs a quote.

[davecheney]

I'm sorry, I'm no very confused. Could you please explain to me what would be a good resolution to this issue for you. Thanks.

On Sun, 8 May 2016, 09:30 jd3nn1s, notifications@github.com wrote:

I should clarify that I meant multiple lines of code.

I don't have a problem with multi-line log statements in my use-case, if there are more than a few levels of wrapped errors it could be pretty unreadable on a single line.

If the error message is used in JSON it should be encoded/escaped anyway in case something logs a quote.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/pkg/errors/issues/20#issuecomment-217676710

[jd3nn1s]

A function that returns a string containing what .Fprint() writes to w.

[elithrar]

A function that returns a string containing what .Fprint() writes to w.

func printErrors(err error) string {
    var b bytes.Buffer
    errors.Fprint(&b, err)
    var s string
    // Format it however you'd like
    // ...
    return s
}

Would this wrapper not achieve what you are asking for? Right now there are already multiple conflicting requests—one to simply write out a string, and another to concatenate it with custom separators. All of this seems to be better suited to a simple helper function in your own application since it doesn't need to access any private types.

[jd3nn1s]

I'm suggesting something simpler:

func Sprint(err error) string {
    var b bytes.Buffer
    errors.Fprint(&b, err)
    return b.String()
}

Sure, it could be implemented outside as a helper function. IMHO it is useful enough to be part of the library the same way Print() is. It doesn't introduce any new string format either, it reuses the one established by Fprint().

[elithrar]

You are suggesting that, but @jimah is not—hence the issue (there's not even consensus, which is an indicator that this is better left outside of this package).

[jd3nn1s]

I think that's a false dichotomy. The question of changing the format is separate from making the existing format available as a string. @davecheney has also said that the package will only support one string format, so I'm not sure even that is in contention.

[davecheney]

@jd3nn1s I'm sorry but I won't be adding the function you described in https://github.com/pkg/errors/issues/20#issuecomment-217751374, it's not unique or important enough to add to this package. Fprintf is more general and can be used to implement the three line helper you suggested.

Thank you for your understanding.

[davecheney]

@jimah I'm concerned this issue is going around in circles. Could you please reply to https://github.com/pkg/errors/issues/20#issuecomment-217629390 with your thoughts. Thanks

[dammitjim]

@davecheney Hi Dave,

I believe the string representation should be separated by > symbols to indicate the failure path of the code. I agree that a helper function which only returns the string representation of the current format should stay outside of the package.

[dammitjim]

As a note, the one line string format could also be created as a helper function outside of the package in a fairly straightforward way as follows:

var b bytes.Buffer

Fprint(&b, err)

exploded := strings.Split(b.String(), "\n")

exploded = exploded[:len(exploded)-1]

return strings.Join(exploded, " > ")

Including this within the package would, however, encourage a standardised one line format.

So

readfile.go:27: could not read config readfile.go:14: open failed open /Users/dfc/.settings.xml: no such file or directory

Becomes

readfile.go:27: could not read config > readfile.go:14: open failed > open /Users/dfc/.settings.xml: no such file or directory

[davecheney]

@jimah why don't we just change the output from errrors.New().Error() and friends to include location information, ie

readfile.go:27: could not read config: readfile.go:14: open failed: open /Users/dfc/.settings.xml: no such file or directory

Then we can drop Print and Fprint entirely.

[dammitjim]

@davecheney I like this idea a lot, I do think that the separator between error causes needs to be something other than a colon though in case people want to transform it within their own applications (as colons are already used for file:line).

Maybe a semicolon?

[davecheney]

Sorry, it has to be a colon, that's the tradition established by fmt.Errorf and gopl.io

[davecheney]

@jimah alternatively, would you prefer something like this

StackTrace(err error) []string

[dammitjim]

Ah ok, that makes sense.

[dammitjim]

Apologies for closing and reopening, they put those buttons so close together!

A Stacktrace would be more flexible and won't interfere with people's current implementations, that may be the best path forward. It would also be better for our implementation as we could split the errors out into different json keys.

[davecheney]

@jimah i'm having trouble following your use case. In https://github.com/pkg/errors/issues/20#issuecomment-217817067 you propose transforming \n's to >'s, now you're talking about turing a []string into JSON.

It would be helpful if you could describe the end result, preferably in JSON.

[dammitjim]

The first, single line solution would result in this (excluding any other data):

{
  "error": "readfile.go:27: could not read config: readfile.go:14: open failed: open /Users/dfc/.settings.xml: no such file or directory"
}

Whereas the alternative stacktrace approach would result in this:

{
  "errors": [
    "readfile.go:27: could not read config",
    "readfile.go:14: open failed",
    "open /Users/dfc/.settings.xml: no such file or directory"
  ]
}

I'm open to implementing either result, I think the stacktrace approach would be easier to read especially if there are many errors chained.

[davecheney]

Why can't you just do this

var b bytes.Buffer
errors.Fprint(&b, err)
stack := strings.Split(b.String(), "\n")

[dammitjim]

That would work and I'm happy to go with it as a helper function outside of the package if it is decided not to be included.

Do you think there is value in offering this as a part of this package? It's not much code and it may prove useful to other people offering it out of the box.

[davecheney]

Do you think there is value in offering this as a part of this package? It's not much code and it may prove useful to other people offering it out of the box.

None, for two reasons.

1. if this package stands a chance of being included in the standard library, it must be small, much smaller than it is now.
2. All this helper would begat is another helper with a slightly different syntax, which would open the floodgates to a third, and so on.

[dammitjim]

Alright, good stuff.

Thank you for your time. I'm happy to close the issue if the other participants are.

[jd3nn1s]

Not surprised to see Print() removed after this discussion - I'm also fine with this issue being closed. Thank you for your work to improve error handling in Go!

[davecheney]

Thank you for helping me clarify the purpose of this package. It needs to either offer all the bells and whistles, every helper, every formatter, and so on, or offer none of them. And in the spirit of the lanaguge I've chosen the latter.

On Tue, 10 May 2016, 14:10 jd3nn1s, notifications@github.com wrote:

Not surprised to see Print() removed after this discussion - I'm also fine with this issue being closed. Thank you for your work to improve error handling in Go!

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/pkg/errors/issues/20#issuecomment-218054578

[davecheney]

Hello,

I wanted to draw your attention to #37 as a possible replacement for Fprintf and others. #37 is not a complete implementation, but the plan is to defer any formatting or layout decisions to the fmt package, controlled by various formatting modifiers.