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

feat(new transform): Initial aws_ec2_metadata transform imple… #1325

Merged
merged 15 commits into from
Dec 16, 2019

Conversation

LucioFranco
Copy link
Contributor

@LucioFranco LucioFranco commented Dec 6, 2019

Opening this for initial feedback, most of the internals are complete, what is left is to build out the integration tests and figure out how we want to handle the subnet_id and vpc_id since those require us to supply a mac address of the network interface we want to get the id's from.

As for internal implementation, we are using an evmap that allows us to fill a map and then switch the pointers which should mean we don't have to acquire a lock for each event. This should perform just about the same as a regular transform. I think this method improves upon the previous design I came up with while being simpler in the critical path.

This PR also introduces ways we can start to use async/await without our codebase.

Closes #736

Ref fluent/fluent-bit#1780

Signed-off-by: Lucio Franco luciofranco14@gmail.com

docker-compose.yml Outdated Show resolved Hide resolved
@binarylogic binarylogic changed the title feat(new transform): Add aws_ec2_metadat transform feat(new transform): Initial aws_ec2_metadata transform implementation Dec 7, 2019
@LucioFranco
Copy link
Contributor Author

Update on this: I am not going to block this PR on this issue #1338 but it will mean that we will ship this initially without proper correlation logging. I don't think this will be a big deal since it should be quite obvious where the logs are coming from but this would be a good enhancment.

@LucioFranco LucioFranco marked this pull request as ready for review December 9, 2019 23:23
@LucioFranco
Copy link
Contributor Author

This PR should be ready for review, I plan on adding docs tomorrow morning.

@binarylogic just FYI this does not add iam role name as I couldn't find a way to fetch that, the api you provided expects the user to pass the role-name and the iam/info api call doesn't provide the role name. If we find a better solution we probably want to add that in a follow up.

@binarylogic
Copy link
Contributor

just FYI this does not add iam role name

That's ok. Thanks for letting me know.

@jszwedko
Copy link
Member

jszwedko commented Dec 10, 2019

You should be able to fetch the IAM role via:

$ curl http://169.254.169.254/latest/meta-data/iam/info
{
  "Code" : "Success",
  "LastUpdated" : "2019-12-10T16:43:08Z",
  "InstanceProfileArn" : "arn:aws:iam::754402436383:instance-profile/odin_client_logs_hive_ingestion/production/odin_client_logs_hive_ingestion-production-ec2-role",
  "InstanceProfileId" : "AIPAJFMF2ZTI22JBHEJ54"
}

The InstanceProfileArn there is the role. Realized that that is the instance profile and may not match.

You can curl /iam/security-credentials and get a "directory listing":

$ curl http://169.254.169.254/latest/meta-data/iam/security-credentials
odin_client_logs_hive_ingestion-production-ec2-role

@LucioFranco
Copy link
Contributor Author

@jszwedko ah good point, the docs are not very clear about it being a directory listing but instead suggests that you need to have the name prior to invoking that request. https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instancedata-data-categories.html

I will add this now then. Looks like a decent alternative.

Copy link
Member

@lukesteensen lukesteensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nice! I was going to say we should chat before bringing in the new futures stuff, but I definitely do not want to review the version of this that doesn't use async/await 😄

@@ -45,6 +45,7 @@ tracing-limit = { path = "lib/tracing-limit" }

# Tokio / Futures
futures = "0.1.25"
futures03 = { package = "futures", version = "0.3", default-features = false, features = ["compat"] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😱

src/transforms/aws_ec2_metadata.rs Outdated Show resolved Hide resolved
src/transforms/aws_ec2_metadata.rs Outdated Show resolved Hide resolved
src/transforms/aws_ec2_metadata.rs Show resolved Hide resolved
src/transforms/aws_ec2_metadata.rs Outdated Show resolved Hide resolved
src/transforms/aws_ec2_metadata.rs Outdated Show resolved Hide resolved
src/transforms/aws_ec2_metadata.rs Outdated Show resolved Hide resolved
src/transforms/aws_ec2_metadata.rs Show resolved Hide resolved
LucioFranco and others added 15 commits December 16, 2019 13:50
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
Copy link
Contributor

@binarylogic binarylogic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the output fields to the docs. Everything else is good. :shipit:

@LucioFranco LucioFranco changed the title feat(new transform): Initial aws_ec2_metadata transform implementation feat(new transform): Initial aws_ec2_metadata transform imple… Dec 16, 2019
@LucioFranco LucioFranco merged commit 648e28c into master Dec 16, 2019
@LucioFranco LucioFranco deleted the lucio/ec2-metadata branch December 16, 2019 21:36
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.

New aws_ec2_metadata transform
4 participants