Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sublogger not working in web host #156

Closed
gautelo opened this issue Dec 10, 2018 · 6 comments
Closed

Sublogger not working in web host #156

gautelo opened this issue Dec 10, 2018 · 6 comments
Labels

Comments

@gautelo
Copy link

gautelo commented Dec 10, 2018

When using a web host subloggers don't seem to work.

I've created a very stripped down version of what we're doing and uploaded that as a reference repo in the hopes that you might point out what I'm doing wrong, or figure out why it's failing.

As you can see I have a web host and a console host. They both use the same logging configuration, yet the sublogger only work for the console host. The interresting bit is that the straight file logger works for both hosts, so it's not that serilog is not working at all, it's just that the subloggers aren't working.

I've also included an alternate configuration option for configuring through code instead of through settings. Swap between SerilogConfig.RegisterBySettings and SerilogConfig.RegisterByCode to see how using SerilogConfig.RegisterByCode causes the sublogger to work in both hosts.

Note: We're in a bit of a crossover between older and newer tech which might be a reason why this does not work, though I don't see why it would cause an issue seing as it works for SerilogConfig.RegisterByCode.

@tsimbalar
Copy link
Member

Thanks a million for the repro ! You've obviously spent some time preparing it, so it feels only fair to spend some time reviewing it ;)

I don't have the proper environment right now to actually run it, but here are my 2 cents :)

Swap between SerilogConfig.RegisterBySettings and SerilogConfig.RegisterByCode to see how using SerilogConfig.RegisterByCode causes the sublogger to work in both hosts.

this definitely makes me think there is something fishy about the configuration file somehow, so I'll focus on that for now :) ...

I see your json config file for the "sub-logger" looks like this :

{
  "Serilog": {
    "MinimumLevel": "Information",
    "Enrich": [
      "WithThreadId"
    ],
    "Using": {
      "File": "Serilog.Sinks.File"
    },
    "WriteTo": {
      "MainFile": {
        "Name": "Logger",
        "Args:configureLogger:WriteTo:File": {
          "Name": "File",
          "Args": {
            "path": "%SERILOG_OUTPUT_PATH%\\ConfigSub-.log",
            "outputTemplate": "{Timestamp:HH:mm:ss.fff} {Level:u3} {ThreadId,3} {SourceContext} :> {Message}{NewLine}{Exception}",
            "shared": true,
            "flushToDiskInterval": "00:00:03",
            "rollingInterval": "Day",
            "retainedFileCountLimit": 31
          }
        }
      }
    }
  }
}

and 2 things caught my attention :

  1. your using directive ... It should actually be an array of strings and not an object/map : "Using": ["Serilog.Sinks.File"] ... so it may fail to load the assembly from the Nuget package.
    The Serilog.Settings.Configuration codebase does have a few tricks to try and find assemblies even though there are no explicit usings , and they might behave differently in the 2 sorts of hosts. (they have been mostly tested with .NET core Web and console apps ... not so much with full framework, so those scenarios may be not so well covered).
    Could you try changing the format of the using directive, to see if it improves anything ?

  2. I have doubts about the "Args:configureLogger:WriteTo:File" key ... I would have expected the following :

{
  "Serilog": {
    "MinimumLevel": "Information",
    "Enrich": [
      "WithThreadId"
    ],
    "Using": {
      "File": "Serilog.Sinks.File"
    },
    "WriteTo:MainFile": {
      "Name": "Logger",
      "Args": {
        "configureLogger": {
          "WriteTo": {
            "Name": "File",
            "Args": {
              "path": "%SERILOG_OUTPUT_PATH%\\ConfigSub-.log",
              "outputTemplate": "{Timestamp:HH:mm:ss.fff} {Level:u3} {ThreadId,3} {SourceContext} :> {Message}{NewLine}{Exception}",
              "shared": true,
              "flushToDiskInterval": "00:00:03",
              "rollingInterval": "Day",
              "retainedFileCountLimit": 31
            }
          }
        }
      }
    }
  }
}

not sure it makes a difference, but well ... probably worth a try, to see if it makes a difference ! (I use https://github.com/tsimbalar/serilog-settings-comparison/blob/master/docs/README.md#sub-loggers--child-loggers as reference for working configuration examples)

Also while I'm at it, I see that you don't have a Log.CloseAndFlush() call in either the web host or the console host ... You may want to add them to force all the events to be flushed out and written to file.

I hope that helps !

@gautelo
Copy link
Author

gautelo commented Dec 11, 2018

@tsimbalar Thanks for taking the time to look at this.

You mention that since it's working with SerilogConfig.RegisterByCode it seems likely to be an error with the configuration. That's why I included the console application. It uses the same config and produces output for the sublogger which indicates that the configuration is correct.

Regardless, i pushed some changes to the reference repo with the changes you propose. Could you have another look?

PS: The reason why I've been using object-maps instead of arrays is because for my actual application we do some merging of json which doesn't play so nice with arrays. Since Microsoft.Extensions.Configuration seem to iterate the object-map just fine, that's a godsend and a really good feature (accidental or not).

PPS: You mention that you doubt my "Args:configureLogger:WriteTo:File" shorthand, yet the documentation you refer to uses the same trick and object-map instead of array for its WriteTo: "WriteTo:SubLogger1". ;)

@skomis-mm skomis-mm added the bug label Dec 13, 2018
@skomis-mm
Copy link
Contributor

The problem in this one . It should also probe {BaseDirectory}\bin directory (to get Serilog assembly which contains WriteTo.Logger(...)).
@gautelo as a workaround, add Serilog to Using list.

@gautelo
Copy link
Author

gautelo commented Dec 13, 2018

Wow, yep indeed. Tested with a "Serilog" using and that fixed everything. Thank you ever so much @skomis-mm! :)

I suppose what happens is that since AppDomain.CurrentDomain.BaseDirectory is the web folder for a web host it won't find the dll like it does for my console app where BaseDirectory is the bin folder containing Serilog.dll.

I'll close this since it seems you're dealing with it in #151.

@tsimbalar
Copy link
Member

Just for cross-reference, this looks similar to #122

skomis-mm pushed a commit that referenced this issue Jan 7, 2019
DllScanningAssemblyFinder fixes (#157, #150, #122, #156)
@skomis-mm
Copy link
Contributor

skomis-mm commented Jan 7, 2019

Hi, @gautelo . If you have a chance let check this with updated pre-release package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants