Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better log output #4

Closed
brendt opened this issue Oct 13, 2017 · 11 comments
Closed

Better log output #4

brendt opened this issue Oct 13, 2017 · 11 comments
Labels

Comments

@brendt
Copy link
Contributor

brendt commented Oct 13, 2017

Currently, the log output is just a single line. There might be a better format.

Opinions are welcome.

@sebastiandedeyne
Copy link
Member

We've done print_r in the past, there's probably a better solution though.

@brendt
Copy link
Contributor Author

brendt commented Oct 13, 2017

I like a JSON format, because you could copy/paste it right into a REST client.

Maybe the headers should also be logged?

@brendt brendt mentioned this issue Oct 16, 2017
@brendt
Copy link
Contributor Author

brendt commented Oct 16, 2017

As it seems, all newlines are stripped when using the Log. This results in difficult to read output.

@brendt
Copy link
Contributor Author

brendt commented Oct 16, 2017

Turns out Monolog can have different formatters, and you can make your own. Laravel also has its wrapper around this: Illuminate\Log\Writer in which formatter can be configured.

I didn't go as far as making our own formatter, but it seems like a possibility, enabling new lines. I don't think it's worth the time now, but might be a nice feature for the future.

@freekmurze
Copy link
Member

I like a JSON format, because you could copy/paste it right into a REST client.

I very much like this idea.

@brendt
Copy link
Contributor Author

brendt commented Oct 16, 2017

@freekmurze But the question about formatting in the log output itself still stands. What's your opinion? To format the JSON in a pretty way, we'll have to create a custom formatter.

Functionally the unformatted JSON output will of course work just as fine; so pretty format might not be that important.

@pmatseykanets
Copy link

It would be nice to separate concerns of

  • figuring out what requests to log or not shouldLogRequest()
  • and format in which the log is being written logRequest()

This way regardless of logger/formatter requests that shouldn't be logged won't be.

@freekmurze
Copy link
Member

That's a pretty good idea, let's do that @brendt .

@sebastiandedeyne
Copy link
Member

Confused, isn't that what it looks like now? 🤔

https://github.com/spatie/laravel-http-logger/blob/48f75eeba9cbcc87471f9c880e5a3288d29d504b/src/DefaultLogProfile.php

@brendt
Copy link
Contributor Author

brendt commented Oct 17, 2017

I share @sebastiandedeyne's confusion, or were you maybe @pmatseykanets was referring to further split it in separate classes?

@freekmurze
Copy link
Member

Separating concerns will be handled in #7

Current log output is good enough for a v1.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants