Hi, Thanks. Comments inline.
On Mon, Jun 16, 2014 at 9:36 AM, Sven Van Caekenberghe <[email protected]> wrote: > > On 15 Jun 2014, at 15:04, Tudor Girba <[email protected]> wrote: > > > Hi, > > > > I like very much the new energy people are putting into creating the > SystemLogger engine for Pharo. I think this is a specifically important > area for which we have to have a solution out of the box. At the same time, > I also think that Pharo provides an infrastructure that makes room for > ideas that are otherwise hard to reach in other languages or environments. > > > > Stef asked for collaborations around this project, so here is my > literally small contribution: a rather different logging engine. It is > called Beacon, it is based entirely on Announcements, it has ~200 lines of > code, it has no tags or levels, and in my opinion it is fully functional. > > > > You can see a detailed description here including some informal > comparisons with SystemLogger: > > http://www.humane-assessment.com/blog/beacon > > > > Please let me know what you think. I would be happy to join forces to > reach a mature solution that is both versatile and that can show how Pharo > is different. > > Here is some of my feedback: > > - Like I said in my document and like I tried to show in my implementation > for Zinc, I want framework agnostic pure object logging, so I don't want to > subclass from anything (I do from Announcement, but that is an > implementation detail, the technique how #emit is done), can you deal with > the ZnNewLogEvents as the are ? > As you can see at the end of the post, you can just use your own Announcements. There is no real magic there. So, it should be no technical problem to accommodate the logging of ZincEvents as they are. However, one thing that we should also think about is the actual analyses that we might want to do. For example, the inspector extension for RecordingBeacon uses: - timestamp, and - printOneLineContentsOn: And these requirements will grow even more as we are building more specialized tools. So, to this end, it is useful to have some common ground. Now, as far as I know, the only reason why you do not want to subclass BeaconSignal is because you want to use ZnTimestamp. I see no reason to have two Timestamps in the image anyway, so I have no problem pushing just one. Or do you have another reason? > - I find what you did in StackSignal very interesting, when putting > Exceptions in log objects, a natural thing to do, the same problems occur. > I think we are just exploring this area and have lots to learn (debugging > in the past). > Exactly. Andrei and I already used the StackSignal for a real debugging session and I will blog about it shortly. > - The fact that FuelBeacon creates a new file for every signal does not > strike me as efficient enough for real world use. Also, using the > #printString as filename is weird for me. > As mentioned in the post, this is more of an example than something to be used for real. We definitely need real solutions in this space. > - I am also wondering whether the fact that Announcements are synchronous > (I mean distributed to consumers on the same thread as the one that > produced them) is good or not because like this, slow backend processing > for whatever reason will kill performance of the producing code. > Good point. I thought about it as well, but I do not have a good solution. It would definitely be interesting to have solutions both for sync and async. > - In your blog post (very good as usual ;-) I am still bothered by the > usage of strings in the examples (I know you also use the stack signal, but > that is more advanced). That way, reader will still think about text. We > need better examples that show the power of object logging. I'll see if I > can come up with something. > I know! It pained me as well, but I did not have good examples at hand (except for the stack one that I needed first hand) and I thought that it is better to bring it up for discussion than wait until it is too late. > - Small code change > > StringStreamBeacon>>#handle: anAnnoucement > self stream > nextPutAll: anAnnoucement printString; > cr > > would be more efficient (no intermediate string) as > > StringStreamBeacon>>#handle: anAnnoucement > self stream > print: anAnnoucement; > cr > > or even > > StringStreamBeacon>>#handle: anAnnoucement > anAnnoucement printOn: self stream. > self stream cr > > to emphasise the double dispatch. Good catch! Committed. Cheers, Doru > Cheers, > > Doru > > > > -- > > www.tudorgirba.com > > > > "Every thing has its own flow" > > > -- www.tudorgirba.com "Every thing has its own flow"
