Skip to content

Commit

Permalink
Use OTEL scopes better, especially instead of tags (#420)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexmojaki authored Sep 12, 2024
1 parent 6f1a847 commit fae7737
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 45 deletions.
2 changes: 1 addition & 1 deletion logfire/_internal/async_.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def log_slow_callbacks(logfire: Logfire, slow_duration: float) -> ContextManager
Inspired by https://gitlab.com/quantlane/libs/aiodebug.
"""
original_run = asyncio.events.Handle._run
logfire = logfire.with_tags('slow-async')
logfire = logfire.with_settings(custom_scope_suffix='asyncio')
timer = logfire.config.ns_timestamp_generator
slow_duration *= ONE_SECOND_IN_NANOSECONDS

Expand Down
1 change: 1 addition & 0 deletions logfire/_internal/auto_trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ def install_auto_tracing(
)

min_duration = int(min_duration * ONE_SECOND_IN_NANOSECONDS)
logfire = logfire.with_settings(custom_scope_suffix='auto_tracing')
finder = LogfireFinder(logfire, modules, min_duration)
sys.meta_path.insert(0, finder)

Expand Down
2 changes: 1 addition & 1 deletion logfire/_internal/auto_trace/rewrite_ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def rewrite_ast(
min_duration: int,
) -> ast.AST:
tree = ast.parse(source)
logfire_args = LogfireArgs(logfire_instance._tags + ('auto-trace',), logfire_instance._sample_rate) # type: ignore
logfire_args = LogfireArgs(logfire_instance._tags, logfire_instance._sample_rate) # type: ignore
transformer = AutoTraceTransformer(
logfire_args, logfire_name, filename, module_name, logfire_instance, context_factories, min_duration
)
Expand Down
2 changes: 1 addition & 1 deletion logfire/_internal/integrations/fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def __init__(
],
excluded_urls: str | None,
):
self.logfire_instance = logfire_instance.with_tags('fastapi')
self.logfire_instance = logfire_instance.with_settings(custom_scope_suffix='fastapi')
self.request_attributes_mapper = request_attributes_mapper

# These lines, as well as the `excluded_urls_list.url_disabled` call below, are copied from OTEL.
Expand Down
2 changes: 1 addition & 1 deletion logfire/integrations/pydantic.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from pydantic_core import CoreConfig, CoreSchema


METER = GLOBAL_CONFIG._meter_provider.get_meter('pydantic-plugin-meter') # type: ignore
METER = GLOBAL_CONFIG._meter_provider.get_meter('logfire.pydantic') # type: ignore
validation_counter = METER.create_counter('pydantic.validations')


Expand Down
27 changes: 2 additions & 25 deletions tests/otel_integrations/test_fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ def test_path_param(client: TestClient, exporter: TestExporter) -> None:
assert response.status_code == 200
assert response.json() == {'param': 'param_val'}

assert exporter.exported_spans[1].instrumentation_scope.name == 'logfire.fastapi' # type: ignore

span_dicts = exporter.exported_spans_as_dict(_include_pending_spans=True)

# Highlights of the below mega-assert:
Expand Down Expand Up @@ -242,7 +244,6 @@ def test_path_param(client: TestClient, exporter: TestExporter) -> None:
'logfire.msg': 'FastAPI arguments',
'logfire.span_type': 'pending_span',
'logfire.pending_parent_id': '0000000000000001',
'logfire.tags': ('fastapi',),
},
},
{
Expand All @@ -255,7 +256,6 @@ def test_path_param(client: TestClient, exporter: TestExporter) -> None:
'logfire.msg_template': 'FastAPI arguments',
'logfire.msg': 'FastAPI arguments',
'logfire.span_type': 'span',
'logfire.tags': ('fastapi',),
'logfire.level_num': 5,
},
},
Expand All @@ -274,7 +274,6 @@ def test_path_param(client: TestClient, exporter: TestExporter) -> None:
'logfire.msg_template': '{method} {http.route} ({code.function})',
'logfire.span_type': 'pending_span',
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.tags': ('fastapi',),
'logfire.msg': 'GET /with_path_param/{param} (with_path_param)',
'logfire.level_num': 5,
'logfire.pending_parent_id': '0000000000000001',
Expand All @@ -295,7 +294,6 @@ def test_path_param(client: TestClient, exporter: TestExporter) -> None:
'logfire.msg_template': '{method} {http.route} ({code.function})',
'logfire.span_type': 'span',
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.tags': ('fastapi',),
'logfire.msg': 'GET /with_path_param/{param} (with_path_param)',
'logfire.level_num': 5,
},
Expand Down Expand Up @@ -461,7 +459,6 @@ def test_fastapi_instrumentation(client: TestClient, exporter: TestExporter) ->
'logfire.msg': 'FastAPI arguments',
'logfire.span_type': 'pending_span',
'logfire.pending_parent_id': '0000000000000003',
'logfire.tags': ('fastapi',),
},
},
{
Expand All @@ -472,7 +469,6 @@ def test_fastapi_instrumentation(client: TestClient, exporter: TestExporter) ->
'end_time': 4000000000,
'attributes': {
'logfire.span_type': 'span',
'logfire.tags': ('fastapi',),
'logfire.msg_template': 'FastAPI arguments',
'logfire.msg': 'FastAPI arguments',
'logfire.level_num': 5,
Expand All @@ -494,7 +490,6 @@ def test_fastapi_instrumentation(client: TestClient, exporter: TestExporter) ->
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.span_type': 'pending_span',
'logfire.msg': 'GET / (homepage)',
'logfire.tags': ('fastapi',),
'logfire.level_num': 5,
'logfire.pending_parent_id': '0000000000000003',
},
Expand Down Expand Up @@ -530,7 +525,6 @@ def test_fastapi_instrumentation(client: TestClient, exporter: TestExporter) ->
'logfire.msg_template': '{method} {http.route} ({code.function})',
'logfire.span_type': 'span',
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.tags': ('fastapi',),
'logfire.msg': 'GET / (homepage)',
'logfire.level_num': 5,
},
Expand Down Expand Up @@ -684,7 +678,6 @@ def test_fastapi_arguments(client: TestClient, exporter: TestExporter) -> None:
},
}
),
'logfire.tags': ('fastapi',),
},
},
{
Expand Down Expand Up @@ -791,7 +784,6 @@ def test_get_fastapi_arguments(client: TestClient, exporter: TestExporter) -> No
},
}
),
'logfire.tags': ('fastapi',),
},
},
{
Expand All @@ -809,7 +801,6 @@ def test_get_fastapi_arguments(client: TestClient, exporter: TestExporter) -> No
'logfire.msg_template': '{method} {http.route} ({code.function})',
'logfire.msg': 'GET /other (other_route)',
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.tags': ('fastapi',),
'logfire.level_num': 5,
'logfire.span_type': 'span',
},
Expand Down Expand Up @@ -918,7 +909,6 @@ def test_first_lvl_subapp_fastapi_arguments(client: TestClient, exporter: TestEx
},
}
),
'logfire.tags': ('fastapi',),
},
},
{
Expand All @@ -936,7 +926,6 @@ def test_first_lvl_subapp_fastapi_arguments(client: TestClient, exporter: TestEx
'logfire.msg_template': '{method} {http.route} ({code.function})',
'logfire.msg': 'GET /other (other_route)',
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.tags': ('fastapi',),
'logfire.level_num': 5,
'logfire.span_type': 'span',
},
Expand Down Expand Up @@ -1045,7 +1034,6 @@ def test_second_lvl_subapp_fastapi_arguments(client: TestClient, exporter: TestE
},
}
),
'logfire.tags': ('fastapi',),
},
},
{
Expand All @@ -1063,7 +1051,6 @@ def test_second_lvl_subapp_fastapi_arguments(client: TestClient, exporter: TestE
'logfire.msg_template': '{method} {http.route} ({code.function})',
'logfire.msg': 'GET /other (other_route)',
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.tags': ('fastapi',),
'logfire.level_num': 5,
'logfire.span_type': 'span',
},
Expand Down Expand Up @@ -1149,7 +1136,6 @@ def test_fastapi_unhandled_exception(client: TestClient, exporter: TestExporter)
'attributes': {
'logfire.msg_template': 'FastAPI arguments',
'logfire.msg': 'FastAPI arguments',
'logfire.tags': ('fastapi',),
'logfire.span_type': 'span',
'logfire.level_num': 5,
},
Expand All @@ -1169,7 +1155,6 @@ def test_fastapi_unhandled_exception(client: TestClient, exporter: TestExporter)
'logfire.msg_template': '{method} {http.route} ({code.function})',
'logfire.msg': 'GET /exception (exception)',
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.tags': ('fastapi',),
'logfire.span_type': 'span',
'logfire.level_num': 17,
},
Expand Down Expand Up @@ -1251,7 +1236,6 @@ def test_fastapi_handled_exception(client: TestClient, exporter: TestExporter) -
'attributes': {
'logfire.msg_template': 'FastAPI arguments',
'logfire.msg': 'FastAPI arguments',
'logfire.tags': ('fastapi',),
'logfire.span_type': 'span',
'logfire.level_num': 5,
},
Expand All @@ -1271,7 +1255,6 @@ def test_fastapi_handled_exception(client: TestClient, exporter: TestExporter) -
'logfire.msg_template': '{method} {http.route} ({code.function})',
'logfire.msg': 'GET /validation_error (validation_error)',
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.tags': ('fastapi',),
'logfire.span_type': 'span',
'logfire.level_num': 17,
},
Expand Down Expand Up @@ -1390,7 +1373,6 @@ def test_scrubbing(client: TestClient, exporter: TestExporter) -> None:
'fastapi.route.name': 'secret',
'logfire.null_args': ('fastapi.route.operation_id',),
'logfire.json_schema': '{"type":"object","properties":{"values":{"type":"object"},"errors":{"type":"array"},"custom_attr":{},"http.method":{},"http.route":{},"fastapi.route.name":{},"fastapi.route.operation_id":{}}}',
'logfire.tags': ('fastapi',),
'logfire.scrubbed': IsJson(
[
{'path': ['attributes', 'values', 'path_param'], 'matched_substring': 'auth'},
Expand All @@ -1415,7 +1397,6 @@ def test_scrubbing(client: TestClient, exporter: TestExporter) -> None:
'logfire.msg_template': '{method} {http.route} ({code.function})',
'logfire.msg': 'GET /secret/{path_param} (get_secret)',
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.tags': ('fastapi',),
'logfire.level_num': 5,
'logfire.span_type': 'span',
},
Expand Down Expand Up @@ -1541,7 +1522,6 @@ def test_request_hooks_without_send_receiev_spans(exporter: TestExporter):
'attributes': {
'logfire.msg_template': 'FastAPI arguments',
'logfire.msg': 'FastAPI arguments',
'logfire.tags': ('fastapi',),
'logfire.span_type': 'span',
'values': '{}',
'errors': '[]',
Expand Down Expand Up @@ -1583,7 +1563,6 @@ def test_request_hooks_without_send_receiev_spans(exporter: TestExporter):
'logfire.msg_template': '{method} {http.route} ({code.function})',
'logfire.msg': 'POST /echo_body (echo_body)',
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.tags': ('fastapi',),
'logfire.level_num': 5,
'logfire.span_type': 'span',
},
Expand Down Expand Up @@ -1686,7 +1665,6 @@ def test_request_hooks_with_send_receive_spans(exporter: TestExporter):
'attributes': {
'logfire.msg_template': 'FastAPI arguments',
'logfire.msg': 'FastAPI arguments',
'logfire.tags': ('fastapi',),
'logfire.span_type': 'span',
'values': '{}',
'errors': '[]',
Expand Down Expand Up @@ -1741,7 +1719,6 @@ def test_request_hooks_with_send_receive_spans(exporter: TestExporter):
'logfire.msg_template': '{method} {http.route} ({code.function})',
'logfire.msg': 'POST /echo_body (echo_body)',
'logfire.json_schema': '{"type":"object","properties":{"method":{},"http.route":{}}}',
'logfire.tags': ('fastapi',),
'logfire.level_num': 5,
'logfire.span_type': 'span',
},
Expand Down
16 changes: 6 additions & 10 deletions tests/test_auto_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import pytest
from inline_snapshot import snapshot

import logfire
from logfire import DEFAULT_LOGFIRE_INSTANCE, AutoTraceModule, install_auto_tracing
from logfire._internal.auto_trace import (
AutoTraceModuleAlreadyImportedException,
Expand All @@ -21,7 +22,7 @@
def test_auto_trace_sample(exporter: TestExporter) -> None:
meta_path = sys.meta_path.copy()

install_auto_tracing('tests.auto_trace_samples')
logfire.with_tags('testing', 'auto-tracing').install_auto_tracing('tests.auto_trace_samples')
# Check that having multiple LogfireFinders doesn't break things
install_auto_tracing('tests.blablabla')

Expand Down Expand Up @@ -50,6 +51,8 @@ def test_auto_trace_sample(exporter: TestExporter) -> None:
with pytest.raises(IndexError): # foo.bar intentionally raises an error to test that it's recorded below
asyncio.run(foo.bar())

assert exporter.exported_spans[0].instrumentation_scope.name == 'logfire.auto_tracing' # type: ignore

assert exporter.exported_spans_as_dict(_include_pending_spans=True) == snapshot(
[
{
Expand All @@ -63,9 +66,9 @@ def test_auto_trace_sample(exporter: TestExporter) -> None:
'code.lineno': 123,
'code.function': 'bar',
'logfire.msg_template': 'Calling tests.auto_trace_samples.foo.bar',
'logfire.tags': ('testing', 'auto-tracing'),
'logfire.msg': 'Calling tests.auto_trace_samples.foo.bar',
'logfire.span_type': 'pending_span',
'logfire.tags': ('auto-trace',),
'logfire.pending_parent_id': '0000000000000000',
},
},
Expand Down Expand Up @@ -111,8 +114,8 @@ def test_auto_trace_sample(exporter: TestExporter) -> None:
'code.lineno': 123,
'code.function': 'bar',
'logfire.msg_template': 'Calling tests.auto_trace_samples.foo.bar',
'logfire.tags': ('testing', 'auto-tracing'),
'logfire.span_type': 'span',
'logfire.tags': ('auto-trace',),
'logfire.msg': 'Calling tests.auto_trace_samples.foo.bar',
'logfire.level_num': 17,
},
Expand Down Expand Up @@ -256,7 +259,6 @@ def method4(self):
'code.lineno': 8,
'code.function': 'func.<locals>.Class.method',
'logfire.msg_template': 'Calling module.name.func.<locals>.Class.method',
'logfire.tags': ('auto-trace',),
},
),
(
Expand All @@ -266,7 +268,6 @@ def method4(self):
'code.lineno': 16,
'code.function': 'func.<locals>.Class.method2.<locals>.Class2.method3',
'logfire.msg_template': 'Calling module.name.func.<locals>.Class.method2.<locals>.Class2.method3',
'logfire.tags': ('auto-trace',),
},
),
(
Expand All @@ -276,7 +277,6 @@ def method4(self):
'code.lineno': 12,
'code.function': 'func.<locals>.Class.method2',
'logfire.msg_template': 'Calling module.name.func.<locals>.Class.method2',
'logfire.tags': ('auto-trace',),
},
),
(
Expand All @@ -286,7 +286,6 @@ def method4(self):
'code.lineno': 2,
'code.function': 'func',
'logfire.msg_template': 'Calling module.name.func',
'logfire.tags': ('auto-trace',),
},
),
(
Expand All @@ -296,7 +295,6 @@ def method4(self):
'code.lineno': 26,
'code.function': 'Class3.method4',
'logfire.msg_template': 'Calling module.name.Class3.method4',
'logfire.tags': ('auto-trace',),
},
),
]
Expand Down Expand Up @@ -468,7 +466,6 @@ def test_min_duration(exporter: TestExporter):
'code.lineno': 123,
'code.function': 'func2',
'logfire.msg_template': 'Calling tests.auto_trace_samples.simple_nesting.func2',
'logfire.tags': ('auto-trace',),
'logfire.span_type': 'span',
'logfire.msg': 'Calling tests.auto_trace_samples.simple_nesting.func2',
},
Expand All @@ -484,7 +481,6 @@ def test_min_duration(exporter: TestExporter):
'code.lineno': 123,
'code.function': 'func1',
'logfire.msg_template': 'Calling tests.auto_trace_samples.simple_nesting.func1',
'logfire.tags': ('auto-trace',),
'logfire.span_type': 'span',
'logfire.msg': 'Calling tests.auto_trace_samples.simple_nesting.func1',
},
Expand Down
Loading

0 comments on commit fae7737

Please sign in to comment.