-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add support for host logging, deprecating old topics_logger in favor of host-managed topics logger. updates examples. #21
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
Conversation
…favor of host-managed topics logger. updates examples.
|
|
||
| fn log(&self, record: &log::Record) { | ||
| if self.enabled(record.metadata()) { | ||
| let mut buffer = String::with_capacity(128); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder about this length restriction for the host-logging approach. Is it still appropriate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a restriction, it's an initial capacity to help avoid excessive reallocation. That said, we could reach for unsafe here too and reuse the buffer given the single-thread nature of Functions... Let's leave that for another time though :-)
kvcache
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly just have configuration quality of life feedback, which I think is pretty important ffor logging configuration!
|
|
||
| fn log(&self, record: &log::Record) { | ||
| if self.enabled(record.metadata()) { | ||
| let mut buffer = String::with_capacity(128); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a restriction, it's an initial capacity to help avoid excessive reallocation. That said, we could reach for unsafe here too and reuse the buffer given the single-thread nature of Functions... Let's leave that for another time though :-)
Adds
.witbindings for our host-supported log destinations. Deprecates oldtopic_loggerin favor of the new one. Will be merged when host support is merged.This also adds support for shipping logs to a desired CloudWatch Logs Group as well as splitting out specific log levels per destination. For example, you can set all logs
DEBUGand higher to a topic, but onlyINFOand higher to a CW Logs Group.Testing
Verified this works in development cell. Logging example correctly sent logs to my topic, including system logs (when specified).
Breaking changes
momento_functions_log::configure_loggingnow receives aVec<ConfigureLoggingInput>so multiple destinations can be configured at oncemomento_functions_log::configure_loggingnow returns aResult<(), LogConfigError>instead ofSetLoggerErrorTopicLoggeris no longer supported; deprecated in favor ofLogDestination::Topic