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

Stop writing to /var/log/app_engine/ #2997

Closed
JustinBeckwith opened this issue Feb 10, 2017 · 9 comments
Closed

Stop writing to /var/log/app_engine/ #2997

JustinBeckwith opened this issue Feb 10, 2017 · 9 comments
Assignees
Labels
api: logging Issues related to the Cloud Logging API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: question Request for information or clarification. Not an issue.

Comments

@JustinBeckwith
Copy link
Contributor

JustinBeckwith commented Feb 10, 2017

Currently the logging library is writing files to a magical path: /var/log/app_engine/. This is not an official supported path, and would only work for App Engine. At any point -that path could stop working.

The only officially supported ways to write logs are to write to stdout or stderr, or to use the cloud logging APIs. Come chat with me if you have questions!

https://github.com/GoogleCloudPlatform/google-cloud-python/blob/master/logging/google/cloud/logging/handlers/app_engine.py#L34

@theacodes
Copy link
Contributor

@waprin can you take this?

@daspecster daspecster added the api: logging Issues related to the Cloud Logging API. label Feb 10, 2017
@waprin
Copy link
Contributor

waprin commented Feb 10, 2017

@JustinBeckwith it's ok if it only works on App Engine because it's only supposed to be used on App Engine.

stdout/stderr don't capture severity which this was supposed to address.

We can switch it to use the API directly, just need to make sure correct monitored resource gets set.

@danoscarmike danoscarmike added Status: Acknowledged type: question Request for information or clarification. Not an issue. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Feb 28, 2017
@lukesneeringer
Copy link
Contributor

@waprin Can you provide me with some clarity on this? Does this need to be fixed, or is it working as intended?

@waprin
Copy link
Contributor

waprin commented Feb 28, 2017

@lukesneeringer

tl;dr: It's working, but via an unofficially supported mechanism, and so it needs to be fixed, which I can do.

Longer story:

These are for the Python standard logging handlers, which is not the main API interface we document, it's just a helper that I added. I documented it under "Additional tools" in the Cloud docs and a section at the bottom of these Github docs.

As mentioned, on App Engine Flex, stdout/stderr ends up in Stackdriver Logging, but the reason I just don't like logging to that (which is where Python standard logging goes by default), is because the fluentd logger doesn't capture error severity levels. So if you try to search for messages logged at severity error the best you can do is a text search for the word error, and you will get all message that have the word error in them regardless of their log level. It's a small detail but I think a nice one to get right. So the current handler tries to format the messages so the fluentd agent parses the severity correctly. Justin is telling me that I shouldn't rely on the fluentd agent at all and should just write directly to the API.

The only small details to doing that right is to correctly set the monitored resource so the logs show up in the correct App Engine service/version logs rather than in custom global logs.

I can submit a PR to fix it, currently a few things ahead in my queue, maybe this week but if not then after Next is over (next week). @JustinBeckwith can let me know if it's higher priority than I'm realizing.

@lukesneeringer
Copy link
Contributor

@waprin That sounds good. Thank you so much.

@duggelz
Copy link

duggelz commented May 2, 2017

I had always understood that we would tell users to always use the standard Python logging, and never use the API-type functions from this library other than a setup call in their main(), and we'll do what it takes to make it work well (and printing to stdout/stderr is not that). That's what I've been telling others, and what I've been telling the ABSL people who are open-sourcing the internal Google logging code under a similar principle -- call a setup function once, then just use standard Python logging calls, and we'll make it do the right thing.

@waprin
Copy link
Contributor

waprin commented May 2, 2017

@duggelz yes, the issue is that client.setup_logging which uses standard Python logging, currently writes to /var/log/appengine, we just need to hook to make it use the regular API client instead. And then we can put GAE stuff on top of that afterwards.

@waprin
Copy link
Contributor

waprin commented May 11, 2017

from google.cloud.logging.resource import Resource
resource = Resource(
    'gae_app', 
    labels={
        'module_id': os.getenv(GAE_SERVICE),
        'version': os.getenv(GAE_VERSION),
    },
)
logger.log_text(message, resource=resource)

is the basic call you're going to want to end up making. Currently the handler in handlers.app_engine extends RotatingFileHandler but you're going to want to instead make it either extend or something similar to handlers.handlers.CloudLoggingHandler. However the actual log_text or log_struct call is made in SyncTransport and AsyncTransport send method. So you probably want to add a resource flag to the transport send interface and then have the gae handler pass the resource in.

Of course up to you how you do it just trying to brain dump my perspective on what needs to get done, feel free to ping me any questions.

@theacodes
Copy link
Contributor

Closed by #3410

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the Cloud Logging API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

7 participants