Inherit log severity on parent log entry from highest level child entry

[jer-tx]

I'm using the Pyramid framework under appengine flexible.

I've mostly nailed down the grouping issue to the /google/cloud/logging/handlers/_helpers.py file missing support for Pyramid. As such, I recreated the AppengineLogHandler.py file in my own codebase and made the following change to get_gae_labels():

request = pyramid.threadlocal.get_current_request()
if request:
    header = request.headers.get('X-Cloud-Trace-Context')
    if header:
        gae_labels[_TRACE_ID_LABEL] = header.split("/", 1)[0]

Then, where I initialize the client, I have:

import google.cloud.logging 
from google.cloud.logging.handlers import setup_logging

client = google.cloud.logging.Client()
handler = AppEngineHandler()
setup_logging(handler, log_level=logging.INFO)

This helps with tying log entries to an actual request.

Is your feature request related to a problem? Please describe.
Sort of. Log entries seem to be grouping properly but in possibly a weird way. I've posted in detail what I'm running into in a google group for the pyramid framework here: https://groups.google.com/g/pylons-discuss/c/WWe094nnmWo/m/kes2YLpZAQAJ

In summary, I'm able to group log entries, but the parent log entry does not reflect child log entry levels, which makes Logging in its entirely unusable, since I wouldn't be able to filter on errors or warnings.

Additionally, when logging via appengine Standard, child log entries appear instantly, as if they were stored on the parent log entry itself as a whole. From a flexible instance, its almost as if expanding a parent log entry needs to first query the logs for any matching trace ID.

Describe the solution you'd like
Parent log entry level should reflect the highest level of child log entries.

[daniel-sanche]

Parent log entry level should reflect the highest level of child log entries.

Thanks for the report. I did some research into the expected behaviour for this, and it seems like other languages are implemented the same way (see go's docs here). It's important that we have some consistency across the client libraries, so for now I'd suggest following go's suggestion of manually updating the parent severity with the child.

I don't really like that answer though, and this seems like a poorly documented feature. I opened some internal tickets around this to see if we can address it/better document it at a cross-language level. I'll update here if there are any updates on the tickets

[jer-tx]

Thanks for the details.

I'm curious, what's ultimately so different here vs Standard's implementation? It seems to me that what appengine Standard does is not send the entire log entry (with child entries) to stackdriver (or not log to stdout) until the request is entirely completed, hence why it's immediately aware of the highest severity child log.

I'll give that Go implementation a shot though, seems like the end result will be what Im looking for anyway. I'd love to hear back on any updates though :)

edit: scratch that, I misread a bit. the only thing in the Go docs relevant here is a single line "you must update the
parent severity yourself.". To be honest, this really should be handled out of the box (along with grouping).

[daniel-sanche]

Yeah, I agree there is a lot of room for improvement. Unfortunately our team is small and we're focusing on improving compatibility with the newer GCP environments, so we likely won't get to this type of fix in the next quarter or two. It seems like severity inheritance might be better handled at the backend library rather than in the library anyway, so I opened internal bugs around that and will update here if there is any news

As for the grouping, that should happen automatically by capturing traces on AppEngine for Flask and Django, but Pyramid isn't yet supported. I opened a new feature request to add it, but if you have the experience and the bandwidth, we welcome contributors as well :)

[daniel-sanche]

After discussing this more internally, it looks like the parent log is not expected to automatically inherit its severity from child logs, so I'm going to close this for now. (But feel free to re-open if you think I misunderstood)

I do hope to add better pyramid support in #139 when possible though. If trace_id and http_request data is populated, it should be a lot easier to work with