ExcelDna icon indicating copy to clipboard operation
ExcelDna copied to clipboard

Allow developers to remove the `LogDisplay` trace listener

Open augustoproiete opened this issue 4 years ago • 2 comments

Currently there's no way to remove the LogDisplayTraceListener from the ExcelDna.Integration trace source. My expectation is that the code below would remove the (automatically added) LogDisplay listener, the same way that it removes the Default listener, but in practice it has no effect:

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
  <system.diagnostics>
    <sources>
      <source name="ExcelDna.Integration" switchValue="All">
        <listeners>
          <remove name="Default" />
          <remove name="LogDisplay" />

          <!-- Other listeners... -->
        </listeners>
      </source>
    </sources>
  </system.diagnostics>
</configuration>

Start of a File-New Project add-in:

image


Workaround

The current workaround is to add the LogDisplay listener to the configuration, and switch it off (initializeData="Off"), which is not the same as removing it:

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
  <system.diagnostics>
    <sources>
      <source name="ExcelDna.Integration" switchValue="All">
        <listeners>
          <remove name="Default" />

          <add name="LogDisplay"
               type="ExcelDna.Logging.LogDisplayTraceListener, ExcelDna.Integration">
            <filter type="System.Diagnostics.EventTypeFilter" initializeData="Off" />
          </add>

          <!-- Other listeners... -->
        </listeners>
      </source>
    </sources>
  </system.diagnostics>
</configuration>

Actually, I'd expect the code above to throw an exception, given that a listener with the name LogDisplay already exists (by default) so unless it's removed first, we should get an error.

This is my expectation of the correct code to override the default LogDisplay listener:

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
  <system.diagnostics>
    <sources>
      <source name="ExcelDna.Integration" switchValue="All">
        <listeners>
          <remove name="Default" />
          <remove name="LogDisplay" />

          <add name="LogDisplay"
               type="ExcelDna.Logging.LogDisplayTraceListener, ExcelDna.Integration">
            <filter type="System.Diagnostics.EventTypeFilter" initializeData="All" />
          </add>

          <!-- Other listeners... -->
        </listeners>
      </source>
    </sources>
  </system.diagnostics>
</configuration>

Related documentation

augustoproiete avatar Nov 06 '19 20:11 augustoproiete

It looks like I couldn't find the right (early enough) place to put the default LogDisplayTraceListener in the Listeners list (i.e. before the configuration file's add/remove instructions are applied). When there is no configuration at all I wanted to add the LogDisplay listener, and then the only way I could find to disable it as much as possible was to allow for an explicit configuration override to replace the default one.

Would it help to process this override in a special way, so that if the LogDisplayTraceListener is configured as "Off" we completely remove it from the Listeners collection in the code? It would leave the configuration file the same (so no <remove name="LogDisplay"/>) but at least allow you to get it out of the way completely.

The relevant code is here: https://github.com/Excel-DNA/ExcelDna/blob/30738f187d299d375c1db1a1f3ac867fdebb621f/Source/ExcelDna.Integration/Logging.cs#L79-L124

govert avatar Nov 06 '19 21:11 govert

It looks like I couldn't find the right (early enough) place to put the default LogDisplayTraceListener in the Listeners list

Yeah, this seems like a tricky one. The Default listener is added during static initialization with no obvious way to intercept (and add more), and the classes that model the configuration section are all internal :/.

Would it help to process this override in a special way, so that if the LogDisplayTraceListener is configured as "Off" we completely remove it from the Listeners collection in the code?

Yes, that would make it better than it is today.


In the absence of being able to remove via configuration, for Excel-DNA v2.x I'd suggest we make this opt-in instead...

augustoproiete avatar Nov 08 '19 03:11 augustoproiete