-
Notifications
You must be signed in to change notification settings - Fork 436
feat: service discovery by default #726
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
Conversation
efekarakus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome yay, looks really good! just a few questions
iamhopaul123
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 just a small question: why not use "${appName}.local", since the domain is only available inside the same environment?
The reason is in case there's another service discovery namespace in that VPC with |
oh I see what you mean. |
This change enables service discovery by default for
all apps. Service discovery alows apps within an
environment to communicate with each other without
having to traverse the internet (traffic is routed
internally within the VPC).
To access a service via its service discovery endpoint,
you can make a request like:
```golang
resp, _ := http.Get(fmt.Sprintf("http://%s.%s", appName, os.Getenv("ECS_APP_DISCOVERY_ENDPOINT"))
```
Or, with only the project name:
```golang
resp, _ := http.Get(fmt.Sprintf("http://%s.%s.local", appName, projectName))
```
Or, without the string interpolation:
```golang
# Making a request to app `api` in project `kudos`
resp, _ := http.Get("http://api.kudos.local")
```
The same address works across environments, since the requests
are scoped to services within one environment.
__ Why not just .local ?__
A couple of reasons to go for the `.{project}.local` scheme, rather
than just `.local` or just `.{project}`.
The reason for not using just `.local` is just to reduce the chance
of a namespace colision if another project is using the same VPC.
The reason for not just going with `.{project}` is in case the
project name matches an existing TLD. That'd be confusing behavior.
The `.local` TLD doesn't seem to exist.
__ Why make this default for every service? __
You can't add service discovery after the fact, unfortunately.
The only sensible thing is to just enable it by default. In the
future, we can offer a disable knob, but it's very cheap (less
than a dollar per month per env) - so isn't a huge burden for
customers who end up not using it.
resolves aws#599
This change enables service discovery by default for
all apps. Service discovery alows apps within an
environment to communicate with each other without
having to traverse the internet (traffic is routed
internally within the VPC).
To access a service via its service discovery endpoint,
you can make a request like:
Or, with just the project name:
Or, without the string interpolation:
The same address works across environments, since the requests
are scoped to services within one environment.
Most of this change is making our E2E test apps little services so we
can test that they actually spin up and are able to preform HTTP
calls over their service discovery endpoints.
Why not just .local ?
A couple of reasons to go for the
.{project}.localscheme, ratherthan just
.localor just.{project}.The reason for not using just
.localis just to reduce the chanceof a namespace collision if another project is using the same VPC.
The reason for not just going with
.{project}is in case theproject name matches an existing TLD. That'd be confusing behavior.
The
.localTLD doesn't seem to exist.Why make this default for every service?
You can't add service discovery after the fact, unfortunately.
The only sensible thing is to just enable it by default. In the
future, we can offer a disable knob, but it's very cheap (less
than a dollar per month per env) - so isn't a huge burden for
customers who end up not using it.
Resolves #599
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.