Pratham add logs #12
Open
Pratham add logs #12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ganeshjagdale9545
requested changes
May 6, 2025
Collaborator
ganeshjagdale9545
left a comment
ganeshjagdale9545
left a comment
There was a problem hiding this comment.
one minor change needed
cloudtracker/cli.py
Outdated
| cloudtrail_log_paths[log_level]["bucket"] = logging_account["bucketName"] | ||
| except (s3.exceptions.NoSuchKey, s3.exceptions.NoSuchBucket, botocore.exceptions.ClientError) as e: | ||
| continue | ||
| if cloudtrail_log_paths[log_level].get("boto3_session"): |
Collaborator
There was a problem hiding this comment.
Remove this if else condition. if its fail to create session it will throw an error
There was a problem hiding this comment.
Pull Request Overview
This PR introduces detailed logging throughout CloudTracker by adding a new logger parameter (cdx_logger) in multiple modules, replacing direct logging calls with consistent cdx_logger invocations.
- Added detailed logging to the ElasticSearch, Athena, and CLI modules.
- Updated function signatures in init.py to pass and use the new cdx_logger for consistent logging across the codebase.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cloudtracker/datasources/es.py | Updated init signature and logger usage. |
| cloudtracker/datasources/athena.py | Replaced logging calls with cdx_logger and expanded logging in query flows. |
| cloudtracker/cli.py | Modified main() to receive cdx_logger and updated logging calls. |
| cloudtracker/init.py | Updated helper functions to accept cdx_logger and adjust logging calls. |
Comments suppressed due to low confidence (1)
cloudtracker/cli.py:142
- [nitpick] Concatenating principal ARNs into a string using string addition may become hard to maintain as the logic grows; consider using a list to collect ARNs and then joining them with a suitable separator for improved clarity and potential reuse.
principals_arn = ""
Collaborator
|
@pmalu9211 can you please resolve this comments? |
ganeshjagdale9545
approved these changes
May 20, 2025
Collaborator
ganeshjagdale9545
left a comment
ganeshjagdale9545
left a comment
There was a problem hiding this comment.
looks good to me
@pmalu9211 can you please make changes in cloud-iam-entitlements project?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LINK TO CARD