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

pkg/logger #10

Merged
merged 17 commits into from
Jul 2, 2021
Merged

pkg/logger #10

merged 17 commits into from
Jul 2, 2021

Conversation

applejag
Copy link
Contributor

@applejag applejag commented Jun 18, 2021

Logging! It's finally here!

Took heavy inspiration from https://github.com/rs/zerolog in the architecture and pretty console output.
The code base may seem bloated, but I think it's more readable than zerolog's source code.

Dont be afraid of bashing it even though it has all those pretty comments in it.

Motive

The requested feature set was this:

  • LogLevel
  • Multiple writers/sinks (ex: JSON for Kibana & pretty for console)
  • Scope per-module configs (ex: warn for Gin, info for API audits)
  • Easy integration with Gin-Gonic
  • Easy integration with GORM
  • Easy integration with io.Writer's, ex gin.DefaultWriter & gin.DefaultErrorWriter
  • Fields (WithString, WithInt...)

When coding this I tried to find an existing logger with similar features, and then wrap it inside some abstraction. I then noticed that the concrete implementation is so small so I chose to not depend on any external logging library.

Preview

Sample output of the different pre-packaged Sink types.

pkg/logger/consolepretty

consolepretty

pkg/logger/consolejson

{"level":"debug","date":"2021-06-18T15:01:29+02:00","caller":"wharf-core/main.go","line":20,"scope":"TEST","message":"Sample","hello":"world"}
{"level":"info","date":"2021-06-18T15:01:29+02:00","caller":"wharf-core/main.go","line":21,"scope":"TEST","message":"Sample","hello":"world","error":"EOF"}
{"level":"warn","date":"2021-06-18T15:01:29+02:00","caller":"wharf-core/main.go","line":22,"scope":"TEST","message":"Sample","hello":"world","someSpan":1000000000}
{"level":"error","date":"2021-06-18T15:01:29+02:00","caller":"wharf-core/main.go","line":23,"scope":"TEST","message":"Sample","hello":"world"}
{"level":"debug","date":"2021-06-18T15:01:29+02:00","caller":"gorm@v1.21.11/callbacks.go","line":133,"scope":"GORM","rows":0,"elapsed":120476,"sql":"SELECT * FROM \"users\" WHERE \"users\".\"id\" = 125 AND \"users\".\"deleted_at\" IS NULL"}
{"level":"warn","date":"2021-06-18T15:01:29+02:00","caller":"fmt/print.go","line":205,"scope":"GIN-debug","message":"Running in \"debug\" mode. Switch to \"release\" mode in production.\n - using env:\texport GIN_MODE=release\n - using code:\tgin.SetMode(gin.ReleaseMode)"}
{"level":"debug","date":"2021-06-18T15:01:29+02:00","caller":"fmt/print.go","line":205,"scope":"GIN-debug","message":"Environment variable PORT is undefined. Using port :8080 by default"}
{"level":"debug","date":"2021-06-18T15:01:29+02:00","caller":"fmt/print.go","line":205,"scope":"GIN-debug","message":"Listening and serving HTTP on :8080"}
{"level":"debug","date":"2021-06-18T15:01:37+02:00","caller":"gin@v1.7.1/logger.go","line":268,"scope":"GIN","clientIp":"::1","method":"GET","path":"/","status":404,"latency":311}
{"level":"debug","date":"2021-06-18T15:01:38+02:00","caller":"gin@v1.7.1/logger.go","line":268,"scope":"GIN","clientIp":"::1","method":"GET","path":"/favicon.ico","status":404,"latency":391}

@applejag applejag added the enhancement New feature or request label Jun 18, 2021
@applejag applejag added this to In progress in Backlog via automation Jun 18, 2021
@applejag applejag self-assigned this Jun 18, 2021
@Pikabanga
Copy link

Before I review this, I have an important question.

Is this flog compatible? :D

@applejag
Copy link
Contributor Author

Before I review this, I have an important question.

Is this flog compatible? :D

Hehe the JSON variant is, but will add support for our custom syntax

pkg/logger/event.go Outdated Show resolved Hide resolved
pkg/logger/context.go Outdated Show resolved Hide resolved
Copy link
Contributor

@fredx30 fredx30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For 2000+ lines of new functionality we could have an rfc.
The description is good in the aspect that it describes what it is. It need a description of why this. Why not any other exsisting plug and play log library?

@applejag
Copy link
Contributor Author

@fredx30
For 2000+ lines of new functionality we could have an rfc.
The description is good in the aspect that it describes what it is. It need a description of why this. Why not any other exsisting plug and play log library?

Have updated the description with this:

The requested feature set was this:

  • LogLevel
  • Multiple writers/sinks (ex: JSON for Kibana & pretty for console)
  • Scope per-module configs (ex: warn for Gin, info for API audits)
  • Easy integration with Gin-Gonic
  • Easy integration with GORM
  • Easy integration with io.Writer's, ex gin.DefaultWriter & gin.DefaultErrorWriter
  • Fields (WithString, WithInt...)

When coding this I tried to find an existing logger with similar features, and then wrap it inside some abstraction. I then noticed that the concrete implementation is so small so I chose to not depend on any external logging library.

Hopefully that answers your wonder.

It was quite difficult to split this one up into smaller PRs.

An RFC would've been suitable here, yes.

Copy link
Member

@Alexamakans Alexamakans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly nits and typo corrections from me.

internal/traceutil/traceutil.go Outdated Show resolved Hide resolved
internal/traceutil/traceutil.go Outdated Show resolved Hide resolved
pkg/logger/consolejson/json.go Outdated Show resolved Hide resolved
pkg/logger/consolejson/json.go Outdated Show resolved Hide resolved
pkg/logger/consolepretty/pretty.go Outdated Show resolved Hide resolved
pkg/logger/consolepretty/pretty.go Outdated Show resolved Hide resolved
pkg/logger/context.go Outdated Show resolved Hide resolved
Backlog automation moved this from In progress to Review in progress Jun 28, 2021
pkg/logger/logger_test.go Outdated Show resolved Hide resolved
Co-authored-by: Alexamakans <79503481+Alexamakans@users.noreply.github.com>
applejag added a commit to applejag/flog that referenced this pull request Jun 29, 2021
The logging style we added in iver-wharf/wharf-core#10. The JSON logging
was already supported, but the pretty logging is from now on also
supported.

Preview:

![consolepretty](https://user-images.githubusercontent.com/2477952/122565545-e862a880-d046-11eb-965c-9b513e4c7045.jpg)
pkg/logger/context.go Outdated Show resolved Hide resolved
pkg/logger/mock.go Outdated Show resolved Hide resolved
applejag and others added 3 commits June 30, 2021 16:07
Co-authored-by: Alexamakans <79503481+Alexamakans@users.noreply.github.com>
Backlog automation moved this from Review in progress to Reviewer approved Jun 30, 2021
Copy link
Member

@Alexamakans Alexamakans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏻

Backlog automation moved this from Reviewer approved to Review in progress Jun 30, 2021
Copy link
Member

@Alexamakans Alexamakans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, forgot these. Sorry.

pkg/logger/context.go Outdated Show resolved Hide resolved
pkg/logger/context.go Outdated Show resolved Hide resolved
Co-authored-by: Alexamakans <79503481+Alexamakans@users.noreply.github.com>
@applejag applejag requested a review from Alexamakans July 1, 2021 15:53
Copy link
Member

@Alexamakans Alexamakans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏻
For real this time!

Backlog automation moved this from Review in progress to Reviewer approved Jul 1, 2021
@applejag applejag merged commit 1601d2a into master Jul 2, 2021
Backlog automation moved this from Reviewer approved to Done Jul 2, 2021
@applejag applejag deleted the feature/logging-core branch July 2, 2021 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants