Respond to comments. Handle case where bytes is

set in LogRecord.body but is not a proto struct.

Author: DylanRussell

opentelemetry-exporter-gcp-logging/README.rst

@@ -65,7 +65,7 @@ Usage
     # so telemetry is collected only for the application
     logger1 = logging.getLogger("myapp.area1")
 
-    logger1.error({'structured_log_will_go_to_json_payload': 'value'}, extra={'this_will_go_to_LogEntry_labels_field': 'value'})
+    logger1.warning("string log %s", "here")
 
 References
 ----------

opentelemetry-exporter-gcp-logging/src/opentelemetry/exporter/cloud_logging/__init__.py

@@ -13,14 +13,17 @@
 # limitations under the License.
 from __future__ import annotations
 
+import base64
 import datetime
 import json
 import logging
 import urllib.parse
 from typing import Any, Mapping, Optional, Sequence
 
 import google.auth
-from google.api.monitored_resource_pb2 import MonitoredResource  # type: ignore
+from google.api.monitored_resource_pb2 import (  # pylint: disable = no-name-in-module
+    MonitoredResource,
+)
 from google.cloud.logging_v2.services.logging_service_v2 import (
     LoggingServiceV2Client,
 )
@@ -29,9 +32,15 @@
 )
 from google.cloud.logging_v2.types.log_entry import LogEntry
 from google.cloud.logging_v2.types.logging import WriteLogEntriesRequest
-from google.logging.type.log_severity_pb2 import LogSeverity  # type: ignore
-from google.protobuf.struct_pb2 import Struct
-from google.protobuf.timestamp_pb2 import Timestamp
+from google.logging.type.log_severity_pb2 import (  # pylint: disable = no-name-in-module
+    LogSeverity,
+)
+from google.protobuf.struct_pb2 import (  # pylint: disable = no-name-in-module
+    Struct,
+)
+from google.protobuf.timestamp_pb2 import (  # pylint: disable = no-name-in-module
+    Timestamp,
+)
 from opentelemetry.exporter.cloud_logging.version import __version__
 from opentelemetry.resourcedetector.gcp_resource_detector._mapping import (
     get_monitored_resource,
@@ -94,12 +103,21 @@
 
 
 def convert_any_value_to_string(value: Any) -> str:
-    t = type(value)
-    if t is bool or t is int or t is float or t is str:
+    if isinstance(value, bool):
+        return "true" if value else "false"
+    if isinstance(value, bytes):
+        return base64.b64encode(value).decode()
+    if (
+        isinstance(value, int)
+        or isinstance(value, float)
+        or isinstance(value, str)
+    ):
         return str(value)
-    if t is list or t is tuple:
+    if isinstance(value, list) or isinstance(value, tuple):
         return json.dumps(value)
-    logging.warning("Unknown type %s found, cannot convert to string.", t)
+    logging.warning(
+        "Unknown value %s found, cannot convert to string.", type(value)
+    )
     return ""
 
 
@@ -132,6 +150,7 @@ def export(self, batch: Sequence[LogData]):
         now = datetime.datetime.now()
         log_entries = []
         for log_data in batch:
+            log_entry = LogEntry()
             log_record = log_data.log_record
             attributes = log_record.attributes or {}
             project_id = str(
@@ -144,18 +163,7 @@ def export(self, batch: Sequence[LogData]):
                     )
                 )
             )
-            monitored_resource_data = get_monitored_resource(
-                log_record.resource or Resource({})
-            )
-            # convert it to proto
-            monitored_resource: Optional[MonitoredResource] = (
-                MonitoredResource(
-                    type=monitored_resource_data.type,
-                    labels=monitored_resource_data.labels,
-                )
-                if monitored_resource_data
-                else None
-            )
+            log_entry.log_name = f"projects/{project_id}/logs/{log_suffix}"
             # If timestamp is unset fall back to observed_time_unix_nano as recommended,
             # see https://github.com/open-telemetry/opentelemetry-proto/blob/4abbb78/opentelemetry/proto/logs/v1/logs.proto#L176-L179
             ts = Timestamp()
@@ -165,12 +173,15 @@ def export(self, batch: Sequence[LogData]):
                 )
             else:
                 ts.FromDatetime(now)
-            log_name = f"projects/{project_id}/logs/{log_suffix}"
-            log_entry = LogEntry()
             log_entry.timestamp = ts
-            log_entry.log_name = log_name
-            if monitored_resource:
-                log_entry.resource = monitored_resource
+            monitored_resource_data = get_monitored_resource(
+                log_record.resource or Resource({})
+            )
+            if monitored_resource_data:
+                log_entry.resource = MonitoredResource(
+                    type=monitored_resource_data.type,
+                    labels=monitored_resource_data.labels,
+                )
             log_entry.trace_sampled = (
                 log_record.trace_flags is not None
                 and log_record.trace_flags.sampled
@@ -190,30 +201,27 @@ def export(self, batch: Sequence[LogData]):
                 k: convert_any_value_to_string(v)
                 for k, v in attributes.items()
             }
-            if isinstance(log_record.body, Mapping):
-                s = Struct()
-                s.update(log_record.body)
-                log_entry.json_payload = s
-            elif type(log_record.body) is bytes:
-                json_str = log_record.body.decode("utf8")
-                json_dict = json.loads(json_str)
-                if isinstance(json_dict, Mapping):
-                    s = Struct()
-                    s.update(json_dict)
-                    log_entry.json_payload = s
-                else:
-                    logging.warning(
-                        "LogRecord.body was bytes type and json.loads turned body into type %s, expected a dictionary.",
-                        type(json_dict),
-                    )
-            else:
-                log_entry.text_payload = convert_any_value_to_string(
-                    log_record.body
-                )
+            self._set_payload_in_log_entry(log_entry, log_record.body)
             log_entries.append(log_entry)
 
         self._write_log_entries(log_entries)
 
+    def _set_payload_in_log_entry(self, log_entry: LogEntry, body: Any | None):
+        struct = Struct()
+        if isinstance(body, Mapping):
+            struct.update(body)
+            log_entry.json_payload = struct
+        elif isinstance(body, bytes):
+            json_str = body.decode("utf-8", errors="replace")
+            json_dict = json.loads(json_str)
+            if isinstance(json_dict, Mapping):
+                struct.update(json_dict)
+                log_entry.json_payload = struct
+            else:
+                log_entry.text_payload = base64.b64encode(body).decode()
+        else:
+            log_entry.text_payload = convert_any_value_to_string(body)
+
     def _write_log_entries(self, log_entries: list[LogEntry]):
         batch: list[LogEntry] = []
         batch_byte_size = 0

opentelemetry-exporter-gcp-logging/src/opentelemetry/exporter/cloud_logging/version.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = "0.9b0"
+__version__ = "1.9.0.dev0"

opentelemetry-exporter-gcp-logging/tests/__snapshots__/test_cloud_logging/test_convert_non_json_dict_bytes.json

@@ -11,6 +11,7 @@
           },
           "type": "generic_node"
         },
+        "textPayload": "MTIz",
         "timestamp": "2025-01-15T21:25:10.997977393Z"
       }
     ],

opentelemetry-exporter-gcp-logging/tests/__snapshots__/test_cloud_logging/test_convert_otlp_dict_body.json

@@ -16,7 +16,8 @@
         },
         "labels": {
           "event.name": "gen_ai.system.message",
-          "gen_ai.system": "openai"
+          "gen_ai.system": "true",
+          "test": "23"
         },
         "logName": "projects/fakeproject/logs/test",
         "resource": {

opentelemetry-exporter-gcp-logging/tests/__snapshots__/test_cloud_logging/test_convert_various_types_of_bodies[bool].json

@@ -11,7 +11,7 @@
           },
           "type": "generic_node"
         },
-        "textPayload": "True",
+        "textPayload": "true",
         "timestamp": "2025-01-15T21:25:10.997977393Z"
       }
     ],

opentelemetry-exporter-gcp-logging/tests/test_cloud_logging.py

@@ -79,7 +79,8 @@ def test_convert_otlp_dict_body(
                 trace_id=25,
                 span_id=22,
                 attributes={
-                    "gen_ai.system": "openai",
+                    "gen_ai.system": True,
+                    "test": 23,
                     "event.name": "gen_ai.system.message",
                 },
                 body={
@@ -136,7 +137,6 @@ def test_convert_otlp_various_different_types_in_attrs_and_bytes_body(
 def test_convert_non_json_dict_bytes(
     cloudloggingfake: CloudLoggingFake,
     snapshot_writelogentrycalls: List[WriteLogEntriesCall],
-    caplog,
 ) -> None:
     log_data = [
         LogData(
@@ -149,10 +149,6 @@ def test_convert_non_json_dict_bytes(
     ]
     cloudloggingfake.exporter.export(log_data)
     assert cloudloggingfake.get_calls() == snapshot_writelogentrycalls
-    assert (
-        "LogRecord.body was bytes type and json.loads turned body into type <class 'int'>, expected a dictionary"
-        in caplog.text
-    )
 
 
 @pytest.mark.parametrize(