search

golang / appengine

Go App Engine packages
http://google.golang.org/appengine
Apache License 2.0
671 stars 206 forks source link

Porting an application to App Engine Standard has been extremely painful #106

Closed kevinburke closed 1 year ago

kevinburke commented 7 years ago

I'm a contributor to the Go language and a consultant who spends a lot of time on builds, devops, and deploys. Presumably I should be able to figure out how to deploy an application, but I've spent the better half of a day porting an application to App Engine Standard and still don't have something usable. It's been an immensely frustrating experience. I understand I have to make changes to make it work (put code behind build tags, remove calls to "unsafe", etc) but there have been a host of inscrutable error messages that have made the whole thing incredibly difficult to debug.

The app is an open source application designed to streamline the process of contacting your local city politicians. Several Google employees have used it to contact the Mountain View City Council, for example. It's currently deployed on App Engine Flex, but it gets maybe 20-30 hits a day and I can't justify spending $360 a year on it. I think App Engine Standard is the only way to lower the price to a place I want.

I want to say I got to a point where I actually got this deployed and up and working but I didn't. I can't figure out how to keep "master" at a point where it can support deployments to App Engine Standard, and also normal deployments listening on a socket. I don't have time to investigate all of these fully, but some of the roadblocks I ran into. 

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x184042c]

goroutine 1 [running]:
google.golang.org/appengine/internal.logf(0x0, 0x0, 0x3, 0x1ad9485, 0x14, 0x0, 0x0, 0x0)
    google.golang.org/appengine/internal/api_classic.go:151 +0x20c
google.golang.org/appengine/internal.Logf(0x2abe7eebf058, 0xc008412390, 0x3, 0x1ad9485, 0x14, 0x0, 0x0, 0x0)
    google.golang.org/appengine/internal/api_common.go:82 +0x12e
google.golang.org/appengine/log.Errorf(0x2abe7eebf058, 0xc008412390, 0x1ad9485, 0x14, 0x0, 0x0, 0x0)
    google.golang.org/appengine/log/api.go:34 +0x73
main.init.3()
    main_appengine.go:15 +0x91
main.init()
    push.go:25 +0x396

Here is as far as I got: https://github.com/kevinburke/multi-emailer/compare/appengine?expand=1. Here is the app.yaml I am using: 

api_version: go1
env: standard
runtime: go

handlers:
- url: /.*
  script: _go_app

skip_files:
    - vendor/golang.org/x/sys/unix
    - vendor/google.golang.org/appengine
    - vendor/google.golang.org/api
    - vendor/golang.org/x/oauth2
    - vendor/golang.org/x/oauth2/google
    - vendor/golang.org/x/oauth2/jwt

Contact me for a valid config.yml file for configuring the server, or follow the instructions here to get a Google Client ID and Secret. https://github.com/kevinburke/multi-emailer/#installation

I've also tried:

leighmcculloch commented 7 years ago

💯 Most of the dot points above echo the exact same pains I had when I moved looks.wtf (small simple web app) from Pivotal Web Services to AppEngine in May 2017. I finally got the app ported but I don't know how I fixed some of the issues because there were so many errors all at once. I never figured out how to get vendor'd code to work and I settled on not vendoring code instead.

This was my final diff that seems pretty simple, but there were a lot of gotchas to get here: https://github.com/leighmcculloch/looks.wtf/commit/4572ea46c6d994b74b475f6db9e4993bab3287e6

My thoughts off my experience were:

ejcx commented 7 years ago

Since kb and leigh both linked me to this thread while I complained loudly, I figured I would provide my feedback here!

I ported an app that has about 6 API endpoints, used a redis, and a Saas provider . I wanted to move this to appengine and it was previously running on a $5 digital ocean droplet.

I ran in to a lot of troubles:

logs didn't work. I have lots of sanity checks to make sure my code has everything it needs when it starts in the init functions, otherwise it will die. I don't know if there was something I was missing, but it seemed like I could no longer log from init.

I switched over to logging with the appengine logging and it required a context, which I could only get from a request, and couldn't continue logging these types of startup issues in init.

contexts everywhere Contexts are cool, but being required to pass them everywhere or to use a GAE API is a lot. Since I switched my datastore from redis to memcache I had to change almost every function's signature to pass contexts and requests around, all the way down to my get() and set().

new http client I really dislike having to use a different http client than the default on App Engine Standard. Luckily I found a way to hack around this issue in my SaaS providers library but this was almost a major issue for my continued progress.

I'm sure I may hit other issues or painpoints but figured I would pass this along. I really think y'all are doing a great job and I'm sure once I fully get my mind around appengine I won't feel as much pain.

themihai commented 6 years ago

Appengine is a proprietary platform so you should expect it to be painful... if porting to appengine standard is painful try to port it outside of GAE...nothing will work.

sid-code commented 6 years ago

I connect to postgres cloud sql in my init() and I'm trying to figure out why it isn't working. Am I screwed because logging isn't allowed from the init function?

leighmcculloch commented 6 years ago

@sid-code Probably. I think you'll want to move that logic into a function that you call at the beginning of handling any incoming request, and wrap the logic inside the function inside a sync.Once. Like this: https://play.golang.org/p/rmPXf540Qof.

delphinus commented 6 years ago

@sid-code In addition to that, you can init()ing on the “warmup request”. GAE does the requests whenever instances start.

Configuring Warmup Requests to Improve Performance  |  App Engine standard environment for Go  |  Google Cloud

You should use sync.Once on the warmup handler for robust implementation.

sid-code commented 6 years ago

@leighmcculloch @delphinus I'll look into these, thanks.

sbuss commented 6 years ago

Hi all, sorry that I missed this issue! I'll try to address everything, but if I miss something please let me know.

First, hey @kevinburke! Thanks for your tool; I use it all the time to email the SF Supervisors to push YIMBY policies :)

The issues you ran into while porting your app are due to two known issues with Go on App Engine standard which will go away with our next release:

1) Vendor support is broken 2) Our custom builder for App Engine standard has trouble with packages underneath the directory containing app.yaml

To deal with these issues, you have to abandon your vendor directory and only depend on packages on your GOPATH, as well as move your package main and app.yaml files into a subpackage.

You can see what these changes look like on my fork: https://github.com/sbuss/multi-emailer/tree/appengine

I am sure you'll be unhappy about losing vendoring support, and I absolutely feel that pain, but if you want to wait a few more months, you'll be able to vendor on App Engine Standard in our next release.

I think skip_files was being ignored because of the way we re-write the app for deployment. Dependencies end up getting put on the GOPATH, so I think everything in vendor got moved up a directory and no longer matched the patterns you defined.

I'm sorry for the bad and confusing error messages.

github.com/inconshreveable/log15 can, in some build contexts, import golang.org/x/sys/unix. This import is not required by appengine, but its presence in the vendor folder removed the ability to build and deploy the project.

This may have been a false alarm, due to the lack of support for vendoring. I can't say for certain without seeing the error message, though.

I vendored google.golang.org/appengine, and "gcloud app deploy" complained that it was attempting to import an internal package, something along these lines: I got this error message, e.g. "files that cannot be compiled", with no files listed:

The solution to this was "gcloud components update" but it is not clear from the error message or context that that will or should fix the proposed issue.

That's....confusing! I really don't understand why updating the components would have gotten rid of that error message.

I expected standard output to log to the App Engine logger, but it didn't. I then switched the logs to use log.Errorf(), which panics with "nil" when called with a standard library context.Context. I defy anyone to look at this stack trace and conclude "oh, I need to use appengine.NewContext instead:"

This is definitely an unpleasant surprise for everyone that encounters it. The next release of App Engine will capture stdout/stderr and include it in the application logs.

It took me a while to realize that log messages are attached to a request context and you have to hit the arrow to see them, instead of given their own line (I believe on Flex log messages can be on their own line).

Flex has them on their own line because it can't reliably associate stdout/stderr with a request, so every line of stdour/stderr gets included as an individual message and not in the fancy request-log dropdown. The next release of standard will have similar behavior, but we'll keep recommending using google.golang.org/appengine/log to get logs grouped by request.

I had to update a dependency so it used an App Engine context as well: kevinburke/google-oauth-handler@d52403d. That's pretty frustrating, since everyone that vendors it using e.g. "dep" will wonder why they should take a dependency on google.golang.org/appengine.

Thank you to @leighmcculloch for his comments on how to avoid doing that!

I finally blew away the entire vendor directory and managed to get a deployment working. In order to try to get the app working, I tried selectively adding back directories and could not get anything working again. Here's an example error I ran into:

That was the right approach, but then you ran into the issue where no packages could be below a directory containing package main. There's really no way of knowing that from the error messages, and I'm sorry the docs don't spell this out well. This limitation is also going to go away with the next release.

(responses to other comments in subsequent posts)

sbuss commented 6 years ago

@leighmcculloch

If requests always had context's, inside and outside of appengine porting would probably be simpler, removing the need to do appengine.NewContext(r) since requests were built already with an appengine context when deployed on appengine, and with a context.Background() when deployed elsewhere.

The story around context is rather unfortunate. Contexts proliferate everywhere, but we have historically needed the information in the Context struct to do things correctly on our side. I don't see this going away any time soon, but I'll try to keep it in mind.

If there were meaningful error messages about using the built in log package, it'd help us realize sooner why none of our logs are showing up in AppEngine.

This would have been a great thing to add to App Engine standard, but since logs will start showing up in the next release anyway, I'll just punt :)

Meaningful error messages about double imports when an app is vendoring the appengine packages would be \o/.

You're right, I should have added a useful error message if the vendor directory was present. Sorry :\ But since we'll support vendoring in the next release, the errors should just go away entirely!

sbuss commented 6 years ago

@ejcx

logs didn't work. I have lots of sanity checks to make sure my code has everything it needs when it starts in the init functions, otherwise it will die. I don't know if there was something I was missing, but it seemed like I could no longer log from init.

I switched over to logging with the appengine logging and it required a context, which I could only get from a request, and couldn't continue logging these types of startup issues in init.

As I mentioned in the previous two comments, we will be collecting stderr/stdout in our next release, so this should stop being a pain point. I definitely understand your frustration, this is an issue I encounter a lot during internal development!

contexts everywhere Contexts are cool, but being required to pass them everywhere or to use a GAE API is a lot. Since I switched my datastore from redis to memcache I had to change almost every function's signature to pass contexts and requests around, all the way down to my get() and set().

I also feel this frustration. I don't have a good answer, though :\

new http client I really dislike having to use a different http client than the default on App Engine Standard. Luckily I found a way to hack around this issue in my SaaS providers library but this was almost a major issue for my continued progress.

Totally. The next release will let you use http.ListenAndServe and raw sockets normally :)

I'm sure I may hit other issues or painpoints but figured I would pass this along. I really think y'all are doing a great job and I'm sure once I fully get my mind around appengine I won't feel as much pain.

Thanks! I hope you stick with us, good things are coming!

leighmcculloch commented 6 years ago

@sbuss

Thanks for such thorough responses to our questions and ideas. It is much appreciated!

The story around context is rather unfortunate. Contexts proliferate everywhere, but we have historically needed the information in the Context struct to do things correctly on our side. I don't see this going away any time soon, but I'll try to keep it in mind.

I think I did a poor job of presenting the idea I was intending to present. The following is a simple pattern I use for getting the appropriate context into handlers of applications that need to be deployed on both GAE and elsewhere.

app.go

package app

type appHandler func(http.ResponseWriter, *http.Request)

func init() {
    http.Handle("/oauth", appHandler(oauthHandler))
}

func oauthHandler(w http.ResponseWriter, r *http.Request) error {
    c := r.Context()
    ...
}

app_appengine.go

// +build appengine

package app

func (a appHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    c := appengine.NewContext(r)
    r = r.WithContext(c)
    a(w, r)
}

app_native.go

// +build !appengine

package app

func (a appHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    a(w, r)
}

func main() {
    port := os.Getenv("PORT")
    http.ListenAndServe(":"+port, nil)
}

This works because requests have a context by default. When calling Context() the background context is returned in non-GAE apps. If GAE provided the GAE context by default on the requests given to handlers, apps could just use the context in the request without needing to construct a GAE context. The code above would become:

app.go

package app

func init() {
    http.Handle("/oauth", oauthHandler)
}

func oauthHandler(w http.ResponseWriter, r *http.Request) error {
    c := r.Context()
    ...
}

app_native.go

// +build !appengine

package app

func main() {
    port := os.Getenv("PORT")
    http.ListenAndServe(":"+port, nil)
}
kevinburke commented 6 years ago

Steven, thanks for all of your feedback here!

leighmcculloch commented 5 years ago

Given the new go111 runtime addresses many of the pain points we raised here, this could be closed?