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.

Sign up for GitHub

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

Handle null arguments using JSON schema #845

Merged
merged 5 commits into from
Feb 10, 2025
Merged

Handle null arguments using JSON schema #845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0fefb1f
Handle null arguments using JSON schema
alexmojaki Feb 5, 2025
fa585c3
simplify set_user_attribute
alexmojaki Feb 5, 2025
05fa172
remove NULL_ARGS_KEY
alexmojaki Feb 5, 2025
9fe1c07
Simplify prepare_otlp_attributes
alexmojaki Feb 5, 2025
dd5c59c
Merge branch 'main' into alex/null
alexmojaki Feb 10, 2025
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
3 changes: 0 additions & 3 deletions logfire/_internal/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,6 @@ def log_level_attributes(level: LevelName | int) -> dict[str, otel_types.Attribu
ATTRIBUTES_SCRUBBED_KEY = f'{LOGFIRE_ATTRIBUTES_NAMESPACE}.scrubbed'
"""Key in OTEL attributes with metadata about parts of a span that have been scrubbed."""

NULL_ARGS_KEY = 'logfire.null_args'
"""Key in OTEL attributes that collects attributes with a null (None) value."""

PENDING_SPAN_NAME_SUFFIX = ' (pending)'
"""Suffix added to the name of a pending span to indicate it's a pending span and avoid collisions with the real span while in flight."""

Expand Down
4 changes: 2 additions & 2 deletions logfire/_internal/exporters/console.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,10 @@ def _details_parts(self, span: ReadableSpan, indent_str: str) -> TextParts:
return []

file_location_raw = span.attributes.get('code.filepath')
file_location = None if file_location_raw is None else str(file_location_raw)
file_location = None if file_location_raw in (None, 'null') else str(file_location_raw)
if file_location:
lineno = span.attributes.get('code.lineno')
if lineno: # pragma: no branch
if lineno not in (None, 'null'):
file_location += f':{lineno}'

log_level_num: int = span.attributes.get(ATTRIBUTES_LOG_LEVEL_NUM_KEY) # type: ignore
Expand Down
4 changes: 2 additions & 2 deletions logfire/_internal/json_schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def create_json_schema(obj: Any, seen: set[int]) -> JsonDict:
The JSON Schema.
"""
if obj is None:
return {}
return {'type': 'null'}

try:
# cover common types first before calling `type_to_schema` to avoid the overhead of imports if not necessary
Expand Down Expand Up @@ -219,7 +219,7 @@ def _enum_schema(obj: Enum, seen: set[int]) -> JsonDict:

# Schemas for values that are already JSON serializable, i.e. that don't need to be included
# (except at the top level) because the frontend can just render them as plain JSON.
PLAIN_SCHEMAS: tuple[JsonDict, ...] = ({}, {'type': 'object'}, {'type': 'array'})
PLAIN_SCHEMAS: tuple[JsonDict, ...] = ({}, {'type': 'object'}, {'type': 'array'}, {'type': 'null'})


def _mapping_schema(obj: Any, seen: set[int]) -> JsonDict:
Expand Down
37 changes: 9 additions & 28 deletions logfire/_internal/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
Sequence,
TypeVar,
Union,
cast,
overload,
)

Expand Down Expand Up @@ -45,7 +44,6 @@
ATTRIBUTES_TAGS_KEY,
DISABLE_CONSOLE_KEY,
LEVEL_NUMBERS,
NULL_ARGS_KEY,
OTLP_MAX_INT_SIZE,
LevelName,
log_level_attributes,
Expand Down Expand Up @@ -2204,7 +2202,7 @@ def set_attribute(self, key: str, value: Any) -> None:
"""
self._added_attributes = True
self._json_schema_properties[key] = create_json_schema(value, set())
key, otel_value = set_user_attribute(self._otlp_attributes, key, value)
otel_value = self._otlp_attributes[key] = prepare_otlp_attribute(value)
if self._span is not None: # pragma: no branch
self._span.set_attribute(key, otel_value)

Expand Down Expand Up @@ -2333,42 +2331,25 @@ def prepare_otlp_attributes(attributes: dict[str, Any]) -> dict[str, otel_types.

This will convert any non-OpenTelemetry compatible types to JSON.
"""
otlp_attributes: dict[str, otel_types.AttributeValue] = {}
return {key: prepare_otlp_attribute(value) for key, value in attributes.items()}

for key, value in attributes.items():
set_user_attribute(otlp_attributes, key, value)

return otlp_attributes


def set_user_attribute(
otlp_attributes: dict[str, otel_types.AttributeValue], key: str, value: Any
) -> tuple[str, otel_types.AttributeValue]:
"""Convert a user attribute to an OpenTelemetry compatible type and add it to the given dictionary.

Returns the final key and value that was added to the dictionary.
The key will be the original key unless the value was `None`, in which case it will be `NULL_ARGS_KEY`.
"""
otel_value: otel_types.AttributeValue
if value is None:
otel_value = cast('list[str]', otlp_attributes.get(NULL_ARGS_KEY, [])) + [key]
key = NULL_ARGS_KEY
elif isinstance(value, int):
def prepare_otlp_attribute(value: Any) -> otel_types.AttributeValue:
"""Convert a user attribute to an OpenTelemetry compatible type."""
if isinstance(value, int):
if value > OTLP_MAX_INT_SIZE:
warnings.warn(
f'Integer value {value} is larger than the maximum OTLP integer size of {OTLP_MAX_INT_SIZE} (64-bits), '
' if you need support for sending larger integers, please open a feature request',
UserWarning,
)
otel_value = str(value)
return str(value)
else:
otel_value = value
return value
elif isinstance(value, (str, bool, float)):
otel_value = value
return value
else:
otel_value = logfire_json_dumps(value)
otlp_attributes[key] = otel_value
return key, otel_value
return logfire_json_dumps(value)


def set_user_attributes_on_raw_span(span: Span, attributes: dict[str, Any]) -> None:
Expand Down
2 changes: 0 additions & 2 deletions logfire/_internal/scrubbing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
ATTRIBUTES_SCRUBBED_KEY,
ATTRIBUTES_SPAN_TYPE_KEY,
ATTRIBUTES_TAGS_KEY,
NULL_ARGS_KEY,
RESOURCE_ATTRIBUTES_PACKAGE_VERSIONS,
)
from .stack_info import STACK_INFO_KEYS
Expand Down Expand Up @@ -112,7 +111,6 @@ class BaseScrubber(ABC):
ATTRIBUTES_SAMPLE_RATE_KEY,
ATTRIBUTES_LOGGING_NAME,
ATTRIBUTES_SCRUBBED_KEY,
NULL_ARGS_KEY,
RESOURCE_ATTRIBUTES_PACKAGE_VERSIONS,
*STACK_INFO_KEYS,
SpanAttributes.EXCEPTION_STACKTRACE, # See scrub_event_attributes
Expand Down
56 changes: 28 additions & 28 deletions tests/otel_integrations/test_fastapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,9 @@ def test_path_param(client: TestClient, exporter: TestExporter) -> None:
'http.method': 'GET',
'fastapi.route.name': 'with_path_param',
'http.route': '/with_path_param/{param}',
'logfire.null_args': ('fastapi.route.operation_id',),
'fastapi.route.operation_id': 'null',
'logfire.level_num': 5,
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{}}}',
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"}}}',
},
},
{
Expand Down Expand Up @@ -397,8 +397,8 @@ def test_path_param(client: TestClient, exporter: TestExporter) -> None:
'client.port': 50000,
'http.route': '/with_path_param/{param}',
'fastapi.route.name': 'with_path_param',
'logfire.null_args': ('fastapi.route.operation_id',),
'logfire.json_schema': '{"type":"object","properties":{"fastapi.route.name":{},"fastapi.route.operation_id":{}}}',
'fastapi.route.operation_id': 'null',
'logfire.json_schema': '{"type":"object","properties":{"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"}}}',
'http.status_code': 200,
'http.response.status_code': 200,
},
Expand Down Expand Up @@ -492,9 +492,9 @@ def test_fastapi_instrumentation(client: TestClient, exporter: TestExporter) ->
'http.method': 'GET',
'fastapi.route.name': 'homepage',
'http.route': '/',
'logfire.null_args': ('fastapi.route.operation_id',),
'fastapi.route.operation_id': 'null',
'logfire.level_num': 5,
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{}}}',
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"}}}',
},
},
{
Expand Down Expand Up @@ -636,8 +636,8 @@ def test_fastapi_instrumentation(client: TestClient, exporter: TestExporter) ->
'client.port': 50000,
'http.route': '/',
'fastapi.route.name': 'homepage',
'logfire.null_args': ('fastapi.route.operation_id',),
'logfire.json_schema': '{"type":"object","properties":{"fastapi.route.name":{},"fastapi.route.operation_id":{}}}',
'fastapi.route.operation_id': 'null',
'logfire.json_schema': '{"type":"object","properties":{"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"}}}',
'http.status_code': 200,
'http.response.status_code': 200,
},
Expand Down Expand Up @@ -1190,9 +1190,9 @@ def test_fastapi_unhandled_exception(client: TestClient, exporter: TestExporter)
'http.method': 'GET',
'fastapi.route.name': 'exception',
'http.route': '/exception',
'logfire.null_args': ('fastapi.route.operation_id',),
'fastapi.route.operation_id': 'null',
'logfire.level_num': 5,
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{}}}',
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"}}}',
},
},
{
Expand Down Expand Up @@ -1256,8 +1256,8 @@ def test_fastapi_unhandled_exception(client: TestClient, exporter: TestExporter)
'client.port': 50000,
'http.route': '/exception',
'fastapi.route.name': 'exception',
'logfire.null_args': ('fastapi.route.operation_id',),
'logfire.json_schema': '{"type":"object","properties":{"fastapi.route.name":{},"fastapi.route.operation_id":{}}}',
'fastapi.route.operation_id': 'null',
'logfire.json_schema': '{"type":"object","properties":{"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"}}}',
'logfire.level_num': 17,
},
'events': [
Expand Down Expand Up @@ -1298,9 +1298,9 @@ def test_fastapi_handled_exception(client: TestClient, exporter: TestExporter) -
'http.method': 'GET',
'fastapi.route.name': 'validation_error',
'http.route': '/validation_error',
'logfire.null_args': ('fastapi.route.operation_id',),
'fastapi.route.operation_id': 'null',
'logfire.level_num': 5,
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{}}}',
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"}}}',
},
},
{
Expand Down Expand Up @@ -1392,8 +1392,8 @@ def test_fastapi_handled_exception(client: TestClient, exporter: TestExporter) -
'client.port': 50000,
'http.route': '/validation_error',
'fastapi.route.name': 'validation_error',
'logfire.null_args': ('fastapi.route.operation_id',),
'logfire.json_schema': '{"type":"object","properties":{"fastapi.route.name":{},"fastapi.route.operation_id":{}}}',
'fastapi.route.operation_id': 'null',
'logfire.json_schema': '{"type":"object","properties":{"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"}}}',
'http.status_code': 422,
'http.response.status_code': 422,
},
Expand Down Expand Up @@ -1434,11 +1434,11 @@ def test_scrubbing(client: TestClient, exporter: TestExporter) -> None:
),
'errors': '[]',
'custom_attr': 'custom_value',
'fastapi.route.operation_id': 'null',
'http.method': 'GET',
'http.route': '/secret/{path_param}',
'fastapi.route.name': 'secret',
'logfire.null_args': ('fastapi.route.operation_id',),
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{},"values":{"type":"object"},"errors":{"type":"array"},"custom_attr":{}}}',
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"},"values":{"type":"object"},"errors":{"type":"array"},"custom_attr":{}}}',
'logfire.scrubbed': IsJson(
[
{'path': ['attributes', 'values', 'path_param'], 'matched_substring': 'auth'},
Expand Down Expand Up @@ -1527,11 +1527,11 @@ def test_scrubbing(client: TestClient, exporter: TestExporter) -> None:
'http.route': '/secret/{path_param}',
'http.request.header.testauthorization': ("[Scrubbed due to 'auth']",),
'fastapi.route.name': 'secret',
'logfire.null_args': ('fastapi.route.operation_id',),
'fastapi.route.operation_id': 'null',
'fastapi.arguments.values': '{"path_param": "[Scrubbed due to \'auth\']", "foo": "foo_val", "password": "[Scrubbed due to \'password\']", "testauthorization": "[Scrubbed due to \'auth\']"}',
'fastapi.arguments.errors': '[]',
'custom_attr': 'custom_value',
'logfire.json_schema': '{"type":"object","properties":{"fastapi.route.name":{},"fastapi.route.operation_id":{},"custom_attr":{},"fastapi.arguments.values":{"type":"object"},"fastapi.arguments.errors":{"type":"array"}}}',
'logfire.json_schema': '{"type":"object","properties":{"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"},"custom_attr":{},"fastapi.arguments.values":{"type":"object"},"fastapi.arguments.errors":{"type":"array"}}}',
'http.status_code': 200,
'http.response.status_code': 200,
'logfire.scrubbed': IsJson(
Expand Down Expand Up @@ -1619,10 +1619,10 @@ def test_request_hooks_without_send_receiev_spans(exporter: TestExporter):
'values': '{}',
'errors': '[]',
'http.method': 'POST',
'fastapi.route.operation_id': 'null',
'http.route': '/echo_body',
'fastapi.route.name': 'echo_body',
'logfire.null_args': ('fastapi.route.operation_id',),
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{},"values":{"type":"object"},"errors":{"type":"array"}}}',
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"},"values":{"type":"object"},"errors":{"type":"array"}}}',
},
},
{
Expand Down Expand Up @@ -1723,10 +1723,10 @@ def test_request_hooks_without_send_receiev_spans(exporter: TestExporter):
'http.route': '/echo_body',
'attr_key': 'attr_val',
'fastapi.route.name': 'echo_body',
'logfire.null_args': ('fastapi.route.operation_id',),
'fastapi.route.operation_id': 'null',
'fastapi.arguments.values': '{}',
'fastapi.arguments.errors': '[]',
'logfire.json_schema': '{"type":"object","properties":{"attr_key":{},"fastapi.route.name":{},"fastapi.route.operation_id":{},"fastapi.arguments.values":{"type":"object"},"fastapi.arguments.errors":{"type":"array"}}}',
'logfire.json_schema': '{"type":"object","properties":{"attr_key":{},"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"},"fastapi.arguments.values":{"type":"object"},"fastapi.arguments.errors":{"type":"array"}}}',
'http.status_code': 200,
'http.response.status_code': 200,
},
Expand Down Expand Up @@ -1768,10 +1768,10 @@ def test_request_hooks_with_send_receive_spans(exporter: TestExporter):
'values': '{}',
'errors': '[]',
'http.method': 'POST',
'fastapi.route.operation_id': 'null',
'http.route': '/echo_body',
'fastapi.route.name': 'echo_body',
'logfire.null_args': ('fastapi.route.operation_id',),
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{},"values":{"type":"object"},"errors":{"type":"array"}}}',
'logfire.json_schema': '{"type":"object","properties":{"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"},"values":{"type":"object"},"errors":{"type":"array"}}}',
},
},
{
Expand Down Expand Up @@ -1913,10 +1913,10 @@ def test_request_hooks_with_send_receive_spans(exporter: TestExporter):
'http.route': '/echo_body',
'attr_key': 'attr_val',
'fastapi.route.name': 'echo_body',
'logfire.null_args': ('fastapi.route.operation_id',),
'fastapi.route.operation_id': 'null',
'fastapi.arguments.values': '{}',
'fastapi.arguments.errors': '[]',
'logfire.json_schema': '{"type":"object","properties":{"attr_key":{},"fastapi.route.name":{},"fastapi.route.operation_id":{},"fastapi.arguments.values":{"type":"object"},"fastapi.arguments.errors":{"type":"array"}}}',
'logfire.json_schema': '{"type":"object","properties":{"attr_key":{},"fastapi.route.name":{},"fastapi.route.operation_id":{"type":"null"},"fastapi.arguments.values":{"type":"object"},"fastapi.arguments.errors":{"type":"array"}}}',
'http.status_code': 200,
'http.response.status_code': 200,
},
Expand Down
Loading