feat: use standard output logs on serverless environments #228
Conversation
product-auto-label
bot
added
the
api: logging
google-cla
bot
added
the
cla: yes
daniel-sanche
changed the title
| # make logs appear in GCP structured logging format | ||
| self.formatter = logging.Formatter(GCP_FORMAT) | ||
|
|
||
| def format(self, record): |
There was a problem hiding this comment.
don't you need to check that record is not null?
There was a problem hiding this comment.
That shouldn't be necessary. We're just delegating the formatting work to another standard Python formatter object. If there were any error cases to worry about, the function we're calling can handle it.
| if monitored_resource.type == _GAE_RESOURCE_TYPE: | ||
| return AppEngineHandler(self, **kw) | ||
| elif monitored_resource.type == _GKE_RESOURCE_TYPE: | ||
| return ContainerEngineHandler(**kw) |
There was a problem hiding this comment.
how is ContainerEngineHandler different from StructuredLogHandler?
There was a problem hiding this comment.
They are essentially the same, but I'm going to leave ContainerEngineHandler in for now for API consistency. I plan on removing ContainerEngineHandler and AppEngineHandler in v3.0.0 in favor of more general classes
| "AppEngineHandler", | ||
| "CloudLoggingHandler", | ||
| "ContainerEngineHandler", | ||
| "StructuredLogHandler", | ||
| "setup_logging", |
There was a problem hiding this comment.
Worth adding a comment to explain semantics of functionality that each handler adds.
There was a problem hiding this comment.
I don't believe it's standard to use __init__.py for documentation.This is just an internal file used for importing packages. It's likely to get out of date if things change in the future, so I'd prefer to keep documentation alongside code
| record.trace = getattr(record, "trace", inferred_trace) or "" | ||
| record.http_request = getattr(record, "http_request", inferred_http) or {} | ||
| record.request_method = record.http_request.get("requestMethod", "") | ||
| record.request_url = record.http_request.get("requestUrl", "") | ||
| record.user_agent = record.http_request.get("userAgent", "") | ||
| record.protocol = record.http_request.get("protocol", "") |
There was a problem hiding this comment.
are you planning to do anything about log severity too?
There was a problem hiding this comment.
This "Filter" class is to add new fields to the LogRecord object, that we can then use later when exporting logs in the Handler classes. Common fields like "severity" already exist in the LogRecord by default, so no need to add them here
There was a problem hiding this comment.
are you planning to add other httprequest fields from here as well. Nonblocking comment.
There was a problem hiding this comment.
eventually, but some of them are more complicated, and I want to make sure they work consistently across environments. I want to put in the basics first
|
|
||
|
|
||
| class StructuredLogHandler(logging.StreamHandler): | ||
| """Handler to format logs into the Cloud Logging structured log format, |
There was a problem hiding this comment.
Maybe add a link to https://cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry#LogSeverity or another appropriate link, so they know "structured log format" is a formally defined hting
I have an open bug for re-doing the samples/usage guide in v3.0.0. I expect more changes before then |
We have had bug reports that some logs aren't getting through when using the library in serverless environments, because the environment will spin down before logs are flushed.
To fix this, we can log to standard output in JSON format, and allow the environment's native log handling code to extract the logs to Stackdriver.
This method is already used for GKE. Instead of reusing the ContainerEngineHandler, we will create a new more general StructuredLogHandler, and deprecate ContainerEngineHandler at the next major relase
Merging this branch into v2_update_2 to group work for current milestone.
#226