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

[exporter/awscloudwatchlogsexporter] add external id config for role assumption #38062

Merged

Conversation

binarymatt
Copy link
Contributor

Description

When assuming a role in an external AWS account, there are cases where it's desirable to have an external id in the role assumption trust policy. This pull requests adds external id as an optional parameter when role assumption is used to retrieve AWS credentials.

This is reopening this pr

Link to tracking issue

Fixes

Testing

Documentation

Copy link
Contributor

github-actions bot commented Mar 6, 2025

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@binarymatt
Copy link
Contributor Author

what is the best way to move forward with this?

Copy link
Contributor

@dehaansa dehaansa left a comment

Choose a reason for hiding this comment

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

Do any codeowners have relevant environments to validate the approach? Otherwise it looks fine to me, and shouldn't change behavior if the field isn't configured.

@@ -31,6 +31,8 @@ The following settings can be optionally configured:
- `log_retention`: LogRetention is the option to set the log retention policy for only newly created CloudWatch Log Groups. Defaults to Never Expire if not specified or set to 0. Possible values for retention in days are 1, 3, 5, 7, 14, 30, 60, 90, 120, 150, 180, 365, 400, 545, 731, 1827, 2192, 2557, 2922, 3288, or 3653.
- `tags`: Tags is the option to set tags for the CloudWatch Log Group. If specified, please add at most 50 tags. Input is a string to string map like so: { 'key': 'value' }. Keys must be between 1-128 characters and follow the regex pattern: `^([\p{L}\p{Z}\p{N}_.:/=+\-@]+)$`(alphanumerics, whitespace, and _.:/=+-!). Values must be between 1-256 characters and follow the regex pattern: `^([\p{L}\p{Z}\p{N}_.:/=+\-@]*)$`(alphanumerics, whitespace, and _.:/=+-!). [Link to tagging restrictions](https://docs.aws.amazon.com/AmazonCloudWatchLogs/latest/APIReference/API_CreateLogGroup.html#:~:text=Required%3A%20Yes-,tags,-The%20key%2Dvalue)
- `raw_log`: Boolean default false. If set to true, only the log message will be exported to CloudWatch Logs. This needs to be set to true for [EMF logs](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch_Embedded_Metric_Format_Specification.html).
- `role_arn`: IAM role to upload logs to a different account.
- `external_id`: Shared identitier used when assuming an IAM role in an external AWS account.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have any useful AWS documentation links to attach for the docs? (add to changelog subtext as well if available).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added links to the docs and subtext

@binarymatt binarymatt force-pushed the awscloudwatchexporter_external_id branch from b442aaa to 3ae04dd Compare March 6, 2025 17:29
@dehaansa
Copy link
Contributor

dehaansa commented Mar 6, 2025

codeowners CI task issue is due to changes on main, needs a merge or rebase.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@github-actions github-actions bot removed the Stale label Mar 7, 2025
@binarymatt
Copy link
Contributor Author

@dehaansa I noticed that the codegen check is failing (btw, thanks for re-running). When I run make generate locally, i see that this readme exporter/awscloudwatchlogsexporter/README.md is updated. Should i just commit that to see if it solves the failing step?

@dehaansa
Copy link
Contributor

dehaansa commented Mar 7, 2025

@dehaansa I noticed that the codegen check is failing (btw, thanks for re-running). When I run make generate locally, i see that this readme exporter/awscloudwatchlogsexporter/README.md is updated. Should i just commit that to see if it solves the failing step?

Yep, update it 👍 . I hadn't actually read the error from the workflow because we've had a lot of flakiness in the build_examples CI so I just blindly clicked restart :)

@dehaansa dehaansa added the ready to merge Code review completed; ready to merge by maintainers label Mar 7, 2025
@atoulme atoulme merged commit b5b5d0c into open-telemetry:main Mar 7, 2025
197 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/awscloudwatchlogs awscloudwatchlogs exporter exporter/awsemf awsemf exporter exporter/awsxray internal/aws ready to merge Code review completed; ready to merge by maintainers waiting-for-code-owners
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants