Skip to content

Commit 26d416d

Browse files
DylanRussellDylanRussell
committed
Update code to validate log id and name
1 parent 00bd01e commit 26d416d

File tree

4 files changed

+239
-13
lines changed

4 files changed

+239
-13
lines changed

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

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import datetime
1818
import json
1919
import logging
20+
import re
2021
import urllib.parse
2122
from typing import Any, Mapping, MutableMapping, Optional, Sequence
2223

@@ -102,6 +103,8 @@
102103
24: LogSeverity.EMERGENCY,
103104
}
104105

106+
INVALID_LOG_NAME_MESSAGE = "%s is not a valid log name. log name must be <512 characters and only contain characters: A-Za-z0-9/-_."
107+
105108

106109
def _convert_any_value_to_string(value: Any) -> str:
107110
if isinstance(value, bool):
@@ -176,6 +179,12 @@ def _set_payload_in_log_entry(log_entry: LogEntry, body: AnyValue):
176179
log_entry.text_payload = _convert_any_value_to_string(body)
177180

178181

182+
def is_log_id_valid(log_id: str) -> bool:
183+
return len(log_id) < 512 and not bool(
184+
re.search(r"[^A-Za-z0-9\-_/\.]", log_id)
185+
)
186+
187+
179188
class CloudLoggingExporter(LogExporter):
180189
def __init__(
181190
self,
@@ -201,25 +210,28 @@ def __init__(
201210
)
202211
)
203212

213+
def pick_log_id(self, log_name_attr: Any, event_name: str | None) -> str:
214+
if log_name_attr and isinstance(log_name_attr, str):
215+
if is_log_id_valid(log_name_attr):
216+
return log_name_attr.replace("/", "%2F")
217+
logging.warning(INVALID_LOG_NAME_MESSAGE, log_name_attr)
218+
if event_name and is_log_id_valid(event_name):
219+
return event_name.replace("/", "%2F")
220+
return self.default_log_name
221+
204222
def export(self, batch: Sequence[LogData]):
205223
now = datetime.datetime.now()
206224
log_entries = []
207225
for log_data in batch:
208226
log_entry = LogEntry()
209227
log_record = log_data.log_record
210228
attributes = log_record.attributes or {}
229+
if log_record.event_name:
230+
attributes["event_name"] = log_record.event_name
211231
project_id = str(
212232
attributes.get(PROJECT_ID_ATTRIBUTE_KEY, self.project_id)
213233
)
214-
log_suffix = self.default_log_name
215-
log_name_attr = attributes.get(LOG_NAME_ATTRIBUTE_KEY)
216-
if log_name_attr and isinstance(log_name_attr, str):
217-
log_suffix = urllib.parse.quote_plus(log_name_attr)
218-
elif log_record.event_name:
219-
log_suffix = urllib.parse.quote_plus(
220-
log_record.event_name.replace(".", "_")
221-
)
222-
log_entry.log_name = f"projects/{project_id}/logs/{log_suffix}"
234+
log_entry.log_name = f"projects/{project_id}/logs/{self.pick_log_id(attributes.get(LOG_NAME_ATTRIBUTE_KEY), log_record.event_name)}"
223235
# If timestamp is unset fall back to observed_time_unix_nano as recommended,
224236
# see https://github.com/open-telemetry/opentelemetry-proto/blob/4abbb78/opentelemetry/proto/logs/v1/logs.proto#L176-L179
225237
ts = Timestamp()

‎opentelemetry-exporter-gcp-logging/tests/__snapshots__/test_cloud_logging/test_convert_gen_ai_body.json‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
[
2+
{
3+
"entries": [
4+
{
5+
"jsonPayload": {
6+
"gen_ai.input.messages": [
7+
{
8+
"parts": [
9+
{
10+
"content": "Get weather details in New Delhi and San Francisco?",
11+
"type": "text"
12+
}
13+
],
14+
"role": "user"
15+
},
16+
{
17+
"parts": [
18+
{
19+
"arguments": {
20+
"location": "New Delhi"
21+
},
22+
"id": "get_current_weather_0",
23+
"name": "get_current_weather",
24+
"type": "tool_call"
25+
},
26+
{
27+
"arguments": {
28+
"location": "San Francisco"
29+
},
30+
"id": "get_current_weather_1",
31+
"name": "get_current_weather",
32+
"type": "tool_call"
33+
}
34+
],
35+
"role": "model"
36+
},
37+
{
38+
"parts": [
39+
{
40+
"id": "get_current_weather_0",
41+
"response": {
42+
"content": "{\"temperature\": 35, \"unit\": \"C\"}"
43+
},
44+
"type": "tool_call_response"
45+
},
46+
{
47+
"id": "get_current_weather_1",
48+
"response": {
49+
"content": "{\"temperature\": 25, \"unit\": \"C\"}"
50+
},
51+
"type": "tool_call_response"
52+
}
53+
],
54+
"role": "user"
55+
}
56+
],
57+
"gen_ai.output.messages": [
58+
{
59+
"finish_reason": "stop",
60+
"parts": [
61+
{
62+
"content": "The current temperature in New Delhi is 35°C, and in San Francisco, it is 25°C.",
63+
"type": "text"
64+
}
65+
],
66+
"role": "model"
67+
}
68+
],
69+
"gen_ai.system_instructions": [
70+
{
71+
"content": "You are a clever language model",
72+
"type": "text"
73+
}
74+
]
75+
},
76+
"labels": {
77+
"event_name": "gen_ai.client.inference.operation.details"
78+
},
79+
"logName": "projects/fakeproject/logs/gen_ai.client.inference.operation.details",
80+
"resource": {
81+
"labels": {
82+
"location": "global",
83+
"namespace": "",
84+
"node_id": ""
85+
},
86+
"type": "generic_node"
87+
},
88+
"timestamp": "2025-01-15T21:25:10.997977393Z"
89+
}
90+
],
91+
"partialSuccess": true
92+
}
93+
]

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@
2121
}
2222
},
2323
"labels": {
24-
"event.name": "gen_ai.system.message",
24+
"event_name": "random.genai.event",
2525
"gen_ai.system": "true",
2626
"test": "23"
2727
},
28-
"logName": "projects/fakeproject/logs/random_genai_event",
28+
"logName": "projects/fakeproject/logs/random.genai.event",
2929
"resource": {
3030
"labels": {
3131
"location": "global",

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

Lines changed: 123 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,25 @@
2626
Be sure to review the changes.
2727
"""
2828
import re
29+
from functools import partial
2930
from typing import List, Mapping, Union
31+
from unittest.mock import patch
3032

3133
import pytest
3234
from fixtures.cloud_logging_fake import CloudLoggingFake, WriteLogEntriesCall
3335
from google.auth.credentials import AnonymousCredentials
3436
from google.cloud.logging_v2.services.logging_service_v2 import (
3537
LoggingServiceV2Client,
3638
)
39+
from google.cloud.logging_v2.services.logging_service_v2.transports.grpc import (
40+
LoggingServiceV2GrpcTransport,
41+
)
42+
from grpc import insecure_channel
3743
from opentelemetry._logs.severity import SeverityNumber
38-
from opentelemetry.exporter.cloud_logging import CloudLoggingExporter
44+
from opentelemetry.exporter.cloud_logging import (
45+
CloudLoggingExporter,
46+
is_log_id_valid,
47+
)
3948
from opentelemetry.sdk._logs import LogData
4049
from opentelemetry.sdk._logs._internal import LogRecord
4150
from opentelemetry.sdk.resources import Resource
@@ -82,7 +91,6 @@ def test_convert_otlp_dict_body(
8291
attributes={
8392
"gen_ai.system": True,
8493
"test": 23,
85-
"event.name": "gen_ai.system.message",
8694
},
8795
body={
8896
"kvlistValue": {
@@ -154,6 +162,119 @@ def test_convert_non_json_dict_bytes(
154162
assert cloudloggingfake.get_calls() == snapshot_writelogentrycalls
155163

156164

165+
def test_convert_gen_ai_body(
166+
cloudloggingfake: CloudLoggingFake,
167+
snapshot_writelogentrycalls: List[WriteLogEntriesCall],
168+
) -> None:
169+
log_data = [
170+
LogData(
171+
log_record=LogRecord(
172+
event_name="gen_ai.client.inference.operation.details",
173+
timestamp=1736976310997977393,
174+
body={
175+
"gen_ai.input.messages": (
176+
{
177+
"role": "user",
178+
"parts": (
179+
{
180+
"type": "text",
181+
"content": "Get weather details in New Delhi and San Francisco?",
182+
},
183+
),
184+
},
185+
{
186+
"role": "model",
187+
"parts": (
188+
{
189+
"type": "tool_call",
190+
"arguments": {"location": "New Delhi"},
191+
"name": "get_current_weather",
192+
"id": "get_current_weather_0",
193+
},
194+
{
195+
"type": "tool_call",
196+
"arguments": {"location": "San Francisco"},
197+
"name": "get_current_weather",
198+
"id": "get_current_weather_1",
199+
},
200+
),
201+
},
202+
{
203+
"role": "user",
204+
"parts": (
205+
{
206+
"type": "tool_call_response",
207+
"response": {
208+
"content": '{"temperature": 35, "unit": "C"}'
209+
},
210+
"id": "get_current_weather_0",
211+
},
212+
{
213+
"type": "tool_call_response",
214+
"response": {
215+
"content": '{"temperature": 25, "unit": "C"}'
216+
},
217+
"id": "get_current_weather_1",
218+
},
219+
),
220+
},
221+
),
222+
"gen_ai.system_instructions": (
223+
{
224+
"type": "text",
225+
"content": "You are a clever language model",
226+
},
227+
),
228+
"gen_ai.output.messages": (
229+
{
230+
"role": "model",
231+
"parts": (
232+
{
233+
"type": "text",
234+
"content": "The current temperature in New Delhi is 35°C, and in San Francisco, it is 25°C.",
235+
},
236+
),
237+
"finish_reason": "stop",
238+
},
239+
),
240+
},
241+
),
242+
instrumentation_scope=InstrumentationScope("test"),
243+
)
244+
]
245+
cloudloggingfake.exporter.export(log_data)
246+
assert cloudloggingfake.get_calls() == snapshot_writelogentrycalls
247+
248+
249+
def test_is_log_id_valid():
250+
assert is_log_id_valid(";") is False
251+
assert is_log_id_valid("aB12//..--__") is True
252+
assert is_log_id_valid("a" * 512) is False
253+
assert is_log_id_valid("abc1212**") is False
254+
assert is_log_id_valid("gen_ai.client.inference.operation.details") is True
255+
256+
257+
def test_pick_log_id() -> None:
258+
exporter = CloudLoggingExporter(
259+
project_id=PROJECT_ID,
260+
default_log_name="test",
261+
)
262+
assert (
263+
exporter.pick_log_id("valid_log_name_attr", "event_name_str")
264+
== "valid_log_name_attr"
265+
)
266+
assert (
267+
exporter.pick_log_id("invalid_attr**2", "event_name_str")
268+
== "event_name_str"
269+
)
270+
assert exporter.pick_log_id(None, "event_name_str") == "event_name_str"
271+
assert exporter.pick_log_id(None, None) == exporter.default_log_name
272+
assert (
273+
exporter.pick_log_id(None, "invalid_event_name_id24$")
274+
== exporter.default_log_name
275+
)
276+
277+
157278
@pytest.mark.parametrize(
158279
"body",
159280
[

0 commit comments

Comments
 (0)