Skip to content

feat: add cloudtrail tracking to execute_bash #2535

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

Merged
merged 1 commit into from
Aug 11, 2025

Conversation

abhraina-aws
Copy link
Contributor

Issue #, if available:
Cloudtrail logs were not having QCLI identifier if we used execute_bash tool.

Description of changes:
Added the code to enable these identifires.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

#[test]
fn test_cloudtrail_tracking_with_existing_env() {
// Set an existing AWS_EXECUTION_ENV value
unsafe {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not use unsafe - tests run in multiple threads within the same process. Instead, pass the Os type for getting and setting env vars

Copy link
Contributor

Choose a reason for hiding this comment

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

Are we still going to keep the unsafe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is more context here. According to him unsafe here is not unsafe. I had added a non unsafe version but that was very complicated. So he referred to this method.

}

#[tokio::test]
async fn test_cloudtrail_tracking_with_existing_env() {
Copy link
Contributor

Choose a reason for hiding this comment

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

don't construct Os like this, you could just do Os::new then follow with an unsafe set_var (note that this is safe because the test instance uses an in-memory hashmap)

@@ -418,6 +421,36 @@ pub fn queue_function_result(result: &str, updates: &mut impl Write, is_error: b
Ok(())
}

/// Helper function to set up environment variables with user agent metadata for CloudTrail tracking
pub fn user_agent_env_vars(os: &Os) -> std::collections::HashMap<String, String> {
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is including all env vars, probably better to instead call it env_vars_with_user_agent

- Add CloudTrail user agent metadata to execute_bash tool for tracking
- Consolidate CloudTrail constants in consts.rs to eliminate duplication
- Create shared env_vars_with_user_agent() function used by both use_aws and execute_bash
- Add comprehensive tests for CloudTrail tracking functionality
- Support both Unix and Windows platforms with proper environment variable handling
- Maintain backward compatibility and thread-safe implementation
@brandonskiser brandonskiser merged commit 6279b47 into aws:main Aug 11, 2025
1 check passed
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.

3 participants