Individual packaging doesn't work for dotnet projects

[BryanCrotaz]

Use case description

individual packaging zips up the entire project folder. In .NET case it should only zip the bin folder for the appropriate package

Proposed solution

(Updated on the go, as agreement on final solution settles)

1. Refactor core to allow to inject a dedicated packaging logic for any provider/runtime case (and treat currently implemented solution as a default fallback), and keep current packaging logic as default when no provider/runtinespecific packaging logic is provided
2. Implement dedicated packaging solution for AWS/.NET functions (possibly as external plugin, but that's TBD)

[medikoo]

@BryanCrotaz thanks for report. Can you clarify a .NET project (I know what's a .NET, it's just not clear to me, what would be a .NET project in context of Serverless framework?)

Is it on AWS provider? Does it involve any plugins?

[BryanCrotaz]

It's in your guides
https://serverless.com/framework/docs/providers/aws/examples/hello-world/csharp/

So a Lambda using .Net code as the runtime

[medikoo]

@BryanCrotaz thanks for clarification.

Packaging individually means that for each function different includes and excludes rules can be setup, or that you specify generate artifacts for each function.

Otherwise by default whole project is bundled in all cases.

If you want to avoid including whole project you need to specify exclude an include rules

[BryanCrotaz]

For a dotnet project, there’s an extra automated filtering you can do. The entry point tells you which dll it’s in, and the bin folder in that dll sub-project contains all the files you need to bundle. Files with the same name in multiple bin folders will be identical. So you can just merge those bin folders and save 10s of MB on the bundle.

[BryanCrotaz]

If individual bundling is enabled, then merge a subset of the bin folders

[BryanCrotaz]

I don’t mind coding this up if you can point me at the class that would be responsible and how to know it’s a dotnet project. I’ve not contributed to serverless, but I’ve built my own similar framework before serverless got popular so now moving over to it

[medikoo]

Currently packaging logic is configured as not only runtime agnostic but also vendor agnostic, and it's placed in lib/plugins/package/lib/packageService.js
I don't think that we should start adding there any vendor or runtime specific logic, but we can open some doors so such logic can be injected from out side.

We have a lot of issues with current packaging mechanism, there's a dedicated issue where we dicsuss on how to improve that logic: #6580

In this specific case, what may work is to preset some default exclude and include patterns for dotnet lambdas (I take that could be programmed in context of lib/plugins/aws/package/compile/functions/index.js it's already AWS specific, so adding some runtime specific logic won't be harmful)

If that can't be achieved just via patterns, that again it may need some refactor in context of lib/plugins/package/lib/packageService.js to allow more sophisticated handling

[BryanCrotaz]

It would make sense to have a plug-in class that provides the logic. You can see it being completely different for each source language, and the same for 99% of projects using that language.

[BryanCrotaz]

I've realised that we don't need a plugin - the core code already checks for a runtime framework and builds accordingly.

The pre packaging process just needs to handle dotnet differently before zipping. Use the includes list to include a .csproj file for building. The bin directory is self contained so can be zipped without including any other files. Simples.

See #8043 for an implementation that I'm successfully using in production

[medikoo]

@BryanCrotaz can you outline precisely the rules with which you believe .NET projects should be packaged.

From what I understand it should be:

includes:
  - **/*.cproj

Is there anything else? Can you outline for us include and exclude rules.

Thank you!

[BryanCrotaz]

You would put the exact csproj file name in the includes for each function. No excluded or other includes required.

[medikoo]

You would put the exact csproj file name in the includes for each function

Where are those files located? Are they not bundled by default now? If not, do you understand why?

[BryanCrotaz]

The currently bundling just zips the entire src directory. It doesn't run a build process.

[BryanCrotaz]

I do understand why, and I've provided a PR that fixes the problem by introducing the correct build process for dotnet!

[BryanCrotaz]

The files are located wherever the includes entry provided by the developer says they are...

[medikoo]

@BryanCrotaz as I mentioned this PR contains a lot of noise and is not easily readable for me. Can we discuss without given PR being provided as reference?

We see it as good pattern to follow, to first discuss and agree on implementation spec, before moving to writing an actual implementation.

It's generally easier and saves a lot of our time (it's easier to understand spec than written code especially if it doesn't adheres to conventions we've setup and contains unrelated changes)

Is this possible for you to adjust to such workflow?

[BryanCrotaz]

Where are the conventions documented? Happy to refactor to them

[medikoo]

Where are the conventions documented? Happy to refactor to them

https://github.com/serverless/serverless/blob/master/CONTRIBUTING.md#when-you-propose-a-new-feature-or-bug-fix

Line that applies to this case at current stage is:

In non trivial cases please propose and let us review an implementation spec (in corresponding issue) before jumping into implementation.

[BryanCrotaz]

The problem with writing an implementation spec in advance is that it requires deep knowledge of how Serverless is architected. As there isn't even a document of all the events that can be handled and what the responsibility of the handler is, that's virtually impossible for someone that isn't on the core team. The best that can be done is to dive in with a debugger and try and reverse engineer, then find the best way to reach an ease-of-use goal.

[medikoo]

The problem with writing an implementation spec in advance is that it requires deep knowledge of how Serverless is architected.

I believe it's same for implementing the feature. I think we should not alter sensitive parts without understanding why they're written that way, and assume that if things works for us, it'll not break other things for other users and that it aligns with changes that are planned for framework in future.

Serverless Framework, has anatomy which may not be complete, but provides good insight into general idea of how implemented engine works.

If you need answers to some other specific questions I'll be happy to answer.

[BryanCrotaz]

I think it's really simple. Packaging turns source code into a zip.

Serverless should be able to work with any language without your team understanding that language.

You've said it must work out of the box with node. Fine.

You've said that plug-ins can only augment behaviour and cannot replace it without using private API which is a Bad Thing. Fine.

Putting those two together implies that all languages must be supported in the main serverless system without using plugins, otherwise serverless will try to build for node.

I've done that. Where the code looks at the runtime, sees node and merely zips the entire folder, I've put a switch in to run msbuild if it's a dotnet project.

There's not much more to it apart from cleanup. I really don't know what more there is to say that I haven't said already.

[BryanCrotaz]

And a PS, it works! I've built a node project, and I've built a dotnet project. It behaves correctly for both.

In the case of node, the original behaviour is unaltered.

In the case of dotnet, I as the developer am pointing serverless at my dotnet project (which is self contained as always with dotnet) and saying "whatever's in there, build it". There's no case to say "it works for me" - there are no options, no edge cases. Build what was given to you and any problem with missing files is on the dotnet config end, exactly the same as with any other dotnet build process.

[BryanCrotaz]

I'm worried that reading between the lines, you really don't want this feature. If that's the case, please just say so and I'll permanently fork serverless for a serverless-dotnet.

Suffice to say I think that's a really bad idea. In our backend we have a mix of node and C# functions so it makes so much sense to build them both in serverless.

[medikoo]

I'm worried that reading between the lines, you really don't want this feature. If that's the case,

@BryanCrotaz I've never said it. We definitely need and want that feature, but we won't accept a contributions that doesn't adhere to our guidelines (there are very valid reasons for that).

If you don't want to adjust to that, it's fine, you don't have to, but at same time please do not expect that we will just blindly merge any code you propose (your word that "it works" doesn't make the code valid for merge)

So, if you want to contribute to this project, next step in that case is to outline in words (spec) how you want to solve it. If you're not sure how to do it, please check all other issues that are followed with a PR's.

If you don't like this contribution flow, that's totally fine, but then let's not add noise by discussing other things here. Instead let's leave the issue open for someone else who will be willing to provide a spec. I also plan to tackle that problem in scope of #6580

[BryanCrotaz]

I’ve outlined above how I want to solve it, and I have shown with a working example that that methodology works. What do you need me to add/change? You haven’t said what you think is wrong with my proposal above.

[medikoo]

I’ve outlined above how I want to solve it

I have problems seeing it. Can you re-edit main description of an issue, and add there section "Proposed solution" and provide an implementation spec there. See e.g. how it's done here: #8117

(Ideally we should have in this single place implementation spec evolving, until we agree it's the way to go.)

[BryanCrotaz]

Proposed solution:

Serverless should be able to work with any language. Let's implement dotnet.
Rules:

• It must work out of the box with node.
• Plug-ins can only augment behaviour and cannot replace it without using private API which is a Bad Thing.
• Putting those two together implies that all languages must be supported in the main serverless system without using plugins, otherwise serverless will try to build for node.

Where the code looks at the runtime, sees node and merely zips the entire folder, put a switch statement in to run msbuild if it's a dotnet project, and zip the resulting folder, or instead run the existing logic. The dotnet .csproj file is provided in the includes section of the config.

[medikoo]

Thanks @BryanCrotaz

put a switch statement in to run msbuild if it's a dotnet project, and zip the resulting folder

I understand that the idea is to use different packaging logic for packing .NET functions.

I think best way to approach this, is to:

1. Allow to inject a dedicated packaging logic for any provider/runtime case (and treat currently implemented solution as a default fallback)
2. Having above, implement dedicated packaging solution for AWS/.NET functions.

I believe best if both are treated separately and tackled with two consecutive PR's.

How does it sound?

[BryanCrotaz]

If there is going to be an addon for a provider/runtime, there should be an addon for all runtimes. Node shouldn't be treated differently.

By all means, have the node builder as a dependency so it's automatically installed, but place it as a plugin into the default config with a comment saying "Replace with your favourite runtime plugin". Otherwise you break your own rule "Plug-ins can only augment behaviour"

From your comment earlier in the conversation,

Currently packaging logic is configured as not only runtime agnostic but also vendor agnostic, and it's placed in lib/plugins/package/lib/packageService.js
I don't think that we should start adding there any vendor or runtime specific logic, but we can open some doors so such logic can be injected from out side.

I agree with that completely. But the current code is node specific, so it should be taken out into an addon.

[medikoo]

I think we need to have some default packaging logic (that simply packages contents of service folder) that's applied if for given provider/runtime no specific implementation was provided.

and that default imo should stay in a core. It'll be technically what we have now, eventually removing some Node.js specific things to Node.js specific packaging.

Anyway as I proposed above. Point 1 would be about paving path within a core so external packaging logic can be plugged in when needed for specific provider/runtime scenarios, and having 1, point 2 will be about preparing (possibly externally?) a packaging solution for AWS .NET.

I've updated main description to include a Proposed solution, let's keep it as live spec on intended approach.

Let me know if it looks ok to you (If it is, we can move forward to implemention spec)

[BryanCrotaz]

(that simply packages contents of service folder)

Currently the default packaging results in the entire project folder being zipped. I have a 10 line js function that produces a 244MB zip file for Lambda. It includes the package.json, the serverless.yml, the entirety of node_modules including serverless itself etc etc

Serverless should produce a reasonable output without configuration. It can be improved on with include/exclude, but sensible defaults need to be there.

I've done that for .Net by compiling the code into a tmp folder per function and this only includes the required DLLs. This reduces the package size with no configuration from 244MB to 1.5MB.

Serverless is marketed as an out of the box solution - you just need to add your specific code and resource definitions, and run deploy. That isn't the case in real usage.

My proposal is to make the default case work cleanly and easily for all languages supported by Lambda (eventually), with configuration as there already is to modify the defaults. There should be no reason to exclude a language if someone (e.g. me) is willing to make the default behaviour to support it (which I've done, and have been using in production for 5 months).