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

[RFC] Add new action that allows to print text messages with timestamp and thread id but without severity #119

Closed
chshersh opened this issue May 6, 2019 · 6 comments · Fixed by #128
Labels
enhancement New feature or request log-action Everything related to LogAction data type package:co-log For: co-log question Further information is requested

Comments

@chshersh
Copy link
Contributor

chshersh commented May 6, 2019

Currently in order to print message with timestamp, you need to use Message data type which forces you to also specify severity. However, I may see that it might be more convenient sometimes to specify only textual payload and get the timestamp (and maybe thread id) automatically. This may help with moving from putStrLn to co-log easier.

The way to resolve this issue that I see is to change RichMessage data type. Currently it has the following shape:

https://github.com/kowainik/co-log/blob/1d700e04d73b544b871d42322e2f516f3b78c22a/co-log/src/Colog/Message.hs#L264-L268

I propose to perform the same thing that was done with the Message data type, specifically, change it to the following data type:

data RichMsg (m :: Type -> Type) (msg :: Type) = RichMessage
    { richMsgMsg :: {-# UNPACK #-} !msg
    , richMsgMap :: {-# UNPACK #-} !(FieldMap m)
    } deriving (Functor)

type RichMessage m = RichMsg m Message

We need to change types of couple extra functions and implement some formatting in case of textual data (the last one is tricky). Otherwise should be straightforward.

@chshersh chshersh added enhancement New feature or request question Further information is requested package:co-log For: co-log labels May 6, 2019
@sphaso
Copy link
Contributor

sphaso commented May 6, 2019

@chshersh if you don't mind, I'd like to take a stab at this if the proposal is accepted.

@chshersh
Copy link
Contributor Author

chshersh commented May 7, 2019

@sphaso This will be a good addition to the library. I think that the proposal currently in its final form, so feel free to work on that 🙂

@chshersh chshersh added the log-action Everything related to LogAction data type label May 9, 2019
@sphaso
Copy link
Contributor

sphaso commented May 11, 2019

@chshersh I've changed the RichMsg data type and added the RichMessage type as suggested above (had to include the DeriveFunctor extension). The build is clean and tests pass. Is output not tested? I seem to remember it was. Do you have any pointer on how to test this change properly?

@chshersh
Copy link
Contributor Author

@sphaso Try to run the tutorial-custom executable (an executable in the co-log package). And see whether the output has the same format as in the tutorial for that file. This helps you to check whether the old behavior is still working.

In order to check the new behaviour, you need to patch code in the play-colog executable (also in the co-log package) and add some custom logic there.

@sphaso
Copy link
Contributor

sphaso commented May 11, 2019

@chshersh sorry to keep bothering you. I thought of different ways of going about this but I lack some experience with the library. Since the RichMsg type now has a type parameter for the message, I assume you were thinking about creating a new (e.g.) SimpleMsg type without the severity? In this case I would have to refactor fmtRichMessageDefault to pattern match on the different types at some point (the good old RichMessage and SimpleMessage). Otherwise, the severity could be added (and then deleted for the tests) from the defaultMap, instead of sitting in the Msg data type. What do you think?

@chshersh
Copy link
Contributor Author

@sphaso No problems! This issue is not that straightforward and has open space for solutions.

In this case I would have to refactor fmtRichMessageDefault

No need for this. You can keep fmtRichMessageDefault as it is. But in addition you can implement:
fmtRichMessageDefault

fmtSimpleRichMessageDefault :: RichMessage Text -> m Text

So it will be only text, timestamp and thread id

sphaso added a commit to sphaso/co-log that referenced this issue May 21, 2019
chshersh pushed a commit that referenced this issue Jun 10, 2019
#128)

* [#119] new action that logs a message without the severity information

* useless UNPACK pragma

* Update co-log/src/Colog/Message.hs

Co-Authored-By: Dmitrii Kovanikov <kovanikov@gmail.com>

* Update co-log/src/Colog/Message.hs

Co-Authored-By: Dmitrii Kovanikov <kovanikov@gmail.com>

* Fixed indentation and function types

* fixed where indentation

* examples are working with polymorphic formatter for SimpleMsg

* upgradeMessageAction is generic, fixed tutorial accordingly
@chshersh chshersh added this to the v0.4.0.0: Polishtown milestone Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request log-action Everything related to LogAction data type package:co-log For: co-log question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants