chore!: deprecate AppEngineHandler and ContainerEngineHandler (#310)

Author: daniel-sanche

google/cloud/logging_v2/client.py

@@ -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
@@ -352,9 +350,9 @@ def get_default_handler(self, **kw):
 
         if isinstance(monitored_resource, Resource):
             if monitored_resource.type == _GAE_RESOURCE_TYPE:
-                return AppEngineHandler(self, **kw)
+                return 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

google/cloud/logging_v2/handlers/_helpers.py

@@ -17,6 +17,7 @@
 import math
 import json
 import re
+import warnings
 
 try:
     import flask
@@ -39,6 +40,8 @@ def format_stackdriver_json(record, message):
 
     Returns:
         str: JSON str to be written to the log file.
+
+    DEPRECATED:  use StructuredLogHandler to write formatted logs to standard out instead.
     """
     subsecond, second = math.modf(record.created)
 
@@ -48,7 +51,10 @@ def format_stackdriver_json(record, message):
         "thread": record.thread,
         "severity": record.levelname,
     }
-
+    warnings.warn(
+        "format_stackdriver_json is deprecated. Use StructuredLogHandler instead.",
+        DeprecationWarning,
+    )
     return json.dumps(payload, ensure_ascii=False)
 
 
@@ -68,10 +74,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),
     }
 
@@ -96,21 +99,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),
     }
 

google/cloud/logging_v2/handlers/app_engine.py

@@ -20,6 +20,7 @@
 
 import logging
 import os
+import warnings
 
 from google.cloud.logging_v2.handlers._helpers import get_request_data
 from google.cloud.logging_v2.handlers._monitored_resources import (
@@ -36,9 +37,14 @@
 
 _TRACE_ID_LABEL = "appengine.googleapis.com/trace_id"
 
+_DEPRECATION_MSG = "AppEngineHandler is deprecated. Use CloudLoggingHandler instead."
+
 
 class AppEngineHandler(logging.StreamHandler):
-    """A logging handler that sends App Engine-formatted logs to Stackdriver."""
+    """A logging handler that sends App Engine-formatted logs to Stackdriver.
+
+    DEPRECATED:  use CloudLoggingHandler instead.
+    """
 
     def __init__(
         self,
@@ -71,6 +77,8 @@ def __init__(
         self.version_id = os.environ.get(_GAE_VERSION_ENV, "")
         self.resource = self.get_gae_resource()
 
+        warnings.warn(_DEPRECATION_MSG, DeprecationWarning)
+
     def get_gae_resource(self):
         """Return the GAE resource using the environment variables.
 

google/cloud/logging_v2/handlers/container_engine.py

@@ -20,15 +20,22 @@
 """
 
 import logging.handlers
+import warnings
 
 from google.cloud.logging_v2.handlers._helpers import format_stackdriver_json
 
+_DEPRECATION_MSG = (
+    "ContainerEngineHandler is deprecated. Use StructuredLogHandler instead."
+)
+
 
 class ContainerEngineHandler(logging.StreamHandler):
     """Handler to format log messages the format expected by GKE fluent.
 
     This handler is written to format messages for the Google Container Engine
     (GKE) fluentd plugin, so that metadata such as log level are properly set.
+
+    DEPRECATED:  use StructuredLogHandler to write formatted logs to standard out instead.
     """
 
     def __init__(self, *, name=None, stream=None):
@@ -40,6 +47,7 @@ def __init__(self, *, name=None, stream=None):
         """
         super(ContainerEngineHandler, self).__init__(stream=stream)
         self.name = name
+        warnings.warn(_DEPRECATION_MSG, DeprecationWarning)
 
     def format(self, record):
         """Format the message into JSON expected by fluentd.

google/cloud/logging_v2/handlers/handlers.py

@@ -33,8 +33,15 @@
     "werkzeug",
 )
 
+"""These environments require us to remove extra handlers on setup"""
 _CLEAR_HANDLER_RESOURCE_TYPES = ("gae_app", "cloud_function")
 
+"""Extra trace label to be added on App Engine environments"""
+_GAE_TRACE_ID_LABEL = "appengine.googleapis.com/trace_id"
+
+"""Resource name for App Engine environments"""
+_GAE_RESOURCE_TYPE = "gae_app"
+
 
 class CloudLoggingFilter(logging.Filter):
     """Python standard ``logging`` Filter class to add Cloud Logging
@@ -45,10 +52,6 @@ class CloudLoggingFilter(logging.Filter):
     overwritten using the `extras` argument when writing logs.
     """
 
-    # The subset of http_request fields have been tested to work consistently across GCP environments
-    # https://cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry#httprequest
-    _supported_http_fields = ("requestMethod", "requestUrl", "userAgent", "protocol")
-
     def __init__(self, project=None, default_labels=None):
         self.project = project
         self.default_labels = default_labels if default_labels else {}
@@ -80,13 +83,6 @@ def filter(self, record):
         user_labels = getattr(record, "labels", {})
         # infer request data from the environment
         inferred_http, inferred_trace, inferred_span = get_request_data()
-        if inferred_http is not None:
-            # filter inferred_http to include only well-supported fields
-            inferred_http = {
-                k: v
-                for (k, v) in inferred_http.items()
-                if k in self._supported_http_fields and v is not None
-            }
         if inferred_trace is not None and self.project is not None:
             # add full path for detected trace
             inferred_trace = f"projects/{self.project}/traces/{inferred_trace}"
@@ -188,12 +184,17 @@ def emit(self, record):
             record (logging.LogRecord): The record to be logged.
         """
         message = super(CloudLoggingHandler, self).format(record)
+        labels = record._labels
+        resource = record._resource or self.resource
+        if resource.type == _GAE_RESOURCE_TYPE and record._trace is not None:
+            # add GAE-specific label
+            labels = {_GAE_TRACE_ID_LABEL: record._trace, **(labels or {})}
         # send off request
         self.transport.send(
             record,
             message,
-            resource=(record._resource or self.resource),
-            labels=record._labels,
+            resource=resource,
+            labels=labels,
             trace=record._trace,
             span_id=record._span_id,
             http_request=record._http_request,

tests/environment

@@ -719,7 +719,7 @@ def test_get_default_handler_app_engine(self):
         import os
         from google.cloud._testing import _Monkey
         from google.cloud.logging_v2.handlers._monitored_resources import _GAE_ENV_VARS
-        from google.cloud.logging.handlers import AppEngineHandler
+        from google.cloud.logging.handlers import CloudLoggingHandler
 
         credentials = _make_credentials()
         client = self._make_one(
@@ -733,10 +733,10 @@ def test_get_default_handler_app_engine(self):
 
         handler.transport.worker.stop()
 
-        self.assertIsInstance(handler, AppEngineHandler)
+        self.assertIsInstance(handler, CloudLoggingHandler)
 
     def test_get_default_handler_container_engine(self):
-        from google.cloud.logging.handlers import ContainerEngineHandler
+        from google.cloud.logging.handlers import StructuredLogHandler
 
         credentials = _make_credentials()
         client = self._make_one(
@@ -751,7 +751,7 @@ def test_get_default_handler_container_engine(self):
         with patch:
             handler = client.get_default_handler()
 
-        self.assertIsInstance(handler, ContainerEngineHandler)
+        self.assertIsInstance(handler, StructuredLogHandler)
 
     def test_get_default_handler_general(self):
         import io