These are chat archives for exceptionless/Discuss

22nd
Feb 2016
Eric J. Smith
@ejsmith
Feb 22 2016 01:51
aspnet/Logging#366
Hoping that they will make some changes to that interface before release.
Eric J. Smith
@ejsmith
Feb 22 2016 06:17
exceptionless/Foundatio#28
Got the logging stuff mostly done
would love to get some feedback @jamie94bc @frankebersoll @niemyjski
benmaina
@benmaina
Feb 22 2016 06:45
@niemyjski hello needed some help over here.I hosted the exceptionless (Production).Wanted to know how t configure so that my developers can use it to log their exceptions.Kindly assist
Jamie Clarke
@jamie94bc
Feb 22 2016 08:04
the build is failing with "A stable release of a package should not have on a prerelease dependency. Either modify the version spec of dependency Microsoft.Extensions.Logging.Abstractions (� 1.0.0-rc1-final)" or update the version field." - I thought that was removed?
@ejsmith LGTM - there's a few strings which use numerical rather than named formatting though
and I'm not the biggest fan of .Error()Exception().Message().Write() - would prefer:
// With exception
_logger.Error(ex, "Unhandled {ExceptionType} was thrown", ex.GetType().Name);
// Without
_logger.Error("Failed to do perform action {ActionName}",  myAction.Name);
Jamie Clarke
@jamie94bc
Feb 22 2016 08:20
some of the overloads are useful - but I think there should be single methods for common usage too :smile:
Blake Niemyjski
@niemyjski
Feb 22 2016 13:32
@benmaina sure let me know your questions. You’d just need to configure the server url that the clients point to. https://github.com/exceptionless/Exceptionless.Net/wiki/Configuration#self-hosted-options
kinda agree with @jamie94bc but I’d errro on the side of doing what the .net framework is doing (I’m unsure what they are doing in this use case)
I think we should mock there api (1 to 1)
would be super nice if we just could not even compile our logging stuff when targeting core clr :)
Jamie Clarke
@jamie94bc
Feb 22 2016 13:36
i'm not a massive fan of the method signature they've chosen...
public static void LogDebug(this ILogger logger, EventId eventId, Exception exception, string message, params object[] args)
Blake Niemyjski
@niemyjski
Feb 22 2016 13:36
wth, event id?
Jamie Clarke
@jamie94bc
Feb 22 2016 13:37
yeah, exactly
Blake Niemyjski
@niemyjski
Feb 22 2016 13:37
I hate that they have formatted args
Jamie Clarke
@jamie94bc
Feb 22 2016 13:37
also "LogDebug" so you're doing _logger.LogDebug
Blake Niemyjski
@niemyjski
Feb 22 2016 13:37
like you now have $”{blah}
why do we need it anhmore
Jamie Clarke
@jamie94bc
Feb 22 2016 13:37
for structured logging!
that's what serilog is all about
Blake Niemyjski
@niemyjski
Feb 22 2016 13:37
yeah that’s stupid
too verbose
what do you mean structured logging
it’s for concatting messages together no?
Jamie Clarke
@jamie94bc
Feb 22 2016 13:38
errr ish
so if I go _logger.Information(ex, "Queue entry {QueueEntryId} failed with {ExceptionType}", queueEntry.Id, ex.GetType().Name);
Blake Niemyjski
@niemyjski
Feb 22 2016 13:39
yeah?
but using the new syntax
you could just do
Jamie Clarke
@jamie94bc
Feb 22 2016 13:39
you can make the log entries searchable
so QueueEntryId == 1
would return the entry i've just logged
Blake Niemyjski
@niemyjski
Feb 22 2016 13:40
_logger.Information(ex, $"Queue entry {queueEntry.Id} failed with {ex.GetType().Name}");
hmm
yeah that would be kind of cool
Jamie Clarke
@jamie94bc
Feb 22 2016 13:40
Blake Niemyjski
@niemyjski
Feb 22 2016 13:40
so it does the format and then adds named context data to the log event
Jamie Clarke
@jamie94bc
Feb 22 2016 13:41
i think seq formats at runtime
stores the template and variables separately
Blake Niemyjski
@niemyjski
Feb 22 2016 13:42
yeah
so question
they have to parse that which might be slower but
why do they need event id?
Jamie Clarke
@jamie94bc
Feb 22 2016 13:42
no idea why msft need event id
or where you'd get it from
so there is an overload where it's not required, but you can't log exceptions without specifying eventid
public static void LogError(this ILogger logger, string message, params object[] args)
Blake Niemyjski
@niemyjski
Feb 22 2016 13:44
want to create an issue asking them? Or I can
Jamie Clarke
@jamie94bc
Feb 22 2016 13:44
go for it! :smile:
Blake Niemyjski
@niemyjski
Feb 22 2016 13:49
aspnet/Logging#367
please add your comments
aspnet/Logging#368
Blake Niemyjski
@niemyjski
Feb 22 2016 13:52
yeah?
Jamie Clarke
@jamie94bc
Feb 22 2016 13:52
integer event ids :cry:
This message was deleted
Blake Niemyjski
@niemyjski
Feb 22 2016 13:53
:(
shouldn’t be there
Eric J. Smith
@ejsmith
Feb 22 2016 13:53
@niemyjski @jamie94bc We need to get rid of all $ strings in logging.
Jamie Clarke
@jamie94bc
Feb 22 2016 13:53
that's for the sink / behaviours to deal with
not the api
@ejsmith agreed!
Eric J. Smith
@ejsmith
Feb 22 2016 13:53
I am just taking an incremental approach.
The builder stuff is sitting on top of their API so that I didn't have to go change everything.
And I actually like the builder API. Seems like it works pretty well.
So at first I was thinking I would source import their stuff and no take on the dependency, but I changed my mind. It was too much of a PITA and was probably silly to not want a small dependency.
They are using the event id to be able to quickly identify messages. Which are good because I will put that into log stash as a property .
Also kind of good for test assertions.
Jamie Clarke
@jamie94bc
Feb 22 2016 13:59
i can see how it would be useful for test assertions
but shouldn't be required for logging exceptions
Eric J. Smith
@ejsmith
Feb 22 2016 14:02
Yeah, can just set it to 0, but there is an issue requesting an extension method.
And we can build our own extension methods as well. If needed.
Jamie Clarke
@jamie94bc
Feb 22 2016 14:02
yeah, that's a big benefit over a static class
Eric J. Smith
@ejsmith
Feb 22 2016 14:02
The API already changes quite a bit from RC1 to RC2
Yep
The logging output for the tests seems pretty nice too. :-) Seems to be working.
Blake Niemyjski
@niemyjski
Feb 22 2016 14:03
:)
Eric J. Smith
@ejsmith
Feb 22 2016 14:05
I made the logger factory null able in all the services. I am thinking the plan is that unless you want logging, then you don't give us a logger
Blake Niemyjski
@niemyjski
Feb 22 2016 14:05
yeah
question is
Eric J. Smith
@ejsmith
Feb 22 2016 14:05
Most people don't want log messages from a 3rd party lib.
Blake Niemyjski
@niemyjski
Feb 22 2016 14:05
will some di containers auto inject it
Eric J. Smith
@ejsmith
Feb 22 2016 14:06
Well if they do then they would have to configure their log levels.
Most DIs don't like our constructors anyway because of optional parameters
So you normally have to use a lambda to create the object in your DI
I removed all the #if conditionals because the way they have structured the logging makes it so it can be a very cheap operation.
Blake Niemyjski
@niemyjski
Feb 22 2016 14:29
wait
ah ok
you use a null logger instance
Eric J. Smith
@ejsmith
Feb 22 2016 15:28
yes, it should be very cheap to leave those log calls in there
only expensive part right now is the string.
but all those strings will get interned because they are the same.
but that is why we need to go kill all $”” strings
Blake Niemyjski
@niemyjski
Feb 22 2016 15:29
yeah
Eric J. Smith
@ejsmith
Feb 22 2016 15:29
because that is causing work to be done when the log message might not even be written
Blake Niemyjski
@niemyjski
Feb 22 2016 15:29
want me to do that
Eric J. Smith
@ejsmith
Feb 22 2016 15:29
also, the interface that have has ways of making logging even cheaper.
Blake Niemyjski
@niemyjski
Feb 22 2016 15:29
or are you going to do that
Eric J. Smith
@ejsmith
Feb 22 2016 15:30
I will probably do that… I want you to take a look at the session stuff I dud
did
and try and finish that up.
Blake Niemyjski
@niemyjski
Feb 22 2016 15:30
ok
Eric J. Smith
@ejsmith
Feb 22 2016 15:30
because that is affecting users
Blake Niemyjski
@niemyjski
Feb 22 2016 15:30
ok
I’ll take a look into that
create a github issue for the model changes and well discuss it
Eric J. Smith
@ejsmith
Feb 22 2016 15:31
we need to finish that and then work with bill and boyd to make sure it’s working correctly
Blake Niemyjski
@niemyjski
Feb 22 2016 15:31
ok
Jamie Clarke
@jamie94bc
Feb 22 2016 15:51
woo!
that would be very useful
Eric J. Smith
@ejsmith
Feb 22 2016 15:51
So @jamie94bc the pre-release thing on the logging lib is gonna be a PITA.
Jamie Clarke
@jamie94bc
Feb 22 2016 15:52
and space-saving
Eric J. Smith
@ejsmith
Feb 22 2016 15:52
yeah, I wish they would build that into .net.
they should know if a method is called using the async keyword and then flatten out the stack for those.
Jamie Clarke
@jamie94bc
Feb 22 2016 15:52
agreed - strange they abstract away async but still give you the stack trace for the underlying implementation
Eric J. Smith
@ejsmith
Feb 22 2016 15:53
if you do your own continuations then it can’t use it
which is fine
because you expect those stack traces to be complicated.
Jamie Clarke
@jamie94bc
Feb 22 2016 16:00
what's the issue with the pre-release dependency?
Eric J. Smith
@ejsmith
Feb 22 2016 16:10
are you the one that was saying the build was failing from it?
don’t think it will let you build a non-prerelease package that contains a prerelease package.
Jamie Clarke
@jamie94bc
Feb 22 2016 16:10
aha
yeah i remember - A stable release of a package should not have on a prerelease dependency.
Eric J. Smith
@ejsmith
Feb 22 2016 16:11
yeah
so that is kind of an issue because that package is probably going to be pre-release for a while.
going down the path of source importing was gonna be a big pita too.
kind of going against the grain when there is already a lot of traction behind this logging abstraction.
kind of feel like we probably should go with the flow and be ok taking on package dependencies
because in the new .net core world, everything is a package.
and building packages without package dependencies will be impossible.
Blake Niemyjski
@niemyjski
Feb 22 2016 16:14
yeah
we could always make logging a third nuget package
but then I guess our other packages would require it
so yeah.. guess I’m fine with it
Jamie Clarke
@jamie94bc
Feb 22 2016 16:33
okay - so source import or make the package pre-release?
i'd say source import for now - it could be pre-release until the end of the year
Eric J. Smith
@ejsmith
Feb 22 2016 16:40
Doing that causes a lot of issues too. There is a lot of code to bring in for the logging implementations to make the
Jamie Clarke
@jamie94bc
Feb 22 2016 16:51
:cry:
well we could just use what you've done so far to diagnose the failing test?
and leave it until the logging lib reaches GA
Blake Niemyjski
@niemyjski
Feb 22 2016 16:56
yeah
Blake Niemyjski
@niemyjski
Feb 22 2016 17:03
@ejsmith what’s up with the session stuff you were working on
your grouping sesions into groups (session start triggers a new group
then you do this
// cancel duplicate start events
session.Where(ev => ev.Event.IsSessionStart()).Skip(1).ForEach(ev => ev.IsCancelled = true);
which will never happen
so how do they ever get rolled up?
wait
never mind your doing it by session end
need to get glasses
                // cancel heart beat events (we don't save them)
                session.Where(ev => ev.Event.IsSessionHeartbeat()).ForEach(ev => ev.IsCancelled = true);
I almost think we should hide them
if someone is sending session heartbeats
and they eat up there plan limit
they are going to be like WTF
and we are going to be like ahhh
I mean they add no value other than to be a marker but it’s going to be hard to explain the event difference
Blake Niemyjski
@niemyjski
Feb 22 2016 17:25
so I show that you rolled up session ids
but you didn’t cancel existing session ends
Eric J. Smith
@ejsmith
Feb 22 2016 18:00
yeah, that is the reason for not tossing them
people need to know where their events are being used.
so we need to not ditch them
what do you mean rolled up session ids?
Blake Niemyjski
@niemyjski
Feb 22 2016 18:02
I fixed a bug
with that
that I’ll commit
like the new logic a bit more straight forward
so you think just mark them as hidden?
or cancel them?
            GenerateEvent(firstEventDate, "eric@exceptionless.io"),
            GenerateEvent(firstEventDate.AddSeconds(30), "eric@exceptionless.io", Event.KnownTypes.Session),
            GenerateEvent(lastEventDate, "eric@exceptionless.io")
in this case the plugin won’t sync the session start time
should we?
the first event logic will also not work right
Eric J. Smith
@ejsmith
Feb 22 2016 18:03
hidden
Blake Niemyjski
@niemyjski
Feb 22 2016 18:03
don’t really support that scenario
Eric J. Smith
@ejsmith
Feb 22 2016 18:03
don’t know what that means.
Blake Niemyjski
@niemyjski
Feb 22 2016 18:04
like
first event is 30 seconds in the past and a normal event
30 seconds later is a session start event
all of our logic is checking the last or first event context
so the session start event will be 30 seconds from where it should be
Blake Niemyjski
@niemyjski
Feb 22 2016 18:10
should we just discard any event that occurred before a session start grouping for event?
eh probably not
btw, heartbeats should be ignored at the stack level
are you fine with special casing it there
I am
Eric J. Smith
@ejsmith
Feb 22 2016 18:10
no
no idea what you mean there either.
Blake Niemyjski
@niemyjski
Feb 22 2016 18:11
if you click mark not hidden at the stack level for heartbeats we should respect that...
Eric J. Smith
@ejsmith
Feb 22 2016 18:11
yes
Blake Niemyjski
@niemyjski
Feb 22 2016 18:11
saying that assign to stack should look if it’s a new stack and a heartbeat and if it is we mark it as hidden
Eric J. Smith
@ejsmith
Feb 22 2016 18:11
actually thats what we should be using in the session events tab.
filter out hidden
Blake Niemyjski
@niemyjski
Feb 22 2016 18:11
yeah
Eric J. Smith
@ejsmith
Feb 22 2016 18:11
and give them the ability to show hidden too
Blake Niemyjski
@niemyjski
Feb 22 2016 18:11
yeah...
so what I’m saying is you have 3 events.. the session start one is in the middle
Eric J. Smith
@ejsmith
Feb 22 2016 18:12
so they could see how many heartbeats got sent.
Blake Niemyjski
@niemyjski
Feb 22 2016 18:12
we don’t sync the start time of that session event to the first event
and all of our logic goes off of the first event in the session group
Eric J. Smith
@ejsmith
Feb 22 2016 18:12
we have the 1st event and the start
Blake Niemyjski
@niemyjski
Feb 22 2016 18:12
yeah
                var firstSessionEvent = session.First();
                var lastSessionEvent = session.Last();
Eric J. Smith
@ejsmith
Feb 22 2016 18:12
we could set the session start event time to be the less of the 2
don’t know if we really need to be going and updating session starts from previous batches of events for the start time though.
Blake Niemyjski
@niemyjski
Feb 22 2016 18:13
yeah
I kinda of agree
Eric J. Smith
@ejsmith
Feb 22 2016 18:13
they should send the session start 1st
Blake Niemyjski
@niemyjski
Feb 22 2016 18:13
but we are putting them all into the same session
Eric J. Smith
@ejsmith
Feb 22 2016 18:13
yes
but here is the problem
if the 1st event is in a different batch, it would auto create a start event
so we are back to the part where we would need to merge session starts.
don’t want to do that right now
Blake Niemyjski
@niemyjski
Feb 22 2016 18:14
so what do we do in this case..
Eric J. Smith
@ejsmith
Feb 22 2016 18:15
just update the session start event to use the less of the the 1st event and the session event dates
when they are in the same batch
Blake Niemyjski
@niemyjski
Feb 22 2016 18:15
seems like CreateSessionGroups should look at the event date
and if it’s x time apart between events it creates new session
like days apart
Eric J. Smith
@ejsmith
Feb 22 2016 18:15
maybe
Blake Niemyjski
@niemyjski
Feb 22 2016 18:16
ok
Eric J. Smith
@ejsmith
Feb 22 2016 18:16
that would be some sort offline scenario
Blake Niemyjski
@niemyjski
Feb 22 2016 18:18
yeah
but for auto sessions
if the event dates are so far apart
I’d think they would be different event sessions
like 6+ hours between events is a crazy long time
a day apart is like an eternity.
Blake Niemyjski
@niemyjski
Feb 22 2016 18:24
what should we do with all existing heartbeats in the system
after we make this change...
they are all going to be visible
I could make a manual job that changes them all to hidden
Sander Rijken
@srijken
Feb 22 2016 18:33
hi
Blake Niemyjski
@niemyjski
Feb 22 2016 18:33
hey
Sander Rijken
@srijken
Feb 22 2016 18:34
I’ll try to work on the tests tonight, might have some questions
tonight is now btw
Blake Niemyjski
@niemyjski
Feb 22 2016 18:34
hehe
sounds good
I’ll be around for 4 more hours
then working out
Sander Rijken
@srijken
Feb 22 2016 18:34
been sick, no working out yet
maybe tomorrow :)
Blake Niemyjski
@niemyjski
Feb 22 2016 18:35
:(
erics been lucky
if I had kids I think I’d be sick a lot more often
every day they come home with new fauna lol
Sander Rijken
@srijken
Feb 22 2016 18:35
yeah
one time I thought it would be a good idea to do some bicycling while the whole coughing and snot situation was still around.. thought it’d help solve the problem… bad idea
Blake Niemyjski
@niemyjski
Feb 22 2016 18:39
Just eat something hot like wings and that will clear you up :)
Sander Rijken
@srijken
Feb 22 2016 18:44
hehehe
Blake Niemyjski
@niemyjski
Feb 22 2016 18:47
just heard this on the radio… should play this when there is only one exception
Jamie Clarke
@jamie94bc
Feb 22 2016 18:48
haha that would be excellent
licensing issues maybe ;)
Blake Niemyjski
@niemyjski
Feb 22 2016 18:48
yeah
lol
embedded youtube
I need a vacation from just reading that
lol
Blake Niemyjski
@niemyjski
Feb 22 2016 19:30
exceptionless/Exceptionless#195
Jamie Clarke
@jamie94bc
Feb 22 2016 19:42
totally agree re needing vacations!
Blake Niemyjski
@niemyjski
Feb 22 2016 19:43
@ejsmith can you review that pr and merge it when ready. everything passes
is anyone using the pcl client in windows phone 8.1?
Just got an email saying there were build time errors when it was referenced in that project and we specifically target that profile
Eric J. Smith
@ejsmith
Feb 22 2016 19:52
they need to hurry up with the netstandard crap
Blake Niemyjski
@niemyjski
Feb 22 2016 20:17
yeah
@ejsmith merge it good :)
should probably add a unit tests for the marking the stack as hidden
guess I could do that now
Eric J. Smith
@ejsmith
Feb 22 2016 20:21
don’t think all the tests passed.
Blake Niemyjski
@niemyjski
Feb 22 2016 20:21
they were random perf ones
all the stack ones passed
not sure what the heck is up with appveyor lately
all pass locally
added two new tests
guess well see if they pass npw
Blake Niemyjski
@niemyjski
Feb 22 2016 20:27
I’m starting to think microsoft should rebrand asp.net core again
due to the poor execution and the state of nuget
I’ve kind of lost momentium for it… I mean they totally screwed up rc1 with making huge breaking changes they knew they were making
across every library
guess they gotta do what they gotta do
in the end all I care about is that it’s awesome
wish it was out by now
Blake Niemyjski
@niemyjski
Feb 22 2016 20:38
they could of released the core bits and keep on releasing it at a quick pace
Blake Niemyjski
@niemyjski
Feb 22 2016 22:05
I hope microsoft is logging these blue screens
second this week
Jamie Clarke
@jamie94bc
Feb 22 2016 22:18
reckon in a few years time asp.net (non-core) will be like still using web forms?
actually scratch that, people will still be using web forms in a few years time
Eric J. Smith
@ejsmith
Feb 22 2016 22:19
lol yeah, they will be using it for 10 years.
Blake Niemyjski
@niemyjski
Feb 22 2016 22:35
@ejsmith this foundatio logging is getting hit for repos as wel
jesus wonder how much faster we can make stuff by doing these logging changes
just noticed and I think it’s slowing down the tests
Eric J. Smith
@ejsmith
Feb 22 2016 22:41
yes, I’ve seen logging show up in profiles a bunch of times and that’s not good.
Blake Niemyjski
@niemyjski
Feb 22 2016 22:47
exceptionless/Exceptionless#196
Eric J. Smith
@ejsmith
Feb 22 2016 22:50
aspnet/Logging#367
close that issue
got several others going at the same time
Blake Niemyjski
@niemyjski
Feb 22 2016 22:58
closed