[WIP] Log marker

[cyucelen]

It stands to reason that one of the strongest use cases would be for users to colorize their logs for easy reading.

• [x] Build a abstraction structure upon Golang's logger to colorize. (We can experiment here!)
• [x] Implement io.Writer
• [x] Add Mark Rules (Mark Rule name can change)
  • [x] Builder way
  • [x] Passing Rule slice
• [x] README documentation

We can add further features to the checklist!

Closes #17

[codecov-io]

Codecov Report

Merging #24 into master will decrease coverage by 1.62%. The diff coverage is 88.23%.

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
- Coverage     100%   98.37%   -1.63%     
==========================================
  Files           3        4       +1     
  Lines         106      123      +17     
==========================================
+ Hits          106      121      +15     
- Misses          0        1       +1     
- Partials        0        1       +1

Impacted Files	Coverage Δ
log.go	88.23% <88.23%> (ø)
matcher.go	100% <0%> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 03dc5bf...70b11f8. Read the comment docs.

[cyucelen]

Do we need to create a variadic AddRule method? What do you think about its usefulness?

[Darkclainer]

I think it should be another PR. See issue #27.

[cyucelen]

What should be another PR? Buffering writer output until \n?

[jnatalzia]

Re: Variadic method

For the first pass I feel like the slice approach is good enough but if it's not a huge ordeal we might as well put it in this PR. What do you think the LoE is?

[cyucelen]

Oh, we need to test the rule apply order by the way. Since we are applying the rules sequentially, some matches can overlap - later order matcher will override the previous ones if there is an overlap.

Users need to be aware of it and add their mark rules according to the order too. We need to document this and give an example for it too.

[cyucelen]

Re: Variadic method

For the first pass I feel like the slice approach is good enough but if it's not a huge ordeal we might as well put it in this PR. What do you think the LoE is?

I don't know what LoE is :smile: but we can skip it for now, less is more!

[cyucelen]

Implemented some helper functionality for our assertions in assert.go and tested mark rules with it. Can you check it out? Then we can resolve the corresponding reviews.

[Darkclainer]

You can not compare function. See https://stackoverflow.com/questions/42140758/collection-of-unique-functions-in-go/42147285#42147285

[cyucelen]

You can not compare function. See https://stackoverflow.com/questions/42140758/collection-of-unique-functions-in-go/42147285#42147285

This approach actually looks like working. I tested it for our MatcherFunc type and it fails when MatcherFuncs actually not equal.

[Darkclainer]

This behaviour is implementation defined.

[cyucelen]

This behaviour is implementation defined.

You mean undefined?

[Darkclainer]

No. Current standard says that functions are incomparable, see https://golang.org/ref/spec#Comparison_operators . Using reflect in that particular case, with current go implementation you got desired behavior.

[cyucelen]

I get it.

This behavior is not specified and so, can vary between different compiler implementations. Here is the question; is it really a big problem for us?

• If it is, then, is there any other option for directly testing the AddRules behaviour via asserting MarkRule equality. (You said something about wrapping it with interface?)
• Or, we can test it indirectly in Write functions' tests, this is fine too. (I mean just testing the behavior of Write can cover AddRules logic too)

I prefer the second way if the tests for the first option will impact the implementation and make it more complex.

[Darkclainer]

I think second option is fine, but you should come up with test that cover order of rules too.

[cyucelen]

Aye aye, captain!

[jnatalzia]

@cyucelen is it just the readme docs that remain for this feature? super excited to integrate it into some projects

[cyucelen]

@jnatalzia yes, can you help about docs?

[jnatalzia]

Can do!

[jnatalzia]

Docs added! Also refactored a small portion of the code, added a comment explaining what and why to the PR

[cyucelen]

We merged the @Darkclainer s unified image PR to master. Can you integrate it to this branch? I will review your last commit about Writer asap!

[jnatalzia]

Yes I can! Also seems like I need to add some code coverage, gotta love CI. Will fix it up today at some point

[codecov-io]

Codecov Report

Merging #24 into master will not change coverage. The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #24   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      4    +1     
  Lines         106    121   +15     
=====================================
+ Hits          106    121   +15

Impacted Files	Coverage Δ
log.go	100% <100%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 03dc5bf...71e4d90. Read the comment docs.

[jnatalzia]

Hmmm @Darkclainer can we make an issue/PR to update the docs for using the image generator? It requires a pip install (maybe foreign for a go repo) as well as two separate terminal dependencies but no indication of where to get them. I can guess but given I'm installing them in the shell, I'd prefer to have sanctioned sources

cc @cyucelen

[cyucelen]

Awesome PR @jnatalzia!

I just added some docs for image_generator, also updated the images of log feature in readme.

Maybe we can get rid of functional options in WriteMarker since we are not using it anymore ?

Looks like we are getting ready for the deployment! :sunglasses:

Also, what do you think about #29 ? @jnatalzia @Darkclainer

[jnatalzia]

Maybe we can get rid of functional options in WriteMarker since we are not using it anymore ?

Sounds like a plan! Will push an update with those removed and then I think we're good to merge. Will mark as no longer draft.

Will comment on #29 on the issue!

[cyucelen]

Looks like we are good to go! Do the final touch, merge it!