Skip to content
This repository has been archived by the owner on Dec 14, 2017. It is now read-only.

Update Logging.md #220

Open
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from
Open

Update Logging.md #220

wants to merge 1 commit into from

Conversation

richardterris
Copy link

Additional info for enabling logging when writing unit tests

Additional info for enabling logging when writing unit tests
In order to make logging work you will have to wrap the code which makes requests to IdentityServer in a using statment such as:
```csharp
using (var logger = new LoggerConfiguration()
.MinimumLevel.Debug()
Copy link
Member

Choose a reason for hiding this comment

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

For the Serilog logger, don't you need to assign it to the static Log instance? I can't tell from this how IdSvr knows about your logger variable.

Copy link
Author

Choose a reason for hiding this comment

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

This is very strange... Last night when I got this working, it produced a good amount of logs and the issue seemed to be that the certificate installed had a key length of 1024 bytes.

Now when I run this, I am getting a log file, but only with the line I add:
logger.Debug("test!");

To try and figure this out, I have commented out the logging setup code in my Startup class and recompiled the project, and the logger creation certainly seems to work with the code in this PR - I assumed what I was doing above was creating an in-memory logger which would live for the duration of my unit test only? Or am I misunderstanding something?

I'm really interested in contributing to this because it's a great project and I'm trying to become more skilled in security in general, but obviously I'm quite at the beginning with this.

I'll keep playing just now to see if I can figure this out and I'll update the PR when I do.
But of course, if I'm doing something wrong please tell me.

Richard

Copy link
Member

Choose a reason for hiding this comment

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

Well, I don't know too much of how Serilog works internally, but the way IdSvr "knows" about it is that it discovers it via the static Log property that serilog has. So for your unit tests, I'd expect this static property to be set somewhere at initialization time (maybe even statically, as opposed to per-test).

@richardterris
Copy link
Author

This is possibly becoming the wrong place for this, but....
I have now gone as far as set up OWIN TestServer within my unit test project to try and get an in-memory OWIN pipeline. Within the Startup I am creating the SeriLog logger and trying to tie that to the IdentityServer Logging, but I can't see how to do that because I don't see any such static Log property.

I do see 3 methods:
public static ILog GetCurrentClassLogger();
public static ILog GetLogger(Type type);
public static ILog GetLogger(string name);

I have tried ` var logger = new LoggerConfiguration()
.MinimumLevel.Debug()
.WriteTo.File(@"C:\idsLogs.txt")
.CreateLogger();

        LoggingOptions options = new LoggingOptions();
        options.EnableHttpLogging = true;

        var idserverLogger = LogProvider.GetLogger(logger.GetType().Name);`

and also var idserverLogger = LogProvider.GetLogger(logger.GetType());

neither of these seem to work.

If you can help me figure out how to get IdentityServer to hook up the SeriLog then I may be able to contribute this code to the repo, as people may want to fire up a unit test project like this.

Again, may be now becoming the wrong place to post this - should I open a new issue?

In any case I'm still unable to get to the root of my actual problem (as much as this is fun)

@leastprivilege
Copy link
Member

Quoting our docs

Liblog picks up any of the following logging libraries automatically. There is no IdentityServer3 specific configuration required - you just need to configure one of the above logging frameworks in your host.

And one of our samples:
https://github.com/IdentityServer/IdentityServer3.Samples/blob/master/source/WebHost%20(minimal)/WebHost/Startup.cs#L16

That's all that is needed. Are we making it too easy? ;)

@richardterris
Copy link
Author

@leastprivilege It would seem so, for me at least :D

But jokes aside - what you're suggesting IS really easy but it's not working for me.
The code will create the log file but nothing is getting written to it; it should be because my unit test is returning an internal server error.

I can only assume it's because it's a unit test so I thought I could set up an in-memory OWIN server and wire up the logger manually to Serilog and that would solve it, but I can't seem to get the logger wired up.

Every chance it's me missing something, because I'm open minded about my capacity for stupidity :D

@leastprivilege
Copy link
Member

@damianh Anything special you need to do in liblog for unit tests?

@leastprivilege
Copy link
Member

So can you guarantee that the code that sets up the static serilog logger gets executed in the context of the test?

Which test framework are you using?

Also - have you ever tried to get logging working outside of unit tests? And once that is working - transfer that to you unit test scenario?

@richardterris
Copy link
Author

No, in the unit test the static logger doesn't get created, only when I add that code to the unit test.
I would have thought the startup class would still be hit when the unit test calls tokenClient.RequestClientCredentialsAsync but I guess not.

I'm using NUnit for testing.
I haven't as yet tried to do something outside of the unit tests but I can do later when I finish work for the day.

It does seem like setting up OWIN test server in the unit test project should solve this issue though? When doing so the code is fired because the log file is created, but it seems IdentityServer doesn't wire up the logger to Serilog

@leastprivilege
Copy link
Member

Also the owin test host must be created per test. That's the way it works.

Assume every test runs in a fresh environment. I think that's the cause of all your problems.

Sent from my iPad

On 15 Aug 2016, at 12:02, Richard Terris [email protected] wrote:

No, in the unit test the static logger doesn't get created, only when I add that code to the unit test.
I would have thought the startup class would still be hit when the unit test calls tokenClient.RequestClientCredentialsAsync but I guess not.

I'm using NUnit for testing.
I haven't as yet tried to do something outside of the unit tests but I can do later when I finish work for the day.

It does seem like setting up OWIN test server in the unit test project should solve this issue though? When doing so the code is fired because the log file is created, but it seems IdentityServer doesn't wire up the logger to Serilog


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@richardterris
Copy link
Author

richardterris commented Aug 15, 2016

I have no idea why this editor is modifying my code but the TestServer.Create also specifies OwinTestConfiguration which is what sets up my middleware where I am creating the logger
The code in my unit test is:

`using (var server = TestServer.Create())
{
var tokenResponse = tokenClient.RequestClientCredentialsAsync("applicationLogin").Result;

            if (tokenResponse.IsError)
            {
                Console.Write(tokenResponse.Error);
            }

            Console.Write("Access token: " + TokenHelper.DecodeAndWrite(tokenResponse.AccessToken));

}`

Which is creating a test server only for the lifetime of that test, and this doesn't work

@damianh
Copy link

damianh commented Aug 27, 2016

@leastprivilege Sorry for slow reply mate...

@damianh Anything special you need to do in liblog for unit tests?

Yes if you are using xunit2 because of ITestoutputHelper. http://dhickey.ie/2015/06/capturing-log-output-in-tests-with-xunit2/

I don't know if nunit has a similar concern (parallel testing) or not?

@richardterris which version of serilog are you using?

@richardterris
Copy link
Author

Hi @damianh thanks for the response.
From what I've read, it doesn't look like NUnit supports parallelisation, but I don't think it should be an issue here because I just have (at the moment) a single unit test and I can't get logging wired up.

Serliog version 2.0.0 is what I'm using

Thanks again!

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