add caching to nginx.conf

[cloudfoundry]

add caching to nginx.conf

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/110687262.

[Dannyzen]

@mastertinner Thanks for putting an issue in. The team will discuss your proposal this week. In the interim, please feel free to use nginx.conf in the root folder as you wish.

[cloudlena]

@Dannyzen, thanks for letting me know. I am using a custom nginx.conf but I think it's something that would make sense to put into default nginx.conf. I'm looking forward to hearing about your decision.

[flavorjones]

Worth noting that I believe adding this feature for all applications will cause trouble for apps staged with <1M of memory. Let's please investigate that before considering this PR.

It might be more interesting to consider using the application memory quota to imply an appropriate caching limit (if we can access this information at application startup time).

[Dannyzen]

Hi @mastertinner when you say greatly improve performance what change metrics can you share that support this claim?

[cloudlena]

@Dannyzen, I can't say that in general because it depends on the site size and structure of the respective application. What I can say is that caching makes sense for apps which consist of only static files because then the client doesn't have to download the same content every time it visits the app, which can make the app a lot faster from a client's perspective and use less resources for nginx. I stumbled upon the issue when I was testing my apps hosted by this buildpack with Google Pagespeed and Pingdom Website Speed Test which both advise to use caching for static content.

Thanks for your input, @flavorjones! Maybe it would make sense to make this an opt-in feature through ENV variables like the FORCE_HTTPS feature:

<% if ENV["CACHING_EXPIRY"] %>
  location ~* \.(jpe?g|png|gif|ico|svg|css|js|eot|ttf|woff|woff2)$ {
    expires <%= ENV["CACHING_EXPIRY"] %>;
  }
<% end %>

or

<% if ENV["ENABLE_CACHING"] %>
  location ~* \.(jpe?g|png|gif|ico|svg|css|js|eot|ttf|woff|woff2)$ {
    expires 1M;
  }
<% end %>

[flavorjones]

@mastertinner I need more help understanding exactly what you're hoping to get out of enabling caching withing nginx. I've found that nginx reading static files from disk is pretty fast with out-of-the-box nginx options. Without some benchmarks that show compelling improvements in performance, I would likely vote against adding this to the buildpack.

Now, if you're talking about enabling cache-control headers for use by a CDN, that's something I can definitely get behind, but that's not what the nginx option you're suggesting deals with.

Finally, keep in mind that you're free to use whatever nginx config you choose by providing an nginx.conf file. See the readme for more information.

[cloudlena]

@flavorjones, my concern is not about reading files from disk but rather about the second point you mentioned: setting the right headers to allow a visitor's browser to cache static files so that it doesn't need to download them every time the user visits the site. In my point of view this is important not only for CDNs but for any static website/web app. I think the term I'm trying to explain is referred to as "Leverage Browser Caching": https://developers.google.com/speed/docs/insights/LeverageBrowserCaching

[flavorjones]

@mastertinner Got it! Just to reiterate, the option you suggested above does not control cache headers, but @Dannyzen will be able to talk a bit about cache control headers, as we have another github issue talking about this that I can't find right now.

[Dannyzen]

@mastertinner I'm hesitant to include the optimizations mentioned in this issue without data to prove positive impact. Additionally, the expires 1M; could yield unfavorable results in certain scenarios. With that said, @flavorjones mentioned #59 which we would like to explore more with your help.