Adding support for source-link

[gled4er]

Hello @cgillum,

May I ask you to take a look on this first commit for adding support for source-link as described in #526?

Thank you!

[cgillum]

Is this the only change that is required? Would you be able to somehow demonstrate that it is working as expected, maybe with screenshots? I'm not actually familiar with how to set it up, but I assumed there was more to it.

[gled4er]

Hello @cgillum,

Thank you for the comment.

Great idea about the testing!

I will continue to work on it and update you about the progress.

Thank you!

[gled4er]

Hello @cgillum,

I was able to achieve the expected result with integrating source-link.

Please check the following images:

May I ask you to review the changes and let me know what should be fixed next?

Thank you!

[cgillum]

Awesome! My only question is what happens when we target the full .NET Framework and Functions 1.0? The documentation I've seen around source-link and embedded PDBs is not clear about whether these are just .NET Core features or if they also work with .NET Framework.

I just want to make sure we don't regress any experiences for people who are still using .NET Framework and Functions 1.0.

[cgillum]

Just to clarify further, it's okay if the source-link stuff doesn't work for .NET Framework, but I just want to make sure we don't regress any debugging experiences for people who are still using .NET Framework and Functions 1.0. Does that make sense?

[gled4er]

Hello @cgillum,

Thank you for the great feedback!

Yes, it makes perfect sense.

The best way to test it against .Net Framework is to modify the host.json or you suggest other option?

Thank you!

[cgillum]

I think all you need to do is create a function app which targets Functions 1.0, clear your nuget cache, and make sure that customers can still manually set breakpoints and break into Durable Functions code. If source-link works, that's great to, but not required.

I think targeting 1.0 is as simple as updating the .csproj file (you may also need to make updates to host.json). When you reference your local build of the nuget package, it will target the right framework automatically.

[cgillum]

According to this Twitter thread, it might be expected to work on all versions of .NET: https://twitter.com/kirillosenkov/status/1007052524946255872?lang=en

I'll try to test it as well.

[gled4er]

Great!

Thank you!

I will try it soon.

Thank you!

[gled4er]

Hello @cgillum,

I was able to test and verify that source-link works for .NET Framework as well as you can see from the images below:

In the last commit you will see a small change in DurableTaskExtensions.Initialize method. The problems is that this.nameResolver is null when we execute against .Net Framework and the host is not able to start. The regression was introduced from the change we made for setting TaskHub name from the local.settings.json in https://github.com/Azure/azure-functions-durable-extension/pull/503/files#diff-8e71038a95b35db07c2b23ecbc95b728R150 And the fix I added is:

            if (this.nameResolver != null &&
                this.nameResolver.TryResolveWholeString(this.Options.HubName, out string taskHubName))
            {
                // use the resolved task hub name
                this.Options.HubName = taskHubName;
            }

I was able to detect the error partially thanks to the new source-link feature.

Let me know if you prefer to make the correction in the way presented above or you prefer to have

#if NETSTANDARD2_0
           if (this.nameResolver.TryResolveWholeString(this.Options.HubName, out string taskHubName))
            {
                // use the resolved task hub name
                this.Options.HubName = taskHubName;
            }
#endif

If my understanding is correct, we should try to include this PR in the upcoming 1.7.0 release or make the fix in another PR to prevent the regression for the .Net users.

Thank you!

[cgillum]

Thanks Kanio for making that additional fix. It's good that we did some manual testing to detect this regression.

[gled4er]

Hello @cgillum,

Thank you for the quick response that allowed us add the correct fix immediately!

Thank you!