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

Document IHostStartup (host lightup feature) #4438

Closed
danroth27 opened this issue Sep 28, 2017 · 16 comments
Closed

Document IHostStartup (host lightup feature) #4438

danroth27 opened this issue Sep 28, 2017 · 16 comments

Comments

@danroth27
Copy link
Member

danroth27 commented Sep 28, 2017

See Add docs for IHostedService #3352

@Rick-Anderson Rick-Anderson mentioned this issue Sep 28, 2017
14 tasks
@Rick-Anderson Rick-Anderson added this to the Sprint 125(09/30/17 - 10/2017) milestone Oct 5, 2017
@Rick-Anderson Rick-Anderson modified the milestones: Sprint 125 (09/30/17 - 10/20/17), Sprint 126 (10/21/17 - 11/10/17) Oct 24, 2017
@guardrex
Copy link
Collaborator

@danroth27 You mean "IHostingStartup," correct?

As David would say, "'IHostStartup' isn't a thing ... you just made that up!" 😄 lol

@danroth27
Copy link
Member Author

Oops, yeah 😄 https://github.com/aspnet/Hosting/blob/dev/src/Microsoft.AspNetCore.Hosting.Abstractions/IHostingStartup.cs

@Rick-Anderson
Copy link
Contributor

@guardrex didn't this get completed?

@guardrex
Copy link
Collaborator

@Rick-Anderson No, it's one of the three remaining topics on the list to be written. I plan to get all three done in the three weeks between 🦃 Day and 🎄 break.

@guardrex
Copy link
Collaborator

guardrex commented Dec 2, 2017

@Rick-Anderson

When you edited the OP, you marked this for IHostedService, but it's supposed to be for IHostingStartup and aspnet/Hosting#1000.

The issue title should be ...

Document IHostingStartup (host lightup feature)

@Tratcher I studied the comments, SampleStartups sample, and reference source, but they don't convey quite enough information on how this feature is going to work along the lines of ...

If we added support for executing IHostingStartup implementations that are within the app then tooling could add this kind of functionality by adding a code file with a class implementation to the project.

... about what I had in my original sample, but you said, Gonnae no dae that! ... Gonnae no! (Scottish)

Some comments are almost contradictory: For example when you said "inject code from outside of the application," that's not the same as "adding a code file with a class implementation to the project." One conversation between you and David indicates that only the primary assembly is scanned for the HostingStartupAttribute; but if that's true, then how can it be that this is a light-up feature that works via merely having a bin-deployed assembly and a hosting assemblies env var setting that doesn't require an app redeployment?

I could do it all in the app's assembly (but you said 'no' to my original sample on that), and I could do it as a class lib (but David said 'no' to that), but I can't do it as a bin-deployed assembly that injects code on a light-up basis.

Someone on the engineering side can setup the sample, and I can take the topic forward from there. Otherwise, you'll have to explain the gap out so that I can build it. SampleStartups sample and the remarks thus far don't provide enough information on their own.

Anywho! It's Friday night! 😄 ... so let's pick back up next week. Have a great weekend!

@Tratcher
Copy link
Member

Tratcher commented Dec 2, 2017

@guardrex
Copy link
Collaborator

guardrex commented Dec 2, 2017

Perfect ... thx. I was trying to get it working via setting the hosting startup assembly key, so I'll just need to go with ASPNETCORE_HOSTINGSTARTUPASSEMBLIES, and it should work.

@guardrex
Copy link
Collaborator

guardrex commented Dec 4, 2017

@Tratcher Brainstorming something useful for this feature as a sample ...

How about diagnostics info?

The guts are here 👉 https://github.com/guardrex/HostingStartupSample/blob/master/sample/StartupDiagnostics/StartupDiagnostics.cs

Questions:

  1. I show a few things here, but it feels short on "injecting code" into the app. Do you think this is enough or is there something nice that could interact a bit more directly with the app's code that would be good? So far, it does this ...
    • Places the service list into the container
    • Registers an IStartupFilter for two middlewares:
      • A terminal middleware delegate to show the services
      • A middleware to show the rest of the diagnostic info
  2. Is there a better way to get the service collection into the middleware for display than what I did?
  3. RE: My use of HttpRequestExtensions.IsLocal: Are these diagnostic middlewares going to run before IISIntegation middleware? I'm concerned that RemoteIpAddress and LocalIpAddress aren't set at the point I'm using them. If not, then my attempt to secure these endpoints this way will probably fail.
  4. I'm placing the StartupDiagnostics.deps.json at %ProgramFiles%\dotnet\additionalDeps\StartupDiagnostics\shared\netcoreapp2.0\Microsoft.NETCore.App\2.0.0\, but I must add the file name to the path for DOTNET_ADDITIONAL_DEPS to make it work ... is that correct? I was under the impression that just pointing the env var to the folder would be enough: %ProgramFiles%\dotnet\additionalDeps\StartupDiagnostics ... but it's not working out that way.
  5. I'm having the StartupDiagnostics DLL shot directly into the app's bin folder, but I don't want to do that in the topic. Is it best to place this DLL into the runtime store? ... or someplace else? I'll use the proper directory structure for the store, of course; but is there anything special I need to do to get the assembly picked up? Is it supposed to find it if I leave ASPNETCORE_HOSTINGSTARTUPASSEMBLIES set to StartupDiagnostics?
  6. I don't need to also place the symbols file along with the DLL, correct?
  7. RE: the StartupDiagnostics.deps.json file: I'm manually changing runtime from StartupDiagnostics.dll to lib/netcoreapp2.0/StartupDiagnostics.dll after compiling and before moving to the additionalDeps folder. Is there a way to make it do that ✨ magically ✨ somehow (just to save the manual labor)?

@Tratcher
Copy link
Member

Tratcher commented Dec 4, 2017

  1. If you consider the implementations thus far you'll see that we do tend to be light on the injection. Direct interaction with the app code is not required.
    https://github.com/aspnet/AzureIntegration/blob/f9c4650521553c1ce01ae255afb20713cf7c0883/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/ApplicationInsightsStartupLoader.cs
    https://github.com/aspnet/AzureIntegration/blob/f9c4650521553c1ce01ae255afb20713cf7c0883/src/Microsoft.AspNetCore.AzureAppServices.HostingStartup/AzureAppServicesHostingStartup.cs
    https://github.com/aspnet/AzureIntegration/blob/f9c4650521553c1ce01ae255afb20713cf7c0883/src/Microsoft.AspNetCore.AzureAppServices.HostingStartup/AzureKeyVaultHostingStartup.cs

  2. That makes sense. @pakrym can check if there's another way to enumerate services.

  3. Never use IsLocal for a security check for exactly this reason. Use IsDevelopment instead.

  4. That deps location looks correct. @pakrym can you also check on the file contents?

  5. The dll must be placed in a default loadable location. E.g. the runtime store or bin directory.

  6. No symbols required.

  7. @pakrym ?

@guardrex
Copy link
Collaborator

guardrex commented Dec 4, 2017

Thanks @Tratcher

@pakrym ... RE: 4 ... The .dep.json file name must be on the path value set into DOTNET_ADDITIONAL_DEPS ? ... It doesn't seem to work if I merely set the env var to the directory at %ProgramFiles%\dotnet\additionalDeps\StartupDiagnostics.

@pakrym
Copy link
Contributor

pakrym commented Dec 4, 2017


// To be able to build as <OutputType>Exe</OutputType>
internal class Program { public static void Main() { } }

can be removed, we don't build it this way anymore.

  1. You can drop entire IServiceCollection into container, or wrap it into another class, no need to wrap every descriptor, just make a comment that this is for display purposes only.

4 and 7. As for .deps.json file. To build correct one you would need to have a wrapper project that is referencing hosting startup package and targeting netcoreapp2.0 (https://github.com/aspnet/Universe/tree/dev/build/tools/templates/HostingStartup). It has to be build and then first entry trimmed (https://github.com/aspnet/Universe/blob/4a752d96caf1ff99fbb934e32836d5c6ba570fdf/build/tasks/TrimDeps.cs#L13)

Also see https://github.com/aspnet/Universe/blob/4a752d96caf1ff99fbb934e32836d5c6ba570fdf/build/RuntimeStore.targets#L145

There is a talk about packing all this tooling into a semi-supported package to be used by other team, but it's far from decided.

@pakrym
Copy link
Contributor

pakrym commented Dec 4, 2017

deps.json has to be in shared\Microsoft.NETCore.App\2.0.0-preview3-25507-02\Microsoft.AspNetCore.AzureAppServices.HostingStartup.deps.json where 2.0.0-preview3-25507-02 is the version of shared runtime that your app requested (the one in runtimeconfig) and you set DOTNET_ADDITIONAL_DEPS to a directory containing this entire structure.

So for c:\Users\pakrymet\.dotnet\x64\additionalDeps\Microsoft.AspNetCore.AzureAppServices.HostingStartup\shared\Microsoft.NETCore.App\2.0.0-preview3-25507-02\Microsoft.AspNetCore.AzureAppServices.HostingStartup.deps.json

DOTNET_ADDITIONAL_DEPS = c:\Users\pakrymet\.dotnet\x64\additionalDeps\Microsoft.AspNetCore.AzureAppServices.HostingStartup

@guardrex
Copy link
Collaborator

guardrex commented Dec 4, 2017

@pakrym I was thinking of using the dotnet install folders for this (C:\Program Files\dotnet), since it's a tool that perhaps should be available to all users. However, I already noticed the pain point that admin privileges are required to set this up in the dotnet install folders, which I don't like anyway. So, you're saying set it up in the user profile. I'll give that a shot.

Oh ... and RE: dropping the entire service collection into the container ...

make a comment that this is for display purposes only

I want to make this a sane tool that could actually be used. If how I'm handling the descriptors is a sane way to shuttle the list of services to the middleware, then I'd rather stick with that over having to say "here's a useful tool example ... but don't use it."

@pakrym
Copy link
Contributor

pakrym commented Dec 4, 2017

@guardrex use AddSingleton with factory instead of Configure.

You don't have to put it into C:\Program Files\dotnet but you need to have entire directory structure existing inside your DOTNET_ADDITIONAL_DEPS directory.

@guardrex
Copy link
Collaborator

guardrex commented Dec 5, 2017

@pakrym @Tratcher I implemented the following:

  • Factory for obtaining the service collection for the middleware.
  • IsDevelopment rather than trying to figure out IsLocal; however, I'm a bit 😢 about that, since this could be a production tool, too, if the endpoints could be secured better. I think for now and for our purposes, this is fine as a Development-only tool.
  • Moved the assembly and symbols to the runtime store in the user's profile.
  • Moved the deps.json file to the additionalDeps in the user's profile.

Wrap the tool in another app, build, then trim, and move ... ei ei ei ... Geebers McScreebers! 😵 lol.

If we go that way, we'd be getting wildly afield of the tool explaining all of that out. Also, trying to automate that for the sample is a bit much. It would be vastly simpler imo to do the following in an AfterTargets="Build" for the tool, which is what I have the latest version of the sample:

  1. Create a directory structure for assembly in the runtime store of the user's profile.
  2. Move the IHostingStartup implementation assembly.
  3. Create a directory structure for the symbols in the runtime store of the user's profile.
  4. Move the IHostingStartup implementation symbols.
  5. Run a PS script that mods the deps.json file.
  6. Create a directory structure for the deps.json file in the additionalDeps folder of the user's profile.
  7. Move the deps.json file.

Because that's all automated in the target, all the dev has to do with the sample is build it. 🎉 ... well ... they need PS installed, and there might be some additional PS-on-Linux/PS-on-Mac issues to address.

https://github.com/guardrex/HostingStartupSample/tree/master/sample

@guardrex
Copy link
Collaborator

guardrex commented Dec 7, 2017

@Rick-Anderson The key issue is that to make consumption of the sample as pain-free as possible, I use a build target in the IHostingStartup project to automate the process of placing the assembly, symbols, and deps.json files in the right locations with a PS step that mods the deps.json file. The dev just needs to:

  1. Have PS installed (I'll call it out with links if we go this way with the topic).
  2. Build the IHostingStartup project.
  3. Set the env vars.
  4. Run the sample app.

I tested the PS script on Mac Mini, and it works. One prob tho: I can't run the app on the Mac because it's a 2009 unit, so it can't take Sierra, thus it can't do 2.0. All I could test is that the PS script commands run properly. I think it will be ok, but I can't guarantee it with this ancient Mac Mini.

Shall I proceed to write this up with this approach?

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

6 participants