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.

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

Don't configure automatically #219

Merged
merged 7 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 18 additions & 4 deletions logfire/_internal/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
from .integrations.executors import instrument_executors
from .metrics import ProxyMeterProvider, configure_metrics
from .scrubbing import Scrubber, ScrubCallback
from .stack_info import get_user_frame_and_stacklevel
from .tracer import PendingSpanProcessor, ProxyTracerProvider
from .utils import UnexpectedResponse, ensure_data_dir_exists, get_version, read_toml_file, suppress_instrumentation

Expand Down Expand Up @@ -709,8 +710,7 @@ def get_tracer_provider(self) -> ProxyTracerProvider:
Returns:
The tracer provider.
"""
if not self._initialized:
return self.initialize()
self.warn_if_not_initialized('No logs or spans will be created')
return self._tracer_provider

def get_meter_provider(self) -> ProxyMeterProvider:
Expand All @@ -721,10 +721,20 @@ def get_meter_provider(self) -> ProxyMeterProvider:
Returns:
The meter provider.
"""
if not self._initialized: # pragma: no cover
self.initialize()
self.warn_if_not_initialized('No metrics will be created')
return self._meter_provider

def warn_if_not_initialized(self, message: str):
env_var_name = 'LOGFIRE_IGNORE_NO_CONFIG'
if not self._initialized and not os.environ.get(env_var_name):
_frame, stacklevel = get_user_frame_and_stacklevel()
warnings.warn(
f'{message} until `logfire.configure()` has been called. '
f'Set the environment variable {env_var_name}=1 to suppress this warning.',
category=LogfireNotConfiguredWarning,
stacklevel=stacklevel,
)

@cached_property
def meter(self) -> metrics.Meter:
"""Get a meter from this `LogfireConfig`.
Expand Down Expand Up @@ -1223,3 +1233,7 @@ def sanitize_project_name(name: str) -> str:

def default_project_name():
return sanitize_project_name(os.path.basename(os.getcwd()))


class LogfireNotConfiguredWarning(UserWarning):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be public?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we have a few warnings but none are public. I think that if we decide to make them public we should make a new logfire.warnings module for them in a new PR.

pass
11 changes: 11 additions & 0 deletions logfire/_internal/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,9 @@ def install_auto_tracing(
"""
install_auto_tracing(self, modules, check_imported_modules=check_imported_modules, min_duration=min_duration)

def _warn_if_not_initialized_for_instrumentation(self):
self.config.warn_if_not_initialized('Instrumentation will have no effect')

def instrument_fastapi(
self,
app: FastAPI,
Expand Down Expand Up @@ -850,6 +853,7 @@ def instrument_fastapi(
"""
from .integrations.fastapi import instrument_fastapi

self._warn_if_not_initialized_for_instrumentation()
return instrument_fastapi(
self,
app,
Expand Down Expand Up @@ -922,6 +926,7 @@ def instrument_openai(
from .integrations.llm_providers.llm_provider import instrument_llm_provider
from .integrations.llm_providers.openai import get_endpoint_config, is_async_client, on_response

self._warn_if_not_initialized_for_instrumentation()
return instrument_llm_provider(
self,
openai_client or (openai.OpenAI, openai.AsyncOpenAI),
Expand Down Expand Up @@ -995,6 +1000,7 @@ def instrument_anthropic(
from .integrations.llm_providers.anthropic import get_endpoint_config, is_async_client, on_response
from .integrations.llm_providers.llm_provider import instrument_llm_provider

self._warn_if_not_initialized_for_instrumentation()
return instrument_llm_provider(
self,
anthropic_client or (anthropic.Anthropic, anthropic.AsyncAnthropic),
Expand All @@ -1009,6 +1015,7 @@ def instrument_asyncpg(self):
"""Instrument the `asyncpg` module so that spans are automatically created for each query."""
from .integrations.asyncpg import instrument_asyncpg

self._warn_if_not_initialized_for_instrumentation()
return instrument_asyncpg()

def instrument_httpx(self, **kwargs: Any):
Expand All @@ -1020,6 +1027,7 @@ def instrument_httpx(self, **kwargs: Any):
"""
from .integrations.httpx import instrument_httpx

self._warn_if_not_initialized_for_instrumentation()
return instrument_httpx(**kwargs)

def instrument_django(
Expand Down Expand Up @@ -1061,6 +1069,7 @@ def instrument_django(
"""
from .integrations.django import instrument_django

self._warn_if_not_initialized_for_instrumentation()
return instrument_django(
is_sql_commentor_enabled=is_sql_commentor_enabled,
request_hook=request_hook,
Expand All @@ -1079,6 +1088,7 @@ def instrument_requests(self, excluded_urls: str | None = None, **kwargs: Any):
"""
from .integrations.requests import instrument_requests

self._warn_if_not_initialized_for_instrumentation()
return instrument_requests(excluded_urls=excluded_urls, **kwargs)

def instrument_psycopg(self, conn_or_module: Any = None, **kwargs: Any):
Expand All @@ -1102,6 +1112,7 @@ def instrument_psycopg(self, conn_or_module: Any = None, **kwargs: Any):
"""
from .integrations.psycopg import instrument_psycopg

self._warn_if_not_initialized_for_instrumentation()
return instrument_psycopg(conn_or_module, **kwargs)

def metric_counter(self, name: str, *, unit: str = '', description: str = '') -> Counter:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "hatchling.build"

[project]
name = "logfire"
version = "0.37.0"
version = "0.38.0"
description = "The best Python observability tool! 🪵🔥"
authors = [
{ name = "Pydantic Team", email = "[email protected]" },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
from pathlib import Path
from typing import Any

import logfire

# Build paths inside the project like this: BASE_DIR / 'subdir'.
BASE_DIR = Path(__file__).resolve().parent.parent

Expand Down Expand Up @@ -126,6 +124,3 @@
DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField'

LOGGING_CONFIG = None


logfire.instrument_django()
4 changes: 4 additions & 0 deletions tests/otel_integrations/test_django.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
from django.test import Client
from inline_snapshot import snapshot

import logfire
from logfire.testing import TestExporter


def test_good_route(client: Client, exporter: TestExporter):
logfire.instrument_django()
response: HttpResponse = client.get( # type: ignore
'/django_test_app/123/?very_long_query_param_name=very+long+query+param+value&foo=1'
)
Expand Down Expand Up @@ -41,6 +43,7 @@ def test_good_route(client: Client, exporter: TestExporter):


def test_error_route(client: Client, exporter: TestExporter):
logfire.instrument_django()
response: HttpResponse = client.get('/django_test_app/bad/?foo=1') # type: ignore
assert response.status_code == 400

Expand Down Expand Up @@ -84,6 +87,7 @@ def test_error_route(client: Client, exporter: TestExporter):


def test_no_matching_route(client: Client, exporter: TestExporter):
logfire.instrument_django()
response: HttpResponse = client.get('/django_test_app/nowhere/?foo=1') # type: ignore
assert response.status_code == 404

Expand Down
54 changes: 39 additions & 15 deletions tests/test_logfire.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import logfire
from logfire import Logfire, suppress_instrumentation
from logfire._internal.config import LogfireConfig, configure
from logfire._internal.config import LogfireConfig, LogfireNotConfiguredWarning, configure
from logfire._internal.constants import (
ATTRIBUTES_MESSAGE_KEY,
ATTRIBUTES_MESSAGE_TEMPLATE_KEY,
Expand Down Expand Up @@ -894,21 +894,45 @@ def test_logfire_with_its_own_config(exporter: TestExporter) -> None:
logfire = Logfire(config=config)
logfire1 = logfire.with_tags('tag1', 'tag2')

with pytest.warns(LogfireNotConfiguredWarning) as warnings:
with logfire.span('root'):
with logfire.span('child'):
logfire.info('test1')
logfire1.info('test2')

assert str(warnings[0].message) == (
'No logs or spans will be created until `logfire.configure()` has been called. '
'Set the environment variable LOGFIRE_IGNORE_NO_CONFIG=1 to suppress this warning.'
)
assert warnings[0].lineno == inspect.currentframe().f_lineno - 9 # type: ignore

with pytest.warns(LogfireNotConfiguredWarning) as warnings:
logfire.instrument_django()

assert str(warnings[0].message) == (
'Instrumentation will have no effect until `logfire.configure()` has been '
'called. Set the environment variable LOGFIRE_IGNORE_NO_CONFIG=1 to suppress '
'this warning.'
)
assert warnings[0].lineno == inspect.currentframe().f_lineno - 7 # type: ignore

assert exporter.exported_spans_as_dict(_include_pending_spans=True) == snapshot([])
assert exporter1.exported_spans_as_dict(_include_pending_spans=True) == snapshot([])

config.initialize()
with logfire.span('root'):
with logfire.span('child'):
logfire.info('test1')
logfire1.info('test2')

assert exporter.exported_spans_as_dict(_include_pending_spans=True) == snapshot([])

assert exporter1.exported_spans_as_dict(_include_pending_spans=True) == snapshot(
[
{
'name': 'root (pending)',
'context': {'trace_id': 1, 'span_id': 2, 'is_remote': False},
'parent': {'trace_id': 1, 'span_id': 1, 'is_remote': False},
'start_time': 1000000000,
'end_time': 1000000000,
'start_time': 5000000000,
'end_time': 5000000000,
'attributes': {
'code.filepath': 'test_logfire.py',
'code.lineno': 123,
Expand All @@ -923,8 +947,8 @@ def test_logfire_with_its_own_config(exporter: TestExporter) -> None:
'name': 'child (pending)',
'context': {'trace_id': 1, 'span_id': 4, 'is_remote': False},
'parent': {'trace_id': 1, 'span_id': 3, 'is_remote': False},
'start_time': 2000000000,
'end_time': 2000000000,
'start_time': 6000000000,
'end_time': 6000000000,
'attributes': {
'code.filepath': 'test_logfire.py',
'code.lineno': 123,
Expand All @@ -939,8 +963,8 @@ def test_logfire_with_its_own_config(exporter: TestExporter) -> None:
'name': 'test1',
'context': {'trace_id': 1, 'span_id': 5, 'is_remote': False},
'parent': {'trace_id': 1, 'span_id': 3, 'is_remote': False},
'start_time': 3000000000,
'end_time': 3000000000,
'start_time': 7000000000,
'end_time': 7000000000,
'attributes': {
'logfire.span_type': 'log',
'logfire.level_num': 9,
Expand All @@ -955,8 +979,8 @@ def test_logfire_with_its_own_config(exporter: TestExporter) -> None:
'name': 'test2',
'context': {'trace_id': 1, 'span_id': 6, 'is_remote': False},
'parent': {'trace_id': 1, 'span_id': 3, 'is_remote': False},
'start_time': 4000000000,
'end_time': 4000000000,
'start_time': 8000000000,
'end_time': 8000000000,
'attributes': {
'logfire.span_type': 'log',
'logfire.level_num': 9,
Expand All @@ -972,8 +996,8 @@ def test_logfire_with_its_own_config(exporter: TestExporter) -> None:
'name': 'child',
'context': {'trace_id': 1, 'span_id': 3, 'is_remote': False},
'parent': {'trace_id': 1, 'span_id': 1, 'is_remote': False},
'start_time': 2000000000,
'end_time': 5000000000,
'start_time': 6000000000,
'end_time': 9000000000,
'attributes': {
'code.filepath': 'test_logfire.py',
'code.lineno': 123,
Expand All @@ -987,8 +1011,8 @@ def test_logfire_with_its_own_config(exporter: TestExporter) -> None:
'name': 'root',
'context': {'trace_id': 1, 'span_id': 1, 'is_remote': False},
'parent': None,
'start_time': 1000000000,
'end_time': 6000000000,
'start_time': 5000000000,
'end_time': 10000000000,
'attributes': {
'code.filepath': 'test_logfire.py',
'code.lineno': 123,
Expand Down
22 changes: 0 additions & 22 deletions tests/test_metrics.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

import json
import subprocess
from typing import Any, cast

import pytest
Expand Down Expand Up @@ -322,27 +321,6 @@ def observable_counter(options: CallbackOptions):
)


def test_metrics_without_configure():
# Ensure that methods like logfire.metric_counter() can be called without calling logfire.configure().
# language=python
code = """
import logfire

config = logfire.DEFAULT_LOGFIRE_INSTANCE._config

def initialize():
# Just check that initialize() is called without actually starting metric exporters etc.
config._initialized = True

config.initialize = initialize

assert not config._initialized
logfire.metric_counter('foo')
assert config._initialized
"""
subprocess.check_call(['python', '-c', code])


def get_collected_metrics(metrics_reader: InMemoryMetricReader) -> list[dict[str, Any]]:
exported_metrics = json.loads(cast(MetricsData, metrics_reader.get_metrics_data()).to_json()) # type: ignore
[resource_metric] = exported_metrics['resource_metrics']
Expand Down