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

chore!: deprecate AppEngineHandler and ContainerEngineHandler #298

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions docs/handlers-app-engine.rst

This file was deleted.

6 changes: 0 additions & 6 deletions docs/handlers-container-engine.rst

This file was deleted.

6 changes: 6 additions & 0 deletions docs/handlers-structured-log.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Structured Log Handler
====================================

.. automodule:: google.cloud.logging_v2.handlers.structured_log
:members:
:show-inheritance:
15 changes: 5 additions & 10 deletions docs/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -327,19 +327,14 @@ logging handler can use different transports. The default is
fluentd logging handlers
~~~~~~~~~~~~~~~~~~~~~~~~

Besides :class:`~google.cloud.logging.handlers.handlers.CloudLoggingHandler`,
which writes directly to the API, two other handlers are provided.
:class:`~google.cloud.logging.handlers.app_engine.AppEngineHandler`, which is
recommended when running on the Google App Engine Flexible vanilla runtimes
(i.e. your app.yaml contains ``runtime: python``), and
Comment on lines -332 to -334

Choose a reason for hiding this comment

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

We should mark the classes as deprecated: update the docs to mention that they are not returned anymore and that new classes replace them. Folks who have existing code using 2.* library release should have a path forward that explains how to upgrade to 3.0.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My plan was to go through and update the docs at the end of the milestone, just in case any docs changes become out of date later (#245)

Do you think that makes sense, or should I try to update the docs along-side changes?

Choose a reason for hiding this comment

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

We can update the docs later. The point here is more about the annotation of the classes.

:class:`~google.cloud.logging.handlers.container_engine.ContainerEngineHandler`
, which is recommended when running on `Google Kubernetes Engine`_ with the
Cloud Logging plugin enabled.
Two handler classes are provided: :class:`~google.cloud.logging.handlers.handlers.CloudLoggingHandler`,
which writes directly to the API, and :class:`~google.cloud.logging.handlers.structured_log.StructuredLogHandler`,
which writes logs as structured JSON to standard output (to be later parsed by the environment).

:meth:`~google.cloud.logging.client.Client.get_default_handler` and
:meth:`~google.cloud.logging.client.Client.setup_logging` will attempt to use
the environment to automatically detect whether the code is running in
these platforms and use the appropriate handler.
the environment to automatically detect where the code is running and use the
appropriate handler.

In both cases, the fluentd agent is configured to automatically parse log files
in an expected format and forward them to Cloud Logging. The handlers
Expand Down
3 changes: 1 addition & 2 deletions docs/v2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ v2
sink
stdlib-usage
handlers
handlers-app-engine
handlers-container-engine
handlers-structured-log
transports-sync
transports-thread
transports-base
4 changes: 0 additions & 4 deletions google/cloud/logging/handlers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,14 @@

"""Python :mod:`logging` handlers for Google Cloud Logging."""

from google.cloud.logging_v2.handlers.app_engine import AppEngineHandler
from google.cloud.logging_v2.handlers.container_engine import ContainerEngineHandler
from google.cloud.logging_v2.handlers.structured_log import StructuredLogHandler
from google.cloud.logging_v2.handlers.handlers import CloudLoggingFilter
from google.cloud.logging_v2.handlers.handlers import CloudLoggingHandler
from google.cloud.logging_v2.handlers.handlers import setup_logging

__all__ = [
"AppEngineHandler",
"CloudLoggingFilter",
"CloudLoggingHandler",
"ContainerEngineHandler",
"StructuredLogHandler",
"setup_logging",
]
6 changes: 2 additions & 4 deletions google/cloud/logging_v2/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@
from google.cloud.logging_v2._http import _MetricsAPI as JSONMetricsAPI
from google.cloud.logging_v2._http import _SinksAPI as JSONSinksAPI
from google.cloud.logging_v2.handlers import CloudLoggingHandler
from google.cloud.logging_v2.handlers import AppEngineHandler
from google.cloud.logging_v2.handlers import ContainerEngineHandler
from google.cloud.logging_v2.handlers import StructuredLogHandler
from google.cloud.logging_v2.handlers import setup_logging
from google.cloud.logging_v2.handlers.handlers import EXCLUDED_LOGGER_DEFAULTS
Expand Down Expand Up @@ -352,9 +350,9 @@ def get_default_handler(self, **kw):

if isinstance(monitored_resource, Resource):
Copy link
Contributor

Choose a reason for hiding this comment

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

@daniel-sanche is it possible to document the behavior of get_default_handler more thoroughly somewhere, including the default for each environment? I think some users will be initially confused why their logs are showing up in stdout and/or whichever other sinks they might be streaming their stdout logs to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this suggestion to the open issue around updating the docs for v3.

Let me know if you have thoughts about whether docs should be updated along with each change, or as a pass at the end of the milestone

if monitored_resource.type == _GAE_RESOURCE_TYPE:
return AppEngineHandler(self, **kw)
CloudLoggingHandler(self, resource=monitored_resource, **kw)
elif monitored_resource.type == _GKE_RESOURCE_TYPE:
return ContainerEngineHandler(**kw)
return StructuredLogHandler(**kw, project_id=self.project)
elif (
monitored_resource.type == _GCF_RESOURCE_TYPE
and sys.version_info[0] == 3
Expand Down
4 changes: 0 additions & 4 deletions google/cloud/logging_v2/handlers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,14 @@

"""Python :mod:`logging` handlers for Google Cloud Logging."""

from google.cloud.logging_v2.handlers.app_engine import AppEngineHandler
from google.cloud.logging_v2.handlers.container_engine import ContainerEngineHandler
from google.cloud.logging_v2.handlers.structured_log import StructuredLogHandler
from google.cloud.logging_v2.handlers.handlers import CloudLoggingHandler
from google.cloud.logging_v2.handlers.handlers import CloudLoggingFilter
from google.cloud.logging_v2.handlers.handlers import setup_logging

__all__ = [
"AppEngineHandler",
"CloudLoggingFilter",
"CloudLoggingHandler",
"ContainerEngineHandler",
"StructuredLogHandler",
"setup_logging",
]
33 changes: 0 additions & 33 deletions google/cloud/logging_v2/handlers/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

"""Helper functions for logging handlers."""

import math
import json
import re

try:
Expand All @@ -34,24 +32,6 @@
_PROTOCOL_HEADER = "SERVER_PROTOCOL"


def format_stackdriver_json(record, message):
"""Helper to format a LogRecord in in Stackdriver fluentd format.

Returns:
str: JSON str to be written to the log file.
"""
subsecond, second = math.modf(record.created)

payload = {
"message": message,
"timestamp": {"seconds": int(second), "nanos": int(subsecond * 1e9)},
"thread": record.thread,
"severity": record.levelname,
}

return json.dumps(payload, ensure_ascii=False)


def get_request_data_from_flask():
"""Get http_request and trace data from flask request headers.

Expand All @@ -68,10 +48,7 @@ def get_request_data_from_flask():
http_request = {
"requestMethod": flask.request.method,
"requestUrl": flask.request.url,
"requestSize": flask.request.content_length,
"userAgent": flask.request.user_agent.string,
"remoteIp": flask.request.remote_addr,
"referer": flask.request.referrer,
"protocol": flask.request.environ.get(_PROTOCOL_HEADER),
}

Expand All @@ -96,21 +73,11 @@ def get_request_data_from_django():
if request is None:
return None, None, None

# convert content_length to int if it exists
content_length = None
try:
content_length = int(request.META.get(_DJANGO_CONTENT_LENGTH))
except (ValueError, TypeError):
content_length = None

# build http_request
http_request = {
"requestMethod": request.method,
"requestUrl": request.build_absolute_uri(),
"requestSize": content_length,
"userAgent": request.META.get(_DJANGO_USERAGENT_HEADER),
"remoteIp": request.META.get(_DJANGO_REMOTE_ADDR_HEADER),
"referer": request.META.get(_DJANGO_REFERER_HEADER),
"protocol": request.META.get(_PROTOCOL_HEADER),
}

Expand Down
131 changes: 0 additions & 131 deletions google/cloud/logging_v2/handlers/app_engine.py

This file was deleted.

54 changes: 0 additions & 54 deletions google/cloud/logging_v2/handlers/container_engine.py

This file was deleted.

Loading