Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Fix EventLogger in .NET 4.5.2 #467

Merged
merged 1 commit into from
Jul 28, 2016
Merged

Fix EventLogger in .NET 4.5.2 #467

merged 1 commit into from
Jul 28, 2016

Conversation

BrennanConroy
Copy link
Member

@BrennanConroy BrennanConroy commented Jul 21, 2016

The behavior of OnEventCommand callback changed between .NET 4.5.2 and .NET 4.6. In the latter the internal EventSource level has changed before calling the OnEventCommand callback which was behavior we relied on to set the loggers level. Because it isn't set before the callback in .NET 4.5.2 we delay the original behavior of the callback until a time after the Event change occurs.
#328

@BrennanConroy BrennanConroy changed the title [WIP] Fix EventLogger in 452 Fix EventLogger in .NET 4.5.2 Jul 22, 2016
@BrennanConroy
Copy link
Member Author

@karolz-ms @kichalla

@karolz-ms
Copy link
Contributor

Looks good, but I would add a comment explaining why SetFilterSpec is not immediately called on logging providers when EventSource.SetFilterSpec is called

@BrennanConroy
Copy link
Member Author

Good point

@davidfowl
Copy link
Member

Some explanation would be good in the PR description, what was wrong and what was fixed

@BrennanConroy
Copy link
Member Author

@davidfowl Added some description

@BrennanConroy
Copy link
Member Author

🆙 📅

@BrennanConroy
Copy link
Member Author

Ping

@davidfowl
Copy link
Member

:shipit:

}

[NonEvent]
internal void VerifyLevel()
Copy link
Member

Choose a reason for hiding this comment

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

Any concurrency concerns? Can this be called in parallel?

@BrennanConroy
Copy link
Member Author

🆙 📅
Added lock and added another location to check level

@@ -37,6 +37,7 @@ public EventSourceLoggerProvider(LoggingEventSource eventSource, EventSourceLogg
/// </summary>
public ILogger CreateLogger(string categoryName)
{
_eventSource.VerifyLevel();
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment wherever you call VerifyLevel() (which is the wrong name)

@BrennanConroy BrennanConroy merged commit 62265c0 into dev Jul 28, 2016
@BrennanConroy BrennanConroy deleted the brecon/event_log branch July 28, 2016 23:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants