Skip to content

Conversation

duranbe
Copy link
Contributor

@duranbe duranbe commented Aug 10, 2025

Issue #35000

Reason for this change
Logging Configuration has been recently added to Event Bridge buses and is supported in L1 construct but not L2.
Relevant Cloudformation Template Doc : https://docs.aws.amazon.com/AWSCloudFormation/latest/TemplateReference/aws-properties-events-eventbus-logconfig.html
AWS Doc for Configuring Logs : https://docs.aws.amazon.com/eventbridge/latest/userguide/eb-event-bus-logs.html#eb-event-logs-data

Description of changes

  • Creation of an Enum for IncludeDetail property
export enum IncludeDetail {
  /**
   * FULL: Include all details related to event itself and the request EventBridge sends to the target.
   * Detailed data can be useful for troubleshooting and debugging.
   */
  FULL = 'FULL',
  /**
   * NONE: Does not include any details.
   */
  NONE = 'NONE',
}
  • Creation of an Enum for Level property
export enum Level {
  /**
   * INFO: EventBridge sends any logs related to errors, as well as major steps performed during event processing
   */
  INFO = 'INFO',
  /**
   * ERROR: EventBridge sends any logs related to errors generated during event processing and target delivery.
   */
  ERROR = 'ERROR',
  /**
   * TRACE: EventBridge sends any logs generated during all steps in the event processing.
   */
  TRACE = 'TRACE',
  /**
   *  OFF: EventBridge does not send any logs. This is the default.
   */
  OFF = 'OFF',
}
  • Creation of an Interface for LogConfiguration, with both properties optional as stated in the doc
export interface LogConfig {
  /**
   * Whether EventBridge include detailed event information in the records it generates.
   * @default no details
   */
  readonly includeDetail?: IncludeDetail;
  /**
   * Logging level
   * @default OFF
   */
  readonly level?: Level;
}
  • Update L2 Construct and props with optional property Log Config
readonly logConfig?: LogConfig;

Description of how you validated changes

  • Unit and Integration Tests

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@aws-cdk-automation aws-cdk-automation requested a review from a team August 10, 2025 20:47
@github-actions github-actions bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels Aug 10, 2025
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

(This review is outdated)

@aws-cdk-automation aws-cdk-automation added the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Aug 10, 2025
@aws-cdk-automation aws-cdk-automation dismissed their stale review August 13, 2025 10:30

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@aws-cdk-automation aws-cdk-automation removed the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Aug 13, 2025
@duranbe
Copy link
Contributor Author

duranbe commented Aug 13, 2025

Im not really sure why security guardian failed?

@duranbe
Copy link
Contributor Author

duranbe commented Aug 13, 2025

ah I figured out the issue -> the test was using an IAM root account as assumedBy so I created a role instead

@duranbe
Copy link
Contributor Author

duranbe commented Aug 13, 2025

Ready for review! :)

@rix0rrr rix0rrr self-assigned this Aug 19, 2025
@duranbe duranbe requested a review from rix0rrr August 21, 2025 21:18
Copy link
Contributor

mergify bot commented Aug 26, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Copy link
Contributor

mergify bot commented Aug 26, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@duranbe
Copy link
Contributor Author

duranbe commented Aug 26, 2025

@Mergifyio requeue

Copy link
Contributor

mergify bot commented Aug 26, 2025

requeue

❌ Command disallowed due to command restrictions in the Mergify configuration.

  • sender-permission >= write

@duranbe
Copy link
Contributor Author

duranbe commented Aug 26, 2025

[2/4] Fetching packages...
error Error: https://registry.npmjs.org/fast-uri/-/fast-uri-3.0.6.tgz: Request failed "429 Too Many Requests"
    at ResponseError.ExtendableBuiltin (/usr/local/lib/node_modules/yarn/lib/cli.js:696:66)
    at new ResponseError (/usr/local/lib/node_modules/yarn/lib/cli.js:802:124)
    at Request.<anonymous> (/usr/local/lib/node_modules/yarn/lib/cli.js:66750:16)
    at Request.emit (node:events:524:28)
    at module.exports.Request.onRequestResponse (/usr/local/lib/node_modules/yarn/lib/cli.js:142287:10)
    at ClientRequest.emit (node:events:524:28)
    at HTTPParser.parserOnIncomingClient [as onIncoming] (node:_http_client:702:27)
    at HTTPParser.parserOnHeadersComplete (node:_http_common:118:17)
    at TLSSocket.socketOnData (node:_http_client:544:22)
    at TLSSocket.emit (node:events:524:28)
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

🫠🫠🫠

@duranbe
Copy link
Contributor Author

duranbe commented Aug 26, 2025

I'm missing write permissions to requeue, if @rix0rrr or @pahud can check help

rix0rrr
rix0rrr previously approved these changes Aug 27, 2025
Copy link
Contributor

mergify bot commented Aug 27, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot dismissed rix0rrr’s stale review August 27, 2025 20:24

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. Note that PRs with failing linting check or builds are not reviewed, please ensure your build is passing

To prevent automatic closure:

  • Resume work on the PR
  • OR request an exemption by adding a comment containing 'Exemption Request' with justification e.x "Exemption Request: "
  • OR request clarification by adding a comment containing 'Clarification Request' with a question e.x "Clarification Request: "

This PR will automatically close in 14 days if no action is taken.

@duranbe duranbe requested a review from rix0rrr September 17, 2025 09:57
Copy link
Contributor

mergify bot commented Sep 17, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 7ceaefb into aws:main Sep 17, 2025
22 checks passed
Copy link
Contributor

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants