Do not ignore messages we've been explicitly asked to log

[CyrusNajmabadi]

No description provided.

[jasonmalinowski]

@CyrusNajmabadi @arunchndr How much testing have we done for this change? IIRC we have some places we log ETW events via the logging thing that are very chatty, and this would be a huge flood to the telemetry systems. The design here was places log, and we might produce different events in different places that are appropriate for different systems.

That all said, if we want to say only use the logging methods if it's appropriate for VS Telemetry and delete a bunch of stuff here, I'm OK with that. But I imagine merging this as is might be problematic unless you've tested more.

[CyrusNajmabadi]

This method currently serves no purpose. There is no way to configure it. If you report calling the, it is simply sent to the ether. Despite the clients explicitly adding this telemetry exactly to get this data.

and this would be a huge flood to the telemetry systems

We have asked telemetry to give us a system to log that isn't problematic. At this point, we need to be able to answer these questions. And if that's an issue for this subsystem, it has to be improved.

[ToddGrun]

and this would be a huge flood to the telemetry systems

My understanding is the same as Jason's, that telemetry has this filter to avoid flooding the telemetry system. Other loggers can handle that increased payload.

[CyrusNajmabadi]

So we literally have an API, that if you call, nothing happens. And that's by design?

[ToddGrun]

And that's by design?

I found (and still find) the design of the logging system surprisingly complicated. In the end, telemetry takes part in the logging system, but it also exposes itself to callers that just wish to do telemetry through the TelemetryLogging class (not TelemetryLogger). If telemetry is invoked through TelemetryLogger, the intent is that users fall into a pit of success, whereas changing telemetry through ILogger seemed too scary when I looked at this area when I first joined the team.

[CyrusNajmabadi]

the intent is that users fall into a pit of success

That this API swallows the messages unilaterally, and behaves that way by default, with default args, with no docs saying anything like that, is opposite of pit of success to me :-)

[ToddGrun]

this API

By this API, you mean TelemetryLogger's ILogger implementation (not TelemetryLogging), right? Yes, I agree, the ILogger API is intended for general logging, and callers will not get telemetry through it unless they send in a specially crafted message indicating they wish to.

[CyrusNajmabadi]

and callers will not get telemetry

No. I mean this API literally swallows all these messages, regardless of telemetry or not. It literally just says: the level was lower than this fixed (non configurable) value? Dropped

If you call this API with default values, it does nothing. That's broken.