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/' and write logs to Stackdriver logging API #3410

Merged
merged 6 commits into from
May 17, 2017

Conversation

liyanhui1228
Copy link
Contributor

@liyanhui1228 liyanhui1228 commented May 12, 2017

For #2997 . Modified the AppEngineHandler to extend the CloudLoggingHandler and directly write logs to Stackdriver Logging API.

Screenshot of the gae_app log with AppEngineHandler:
test_gae_app_logging

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 12, 2017
transport=BackgroundThreadTransport,
resource=_GLOBAL_RESOURCE):
super(AppEngineHandler, self).__init__(client, name, transport)
self.resource=resource

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@@ -130,14 +132,14 @@ def _stop(self):
self._stop_condition.release()
self.stopped = True

def enqueue(self, record, message):
def enqueue(self, record, message, resource=_GLOBAL_RESOURCE):

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.


class AppEngineHandler(logging.handlers.RotatingFileHandler):
"""A handler that writes to the App Engine fluentd Stackdriver log file.
_GLOBAL_RESOURCE = Resource(type='global', labels={})

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.


EXCLUDED_LOGGER_DEFAULTS = ('google.cloud', 'oauth2client')

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

_LOG_PATH_TEMPLATE = '/var/log/app_engine/app.{pid}.json'
_MAX_LOG_BYTES = 128 * 1024 * 1024
_LOG_FILE_COUNT = 3
DEFAULT_LOGGER_NAME = 'python'

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.


class AppEngineHandler(logging.handlers.RotatingFileHandler):
"""A handler that writes to the App Engine fluentd Stackdriver log file.
_GLOBAL_RESOURCE = Resource(type='global', labels={})

This comment was marked as spam.


EXCLUDED_LOGGER_DEFAULTS = ('google.cloud', 'oauth2client')

This comment was marked as spam.

transport=BackgroundThreadTransport,
resource=_GLOBAL_RESOURCE):
super(AppEngineHandler, self).__init__(client, name, transport)
self.resource=resource

This comment was marked as spam.

message = super(AppEngineHandler, self).format(record)
return format_stackdriver_json(record, message)
message = super(CloudLoggingHandler, self).format(record)
self.transport.send(record, message, self.resource)

This comment was marked as spam.

This comment was marked as spam.

@@ -130,14 +132,14 @@ def _stop(self):
self._stop_condition.release()
self.stopped = True

def enqueue(self, record, message):
def enqueue(self, record, message, resource=_GLOBAL_RESOURCE):

This comment was marked as spam.

@liyanhui1228 liyanhui1228 force-pushed the yanhuil/2997 branch 2 times, most recently from f990734 to 5b79d48 Compare May 13, 2017 00:15
@liyanhui1228
Copy link
Contributor Author

Ready for another review, but still wait for Bill's comment about the default logName.

@waprin
Copy link
Contributor

waprin commented May 13, 2017

You are right that shutdown part of logName is not quite right, I copied and pasted it from somewhere else. I am not sure exactly what the logName should be to be honest, I think it doesn't matter that much because the resource and labels are what puts it in the right place. I would guess its "projects/coinflow-demo/logs/appengine.googleapis.com%2Fapp" ? Might be worth pinging AppEngine or Logging people who can answer more confidently since I'm just guessing based on what I've seen a few other places.

@theacodes
Copy link
Contributor

theacodes commented May 13, 2017 via email

@tmatsuo
Copy link
Contributor

tmatsuo commented May 13, 2017

FWIW, I'd recommend that you use app for the default logName. The logs with the correct monitored resource and if the logName is "projects/PROJECT_ID/logs/app", then it's available as "app" in the Logging UI drop down menu. PHP logging library is using app as the default logName.


EXCLUDED_LOGGER_DEFAULTS = ('google.cloud', 'oauth2client')

This comment was marked as spam.

GAE_RESOURCE = Resource(
type='gae_app',
labels={
'project_id': os.getenv('GCLOUD_PROJECT'),

This comment was marked as spam.

This comment was marked as spam.

def __init__(self, client,
name=DEFAULT_LOGGER_NAME,
transport=BackgroundThreadTransport,
resource=GAE_RESOURCE):

This comment was marked as spam.

This comment was marked as spam.

_LOG_PATH_TEMPLATE = '/var/log/app_engine/app.{pid}.json'
_MAX_LOG_BYTES = 128 * 1024 * 1024
_LOG_FILE_COUNT = 3
DEFAULT_LOGGER_NAME = 'python'

This comment was marked as spam.

@@ -29,7 +33,7 @@ class SyncTransport(Transport):
def __init__(self, client, name):
self.logger = client.logger(name)

def send(self, record, message):
def send(self, record, message, resource=_GLOBAL_RESOURCE):

This comment was marked as spam.

This comment was marked as spam.

def test_ctor(self):
client = _Client(self.PROJECT)
handler = self._make_one(client, transport=_Transport)
self.assertEqual(handler.client, client)

This comment was marked as spam.

This comment was marked as spam.

@@ -46,20 +47,28 @@ def test_constructor(self):
self.assertEqual(logger.name, name)

def test_send(self):
RESOURCE = Resource(

This comment was marked as spam.

This comment was marked as spam.

@@ -36,6 +38,13 @@ def test_ctor(self):
self.assertEqual(transport.logger.name, 'python_logger')

def test_send(self):
RESOURCE = Resource(

This comment was marked as spam.

This comment was marked as spam.

@@ -14,6 +14,8 @@

import logging
import unittest
from google.cloud.logging.resource import Resource

This comment was marked as spam.

This comment was marked as spam.

@tseaver tseaver added the api: logging Issues related to the Cloud Logging API. label May 15, 2017
@@ -14,60 +14,60 @@

"""Logging handler for App Engine Flexible

Logs to the well-known file that the fluentd sidecar container on App Engine
Flexible is configured to read from and send to Stackdriver Logging.
Send logs to Stackdriver Logging API.

This comment was marked as spam.

This comment was marked as spam.

# This file is largely copied from:
# https://github.com/GoogleCloudPlatform/python-compat-runtime/blob/master
# /appengine-vmruntime/vmruntime/cloud_logging.py
GAE_PROJECT_ENV = 'GCLOUD_PROJECT'

This comment was marked as spam.

This comment was marked as spam.

message = super(AppEngineHandler, self).format(record)
return format_stackdriver_json(record, message)
def __init__(self, client,
name=EXCLUDED_LOGGER_DEFAULTS,

This comment was marked as spam.

This comment was marked as spam.

super(CloudLoggingHandler, self).__init__()
self.name = name
self.client = client
self.transport = transport(client, name)
self.resource=resource

This comment was marked as spam.

@@ -90,7 +96,7 @@ def emit(self, record):
:param record: The record to be logged.
"""
message = super(CloudLoggingHandler, self).format(record)
self.transport.send(record, message)
self.transport.send(record, message, self.resource)

This comment was marked as spam.

"""
self.worker.enqueue(record, message)
self.worker.enqueue(record, message, resource)

This comment was marked as spam.

labels={
'project_id': os.environ.get(GAE_PROJECT_ENV),
'module_id': os.environ.get('GAE_SERVICE'),
'version_id': os.environ.get('GAE_VERSION'),

This comment was marked as spam.

This comment was marked as spam.

PROJECT = 'PROJECT'
from google.cloud.logging.resource import Resource

GAE_PROJECT_ENV = 'PROJECT'

This comment was marked as spam.

client = _Client(self.GAE_PROJECT_ENV)
handler = self._make_one(client, transport=_Transport)
self.assertEqual(handler.client, client)
self.assertEqual(handler.resource, self.GAE_RESOURCE)

This comment was marked as spam.

This comment was marked as spam.

@tmatsuo
Copy link
Contributor

tmatsuo commented May 16, 2017

Re: default logName

I recommended that we use app, but actually it can be anything. The request correlation feature just works with any logName as long as they have the same monitoredResource and the traceId. Both app and python are natural choice (shutdown is not though).

Copy link
Contributor

@theacodes theacodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks almost good to me, @dhermes can you take a look?

class AppEngineHandler(logging.handlers.RotatingFileHandler):
"""A handler that writes to the App Engine fluentd Stackdriver log file.
class AppEngineHandler(CloudLoggingHandler):
"""A handler that directly makes Stackdriver logging API calls.

This comment was marked as spam.


def __init__(self):
"""Construct the handler
This handler can be used to route Python standard logging messages directly

This comment was marked as spam.

super(AppEngineHandler, self).__init__(self.filename,
maxBytes=_MAX_LOG_BYTES,
backupCount=_LOG_FILE_COUNT)
This handler supports both an asynchronous and synchronous transport.

This comment was marked as spam.

:param transport: Class for creating new transport objects. It should
extend from the base :class:`.Transport` type and
implement :meth`.Transport.send`. Defaults to
:class:`.BackgroundThreadTransport`. The other

This comment was marked as spam.

"""
message = super(AppEngineHandler, self).format(record)
return format_stackdriver_json(record, message)
DEFAULT_LOGGER_NAME = 'app'

This comment was marked as spam.


def __init__(self, client,
transport=BackgroundThreadTransport):
super(AppEngineHandler, self).__init__(client, name=self.DEFAULT_LOGGER_NAME,

This comment was marked as spam.

transport=transport, resource=self.gae_resource)

@property
def gae_resource(self):

This comment was marked as spam.

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels May 16, 2017
@liyanhui1228
Copy link
Contributor Author

@dhermes @jonparrott Ready for another review or merge.

@dhermes
Copy link
Contributor

dhermes commented May 17, 2017

I just re-built CircleCI, let's get it green (I realize the failure was due to error_reporting)

@liyanhui1228
Copy link
Contributor Author

OK!

@duggelz duggelz removed their request for review May 17, 2017 18:40
"""
message = super(AppEngineHandler, self).format(record)
return format_stackdriver_json(record, message)

This comment was marked as spam.

This comment was marked as spam.

@@ -52,6 +53,9 @@ class CloudLoggingHandler(logging.StreamHandler):
:class:`.BackgroundThreadTransport`. The other
option is :class:`.SyncTransport`.

:type resource: :class:`~google.cloud.logging.resource.Resource`
:param resource: (Optional) Monitored resource of the entry

This comment was marked as spam.

from google.cloud._testing import _Monkey
from google.cloud.logging.handlers import app_engine as _MUT
def test_constructor(self):
import mock

This comment was marked as spam.

This comment was marked as spam.

class _Transport(object):

def __init__(self, client, name):
pass

This comment was marked as spam.

@@ -278,14 +281,15 @@ def join(self, timeout=None):


class _Batch(object):
from google.cloud.logging.logger import _GLOBAL_RESOURCE

This comment was marked as spam.

def log_struct(self, info, severity=logging.INFO):
self.log_struct_called_with = (info, severity)
def log_struct(self, info, severity=logging.INFO, resource=_GLOBAL_RESOURCE):
self.log_struct_called_with = (info, severity, resource)

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor

dhermes commented May 17, 2017

@liyanhui1228 Looks like you --force pushed over some changes I made yesterday? Pre #3422, the build will fail, so I rebased your branch. There was also a tiny docs error that I made a commit to resolve (error doesn't seem to be present any longer, just ran docs locally).

@liyanhui1228
Copy link
Contributor Author

Yeah did you rebase my branch just now? I didn't find it in git log.

@dhermes
Copy link
Contributor

dhermes commented May 17, 2017

@liyanhui1228 No I didn't do it just now, but a rebase won't show up in your logs (other than the fact that all the commit hashes will be different and the first commit will have a new parent)

@dhermes
Copy link
Contributor

dhermes commented May 17, 2017

This LGTM. We should wait until we can get a green build? WDYT @jonparrott?

@theacodes
Copy link
Contributor

We should wait until we can get a green build?

Yes. I'll give it another review while we wait for CI.

@theacodes
Copy link
Contributor

@dhermes bigquery system tests are failing?

@theacodes
Copy link
Contributor

Confirmed with @dhermes over chat that bigquery tests are borked - merging based on local verification of tests.

@theacodes theacodes merged commit cce9c99 into master May 17, 2017
@theacodes theacodes deleted the yanhuil/2997 branch May 17, 2017 22:15
@dhermes
Copy link
Contributor

dhermes commented May 17, 2017

They are green sometimes but I've seen them fail 4+ times on this PR.

@tseaver
Copy link
Contributor

tseaver commented May 19, 2017

@jonparrott It looks like you merged this PR with failing coverage in the logging package:

$ cd logging
$ nox
...
Name                                                            Stmts   Miss Branch BrPart  Cover   Missing
-----------------------------------------------------------------------------------------------------------
google/cloud/logging/__init__.py                                    7      0      0      0   100%
google/cloud/logging/_gax.py                                      190      0     28      0   100%
google/cloud/logging/_helpers.py                                   12      0      6      0   100%
google/cloud/logging/_http.py                                      98      0     16      0   100%
google/cloud/logging/client.py                                     79      0     20      0   100%
google/cloud/logging/entries.py                                    57      0     10      0   100%
google/cloud/logging/handlers/__init__.py                           6      0      0      0   100%
google/cloud/logging/handlers/_helpers.py                           7      0      0      0   100%
google/cloud/logging/handlers/app_engine.py                        15      0      0      0   100%
google/cloud/logging/handlers/container_engine.py                   7      0      0      0   100%
google/cloud/logging/handlers/handlers.py                          26      0      2      0   100%
google/cloud/logging/handlers/transports/__init__.py                5      0      0      0   100%
google/cloud/logging/handlers/transports/background_thread.py      98      0     24      0   100%
google/cloud/logging/handlers/transports/base.py                    5      0      0      0   100%
google/cloud/logging/handlers/transports/sync.py                    8      0      0      0   100%
google/cloud/logging/logger.py                                    116      0     50      0   100%
google/cloud/logging/metric.py                                     46      0      2      0   100%
google/cloud/logging/resource.py                                    8      0      0      0   100%
google/cloud/logging/sink.py                                       46      0      2      0   100%
tests/unit/__init__.py                                              0      0      0      0   100%
tests/unit/handlers/__init__.py                                     0      0      0      0   100%
tests/unit/handlers/test_app_engine.py                             40      0      0      0   100%
tests/unit/handlers/test_container_engine.py                       19      0      0      0   100%
tests/unit/handlers/test_handlers.py                               60      0      2      0   100%
tests/unit/handlers/transports/__init__.py                          0      0      0      0   100%
tests/unit/handlers/transports/test_background_thread.py          200      0      4      1    99%   293->296
tests/unit/handlers/transports/test_base.py                        15      0      0      0   100%
tests/unit/handlers/transports/test_sync.py                        42      0      0      0   100%
tests/unit/test__gax.py                                           989      0     48      0   100%
tests/unit/test__helpers.py                                        32      0      0      0   100%
tests/unit/test__http.py                                          481      0      2      0   100%
tests/unit/test_client.py                                         375      0      0      0   100%
tests/unit/test_entries.py                                        190      0      0      0   100%
tests/unit/test_logger.py                                         506      0      0      0   100%
tests/unit/test_metric.py                                         152      0      0      0   100%
tests/unit/test_sink.py                                           153      0      0      0   100%
-----------------------------------------------------------------------------------------------------------
TOTAL                                                            4090      0    216      1    99%
nox > Command coverage report --show-missing --fail-under=100 failed with exit code 2
nox > Session cover failed. :(
nox > Ran multiple sessions:
nox > * unit_tests(python_version='2.7'): passed
nox > * unit_tests(python_version='3.4'): passed
nox > * unit_tests(python_version='3.5'): passed
nox > * unit_tests(python_version='3.6'): passed
nox > * system_tests(python_version='2.7'): passed
nox > * system_tests(python_version='3.6'): passed
nox > * lint: passed
nox > * lint_setup_py: passed
nox > * cover: failed

We need to quit running tests for the unaffected packages ASAP. Having BQ back-end slow down shouldn't break our logging CI.

@theacodes
Copy link
Contributor

theacodes commented May 19, 2017 via email

@tseaver
Copy link
Contributor

tseaver commented May 19, 2017

I just merged #3445, which fixes the immediate issue (by suppressing the uncovered branch nested down in the tests).

@theacodes
Copy link
Contributor

theacodes commented May 19, 2017 via email

parthea pushed a commit that referenced this pull request Oct 21, 2023
…tform/python-docs-samples#3410)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [google-cloud-kms](https://togithub.com/googleapis/python-kms) | minor | `==1.3.0` -> `==1.4.0` |

---

### Release Notes

<details>
<summary>googleapis/python-kms</summary>

### [`v1.4.0`](https://togithub.com/googleapis/python-kms/blob/master/CHANGELOG.md#&#8203;140-httpswwwgithubcomgoogleapispython-kmscomparev130v140-2020-04-14)

[Compare Source](https://togithub.com/googleapis/python-kms/compare/v1.3.0...v1.4.0)

##### Features

-   add support for external key manager (via synth) ([#&#8203;8](https://www.github.com/googleapis/python-kms/issues/8)) ([4077fc8](https://www.github.com/googleapis/python-kms/commit/4077fc89943cc09d489d44c05efcf9cab61cdbaf))

</details>

---

### Renovate configuration

:date: **Schedule**: At any time (no schedule defined).

:vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

:recycle: **Rebasing**: Never, or you tick the rebase/retry checkbox.

:no_bell: **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#GoogleCloudPlatform/python-docs-samples).
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. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants