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

Add a "local interface IP mode" to the AWS cloud provider. #518

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sdewitt-newrelic
Copy link
Contributor

One of my customers uses gostatsd in a configuration where each EC2 instance has a gostatsd instance colocated with a single application instance. The application instance and gostatsd communicate with each other via UDP over the loopback interface. In this scenario, the destination IP of the UDP datagram packets is 127.0.0.1.

The current implementation of the AWS cloud provider looks up EC2 instance tags by using the destination addresses from the inbound UDP datagrams in the private-ip-address attribute of the filter on the DescribeInstances operation. This approach breaks if the UDP datagram packets are not the private IP addresses of EC2 instances, particularly if they are local interface IP addresses like 127.0.0.1.

This PR adds support for a "local interface IP mode". It can be enabled by setting the local_ip_mode to allow in the cloud provider configuration. In this mode, the AWS cloud provider will check the inbound IP addresses against a local IP "whitelist". IP addresses which match will cause the cloud provider to locate instances by setting
the instance-id in the DescribeInstances filter to the instance ID returned
by the EC2 instance identity document.

@tiedotguy
Copy link
Collaborator

Hi

I believe this approach is an anti-pattern. When running in a sidecar configuration like this (which is generally how we run it as well these days), the provider isn't needed as the tags can be looked up ahead of time and put in the configuration file.

By doing it ahead of time, it also allows the service owner to pick what actually matters from the available tags (ie, some tags don't add actual value, so shouldn't flow to metrics), and allows enforcing tags by refusing to run if they're absent.

@sdewitt-newrelic
Copy link
Contributor Author

Hi @tiedotguy, I understand what you are saying but this means there is more than one source of truth for the tags. For example, if I add a tag across my EC2 instances with the AWS CLI or console, I now also have to add that tag to my configuration files. This means not only do I now have to redeploy every time I want to add/remove/edit a tag but leads to human error entering those tags in the configuration. I could easily misspell a tag or value. This doesn't seem like very maintainable or scalable approach. But most importantly is, as I said, the fact that there is more than one source.
There should only ever be one source of truth for the tags. As aside, while I know this is not a good technical argument to do anything, other statsD solution provide the exact capability in this PR.

@sdewitt-newrelic
Copy link
Contributor Author

Hi @tiedotguy, let me know if you have any comments on my comment above.

@sdewitt-newrelic
Copy link
Contributor Author

Hi @tiedotguy, any comments on the above? This is something one of my customers is hoping can make it in.

@sdewitt-newrelic
Copy link
Contributor Author

Hi @tiedotguy just checking in again.

@tiedotguy
Copy link
Collaborator

Hi

I've done a bunch of thinking about this, because I don't like this solution, but I recognize it's a pain to add additional tooling to an instance to lookup tags on startup. I'm not going to accept a change in to the cloud provider to treat localhost as special, specifically because it's turning something static in to a dynamic lookup - the cloud provider is for when hosts come and go.

As an alternate approach though, I'd accept a change which (on startup) looks up cloud tags and adds them to the static list (tag keys should be de-duped, with the cloud tag winning if there's a conflict with an existing static tag). The config for this would be a list of cloud tag names, and what they map to. As a hypothetical example (somewhere in the config file)

SERVICE_NAME: "service_id"
GROUP: "service_group"

Then if an ec2 instance has SERVICE_NAME=foo, the static tag list would have service_id:foo added to it (and if there's no GROUP tag, then nothing extra is added)

This provides three main benefits:

  1. You can choose in config which tags you want
  2. You can't request all tags
  3. You can map tags between different standards (ie, "billing" vs metrics)

The reasons for 1 and 2 are that historically the automatic addition of tags has been a problem because it prevents the user from doing global aggregation, and so I believe adding tags should, to the extent possible, always be explicit.

Ideally this would use the existing cloud provider infra to look up the tags, so that future cloud providers (if any) work "for free".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants