Adding EventSource to NetFx

[JRahnama]

This PR removes references to Bid class and replaces them with references to SqlClientEventSource.

[Wraith2]

What consumes the messages that the BID tracing produced? There's a lot of information embedded in the strings and i'm not sure if it needs to remain in that format or if it could be split out as event properties so you can use them for filtering or grouping in consuming tools like perfview.

I'm not a big fan of the field access for the event source instance SqlClientEventSource._log.Trace feels like it should just be SqlClientEventSource.Trace and the method body should resolve the state parameter. I know a lot of the existing code works with internal variables in this way already but i've found that not having clean interfaces makes refactoring and understanding this library difficult at times because you can have instances reaching into each other's internals to do things you wouldn't normally expect.

[Wraith2]

I had a read through https://github.com/Microsoft/dotnet-samples/blob/master/Microsoft.Diagnostics.Tracing/EventSource/docs/EventSource.md earlier and I think the new IfTaceEnabled method will need a [NoEvent] adding to it. Other than that the pattern looks better. The other trace like functions can be dealt with in the same way, by separating the condition check and real call out. And can _log be Log? in the doc it shows it as being publicly exposed and use directly so even though I think it'd look better if it were hidden the requirement for using instance methods means it can't be used that way.

[JRahnama]

@Wraith2 the IfTraceEnabled is marked as [NonEvent]. Initially, I started the the name with Log and VS got it as naming violation. _log/Log is an internal static read only instance of SqlClientEventSource. There are other versions of EventSource in NetFx. I got a bit confused when you mentioned doc. Is it Microsoft Docs or Driver documentation? There should not be any documentation about SqlClientEventSource as it is all internal.

[Wraith2]

@Wraith2 the IfTraceEnabled is marked as [NonEvent]

Excellent, missed that then 😁

I started the the name with Log and VS got it as naming violation

Yeah, and it is. But it's a guideline and not a requirement and code is for humans to read so if the computer doesn't like it we can shut it up but the pattern of calling SqlClientEventSource._log.Trace just doesn't feel right to me. Just a suggestion though.

The doc I linked is the design/usage documentation from the original implementation. It was made for corefx and backported to netfx so the guidance there is first party and quite useful I found.

[David-Engel]

I haven't gotten all the way through yet, but wanted to post what I've done so far. In general, it looks good.

I generally agree with Wraith2 on naming it Log instead of _log for readability. I think we can suppress the naming violation.

As Wraith2 also mentioned, You need to surround _log.* calls with an if (_log.IsTraceEnabled()) block everywhere you are passing in a string that will be formatted before it is passed into the trace function to avoid the performance penalty of formatting and allocating a new string during the function call.

[saurabh500]

Approved with Suggestions.

[cheenamalhotra]

Need more changes.