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

IoHost used as only logger everywhere #32346

Closed
mrgrain opened this issue Dec 1, 2024 · 2 comments
Closed

IoHost used as only logger everywhere #32346

mrgrain opened this issue Dec 1, 2024 · 2 comments
Assignees
Labels

Comments

@mrgrain
Copy link
Contributor

mrgrain commented Dec 1, 2024

With #32345 completed, we can move to using the IoHost.

  • create a global singleton CliIoHost
  • change every log statement to use the new CliIoHost
  • remove all other logger implementations, instead every call must call ioHost.notify(...)
  • We cannot have helpers to call notify
  • However we can have helper methods to create IoMessages of the correct type, i.e. ioHost.notify(error("momo made a boo boo")) is fine

This should not require any changes to tests. If so, explain why.

@khushail khushail added the p2 label Dec 23, 2024
HBobertz added a commit that referenced this issue Jan 14, 2025
### Related to #32346 

### Reason for this change

Capture all logs across the CLI as IoMessages so that when we allow
customers to use the CDK Core functionality programmatically, these logs
are delivered to the customers as messages and now just swallowed across
the CLI.

### Description of changes

Prevented access to `log()` and `formatLogMessage()`. Now the only ways
to log something properly are either through the exported log functions
(which is how everyone was doing it anyway), or through
`CliIoHost.notify()` directly.

CliIoHost is now exposed as a global singleton which is currently only
directly exclusively used in `logging.ts` but is effectively used
everywhere as all logging functions inevitably call `CliIoHost.notify()`

All logging functions now optionally support the following input types
```ts
error(`operation failed: ${e}`)                                 // infers default error code `TOOLKIT_0000`
error('operation failed: %s', e)                                // infers default error code `TOOLKIT_0000`
error({ message: 'operation failed', code: 'SDK_0001' })        // specifies error code `SDK_0001`
error({ message: 'operation failed: %s', code: 'SDK_0001' }, e) // specifies error code `SDK_0001`
```
and everything is now translated into an `IoMessage` and calls
`CliIoHost.notify()` from these logging functions. Nothing currently
specifies any message code so it's all the generic _0000, _1000, _2000

### Description of how you validated changes

added and updated unit tests


### Checklist
- [x] My code adheres to the [CONTRIBUTING
GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and
[DESIGN
GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

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

---------

Co-authored-by: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
Co-authored-by: Momo Kornher <kornherm@amazon.co.uk>
@mrgrain mrgrain closed this as completed Jan 15, 2025
Copy link

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 Jan 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants