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

Deprecate project_name in logfire.configure(), remove old kwargs from signature #428

Merged
merged 7 commits into from
Sep 17, 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
71 changes: 28 additions & 43 deletions logfire/_internal/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from functools import cached_property
from pathlib import Path
from threading import RLock, Thread
from typing import TYPE_CHECKING, Any, Callable, Literal, Sequence, cast
from typing import TYPE_CHECKING, Any, Callable, Literal, Sequence, TypedDict, cast
from urllib.parse import urljoin
from uuid import uuid4
from weakref import WeakSet
Expand Down Expand Up @@ -50,7 +50,7 @@
from opentelemetry.semconv.resource import ResourceAttributes
from rich.console import Console
from rich.prompt import Confirm, Prompt
from typing_extensions import Self
from typing_extensions import Self, Unpack

from logfire.exceptions import LogfireConfigError
from logfire.version import VERSION
Expand Down Expand Up @@ -78,7 +78,7 @@
from .exporters.test import TestExporter
from .integrations.executors import instrument_executors
from .metrics import ProxyMeterProvider
from .scrubbing import NOOP_SCRUBBER, BaseScrubber, Scrubber, ScrubbingOptions, ScrubCallback
from .scrubbing import NOOP_SCRUBBER, BaseScrubber, Scrubber, ScrubbingOptions
from .stack_info import warn_at_user_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 @@ -144,11 +144,15 @@ class PydanticPlugin:
"""Exclude specific modules from instrumentation."""


def configure(
class DeprecatedKwargs(TypedDict):
# Empty so that passing any additional kwargs makes static type checkers complain.
pass
Copy link
Member

Choose a reason for hiding this comment

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

By "undocumented" you mean you don't have type hints? and that's why you didn't add them here? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, added a comment



def configure( # noqa: D417
*,
send_to_logfire: bool | Literal['if-token-present'] | None = None,
token: str | None = None,
project_name: str | None = None,
service_name: str | None = None,
service_version: str | None = None,
trace_sample_rate: float | None = None,
Expand All @@ -157,20 +161,16 @@ def configure(
config_dir: Path | str | None = None,
data_dir: Path | str | None = None,
base_url: str | None = None,
collect_system_metrics: None = None,
id_generator: IdGenerator | None = None,
ns_timestamp_generator: Callable[[], int] | None = None,
processors: None = None,
additional_span_processors: Sequence[SpanProcessor] | None = None,
metric_readers: None = None,
additional_metric_readers: Sequence[MetricReader] | None = None,
pydantic_plugin: PydanticPlugin | None = None,
fast_shutdown: bool = False,
scrubbing_patterns: Sequence[str] | None = None,
scrubbing_callback: ScrubCallback | None = None,
scrubbing: ScrubbingOptions | Literal[False] | None = None,
inspect_arguments: bool | None = None,
tail_sampling: TailSamplingOptions | None = None,
**deprecated_kwargs: Unpack[DeprecatedKwargs],
) -> None:
"""Configure the logfire SDK.

Expand All @@ -179,11 +179,6 @@ def configure(
variable if set, otherwise defaults to `True`. If `if-token-present` is provided, logs will only be sent if
a token is present.
token: The project token. Defaults to the `LOGFIRE_TOKEN` environment variable.
project_name: Name to request when creating a new project. Defaults to the `LOGFIRE_PROJECT_NAME` environment
variable, or the current directory name.
Project name accepts a string value containing alphanumeric characters and
hyphens (-). The hyphen character must not be located at the beginning or end of the string and should
appear in between alphanumeric characters.
service_name: Name of this service. Defaults to the `LOGFIRE_SERVICE_NAME` environment variable.
service_version: Version of this service. Defaults to the `LOGFIRE_SERVICE_VERSION` environment variable, or the
current git commit hash if available.
Expand All @@ -198,39 +193,37 @@ def configure(
`LOGFIRE_CONFIG_DIR` environment variable, otherwise defaults to the current working directory.
data_dir: Directory to store credentials, and logs. If `None` uses the `LOGFIRE_CREDENTIALS_DIR` environment variable, otherwise defaults to `'.logfire'`.
base_url: Root URL for the Logfire API. If `None` uses the `LOGFIRE_BASE_URL` environment variable, otherwise defaults to https://logfire-api.pydantic.dev.
collect_system_metrics: Legacy argument, use [`logfire.instrument_system_metrics()`](https://docs.pydantic.dev/logfire/integrations/system_metrics/) instead.
id_generator: Generator for span IDs. Defaults to `RandomIdGenerator()` from the OpenTelemetry SDK.
ns_timestamp_generator: Generator for nanosecond timestamps. Defaults to [`time.time_ns`][time.time_ns] from the
Python standard library.
processors: Legacy argument, use `additional_span_processors` instead.
additional_span_processors: Span processors to use in addition to the default processor which exports spans to Logfire's API.
metric_readers: Legacy argument, use `additional_metric_readers` instead.
additional_metric_readers: Sequence of metric readers to be used in addition to the default reader
which exports metrics to Logfire's API.
pydantic_plugin: Configuration for the Pydantic plugin. If `None` uses the `LOGFIRE_PYDANTIC_PLUGIN_*` environment
variables, otherwise defaults to `PydanticPlugin(record='off')`.
fast_shutdown: Whether to shut down exporters and providers quickly, mostly used for tests. Defaults to `False`.
scrubbing: Options for scrubbing sensitive data. Set to `False` to disable.
scrubbing_patterns: Deprecated, use `scrubbing=logfire.ScrubbingOptions(extra_patterns=[...])` instead.
scrubbing_callback: Deprecated, use `scrubbing=logfire.ScrubbingOptions(callback=...)` instead.
inspect_arguments: Whether to enable
[f-string magic](https://docs.pydantic.dev/logfire/guides/onboarding_checklist/add_manual_tracing/#f-strings).
If `None` uses the `LOGFIRE_INSPECT_ARGUMENTS` environment variable.
Defaults to `True` if and only if the Python version is at least 3.11.
tail_sampling: Tail sampling options. Not ready for general use.
"""
processors = deprecated_kwargs.pop('processors', None) # type: ignore
if processors is not None: # pragma: no cover
raise ValueError(
'The `processors` argument has been replaced by `additional_span_processors`. '
'Set `send_to_logfire=False` to disable the default processor.'
)

metric_readers = deprecated_kwargs.pop('metric_readers', None) # type: ignore
if metric_readers is not None: # pragma: no cover
raise ValueError(
'The `metric_readers` argument has been replaced by `additional_metric_readers`. '
'Set `send_to_logfire=False` to disable the default metric reader.'
)

collect_system_metrics = deprecated_kwargs.pop('collect_system_metrics', None) # type: ignore
if collect_system_metrics is False:
raise ValueError(
'The `collect_system_metrics` argument has been removed. '
Expand All @@ -243,6 +236,8 @@ def configure(
'Use `logfire.instrument_system_metrics()` instead.'
)

scrubbing_callback = deprecated_kwargs.pop('scrubbing_callback', None) # type: ignore
scrubbing_patterns = deprecated_kwargs.pop('scrubbing_patterns', None) # type: ignore
if scrubbing_callback or scrubbing_patterns:
if scrubbing is not None:
raise ValueError(
Expand All @@ -254,13 +249,22 @@ def configure(
'Use `scrubbing=logfire.ScrubbingOptions(callback=..., extra_patterns=[...])` instead.',
DeprecationWarning,
)
scrubbing = ScrubbingOptions(callback=scrubbing_callback, extra_patterns=scrubbing_patterns)
scrubbing = ScrubbingOptions(callback=scrubbing_callback, extra_patterns=scrubbing_patterns) # type: ignore

project_name = deprecated_kwargs.pop('project_name', None) # type: ignore
if project_name is not None:
warnings.warn(
'The `project_name` argument is deprecated and not needed.',
DeprecationWarning,
)

if deprecated_kwargs:
raise TypeError(f'configure() got unexpected keyword arguments: {", ".join(deprecated_kwargs)}')

GLOBAL_CONFIG.configure(
base_url=base_url,
send_to_logfire=send_to_logfire,
token=token,
project_name=project_name,
service_name=service_name,
service_version=service_version,
trace_sample_rate=trace_sample_rate,
Expand Down Expand Up @@ -308,9 +312,6 @@ class _LogfireConfigData:
token: str | None
"""The Logfire API token to use"""

project_name: str | None
"""The Logfire project name to use"""

service_name: str
"""The name of this service"""

Expand Down Expand Up @@ -361,7 +362,6 @@ def _load_configuration(
base_url: str | None,
send_to_logfire: bool | Literal['if-token-present'] | None,
token: str | None,
project_name: str | None,
service_name: str | None,
service_version: str | None,
trace_sample_rate: float | None,
Expand All @@ -385,7 +385,6 @@ def _load_configuration(
self.base_url = param_manager.load_param('base_url', base_url)
self.send_to_logfire = param_manager.load_param('send_to_logfire', send_to_logfire)
self.token = param_manager.load_param('token', token)
self.project_name = param_manager.load_param('project_name', project_name)
self.service_name = param_manager.load_param('service_name', service_name)
self.service_version = param_manager.load_param('service_version', service_version)
self.trace_sample_rate = param_manager.load_param('trace_sample_rate', trace_sample_rate)
Expand Down Expand Up @@ -461,7 +460,6 @@ def __init__(
base_url: str | None = None,
send_to_logfire: bool | None = None,
token: str | None = None,
project_name: str | None = None,
service_name: str | None = None,
service_version: str | None = None,
trace_sample_rate: float | None = None,
Expand Down Expand Up @@ -491,7 +489,6 @@ def __init__(
base_url=base_url,
send_to_logfire=send_to_logfire,
token=token,
project_name=project_name,
service_name=service_name,
service_version=service_version,
trace_sample_rate=trace_sample_rate,
Expand Down Expand Up @@ -525,7 +522,6 @@ def configure(
base_url: str | None,
send_to_logfire: bool | Literal['if-token-present'] | None,
token: str | None,
project_name: str | None,
service_name: str | None,
service_version: str | None,
trace_sample_rate: float | None,
Expand All @@ -549,7 +545,6 @@ def configure(
base_url,
send_to_logfire,
token,
project_name,
service_name,
service_version,
trace_sample_rate,
Expand Down Expand Up @@ -677,7 +672,6 @@ def add_span_processor(span_processor: SpanProcessor) -> None:
if (credentials := LogfireCredentials.load_creds_file(self.data_dir)) is None: # pragma: no branch
credentials = LogfireCredentials.initialize_project(
logfire_api_url=self.base_url,
project_name=self.project_name,
session=requests.Session(),
)
credentials.write_creds_file(self.data_dir)
Expand Down Expand Up @@ -1157,7 +1151,6 @@ def create_new_project(
organization: str | None = None,
default_organization: bool = False,
project_name: str | None = None,
force_project_name_prompt: bool = False,
) -> dict[str, Any]:
"""Create a new project and configure it to be used by Logfire.

Expand All @@ -1170,8 +1163,6 @@ def create_new_project(
organization: The organization name of the new project.
default_organization: Whether to create the project under the user default organization.
project_name: The default name of the project.
force_project_name_prompt: Whether to force a prompt for the project name.
service_name: Name of the service.

Returns:
The created project informations.
Expand Down Expand Up @@ -1216,11 +1207,10 @@ def create_new_project(
if not confirm:
sys.exit(1)

project_name_default: str | None = project_name or default_project_name()
project_name_default: str | None = default_project_name()
project_name_prompt = 'Enter the project name'
while True:
if force_project_name_prompt or not project_name:
project_name = Prompt.ask(project_name_prompt, default=project_name_default)
project_name = project_name or Prompt.ask(project_name_prompt, default=project_name_default)
while project_name and not re.match(PROJECT_NAME_PATTERN, project_name):
project_name = Prompt.ask(
"\nThe project name you've entered is invalid. Valid project names:\n"
Expand Down Expand Up @@ -1264,15 +1254,12 @@ def initialize_project(
cls,
*,
logfire_api_url: str,
project_name: str | None,
session: requests.Session,
) -> Self:
"""Create a new project or use an existing project on logfire.dev requesting the given project name.

Args:
logfire_api_url: The Logfire API base URL.
project_name: Name for the project.
user_token: The user's token to use to create the new project.
session: HTTP client session used to communicate with the Logfire API.

Returns:
Expand Down Expand Up @@ -1300,8 +1287,6 @@ def initialize_project(
credentials = cls.create_new_project(
session=session,
logfire_api_url=logfire_api_url,
project_name=project_name,
force_project_name_prompt=True,
)

try:
Expand Down
3 changes: 0 additions & 3 deletions logfire/_internal/config_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ class _DefaultCallback:
"""Whether to send spans to Logfire."""
TOKEN = ConfigParam(env_vars=['LOGFIRE_TOKEN'])
"""Token for the Logfire API."""
PROJECT_NAME = ConfigParam(env_vars=['LOGFIRE_PROJECT_NAME'], allow_file_config=True)
"""Name of the project. Project name accepts a string value containing alphanumeric characters and hyphens (-). The hyphen character must not be located at the beginning or end of the string and should appear in between alphanumeric characters."""
SERVICE_NAME = ConfigParam(env_vars=['LOGFIRE_SERVICE_NAME', OTEL_SERVICE_NAME], allow_file_config=True, default='')
"""Name of the service emitting spans. For further details, please refer to the [Service section](https://opentelemetry.io/docs/specs/semconv/resource/#service)."""
SERVICE_VERSION = ConfigParam(env_vars=['LOGFIRE_SERVICE_VERSION', 'OTEL_SERVICE_VERSION'], allow_file_config=True)
Expand Down Expand Up @@ -104,7 +102,6 @@ class _DefaultCallback:
'base_url': BASE_URL,
'send_to_logfire': SEND_TO_LOGFIRE,
'token': TOKEN,
'project_name': PROJECT_NAME,
'service_name': SERVICE_NAME,
'service_version': SERVICE_VERSION,
'trace_sample_rate': TRACE_SAMPLE_RATE,
Expand Down
5 changes: 2 additions & 3 deletions logfire/_internal/scrubbing.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@

@dataclass
class ScrubMatch:
"""An object passed to the [`scrubbing_callback`][logfire.configure(scrubbing_callback)] function."""
"""An object passed to a [`ScrubbingOptions.callback`][logfire.ScrubbingOptions.callback] function."""

path: JsonPath
"""The path to the value in the span being considered for redaction, e.g. `('attributes', 'password')`."""
Expand All @@ -68,7 +68,6 @@ class ScrubMatch:
"""


# See scrubbing_callback in logfire.configure for more info on this type.
ScrubCallback = Callable[[ScrubMatch], Any]


Expand Down Expand Up @@ -152,7 +151,7 @@ class Scrubber(BaseScrubber):
"""Redacts potentially sensitive data."""

def __init__(self, patterns: Sequence[str] | None, callback: ScrubCallback | None = None):
# See scrubbing_patterns and scrubbing_callback in logfire.configure for more info on these parameters.
# See ScrubbingOptions for more info on these parameters.
patterns = [*DEFAULT_PATTERNS, *(patterns or [])]
self._pattern = re.compile('|'.join(patterns), re.IGNORECASE | re.DOTALL)
self._callback = callback
Expand Down
2 changes: 1 addition & 1 deletion tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ def test_projects_new_invalid_project_name(tmp_dir_cwd: Path, default_credential
' * may contain single hyphens\n'
' * may not start or end with a hyphen\n\n'
'Enter the project name you want to use:',
default='invalid name',
default='testprojectsnewinvalidproj0',
),
]
console_calls = [re.sub(r'^call(\(\).)?', '', str(call)) for call in console.mock_calls]
Expand Down
13 changes: 12 additions & 1 deletion tests/test_configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,6 @@ def test_read_config_from_pyproject_toml(tmp_path: Path) -> None:

assert GLOBAL_CONFIG.base_url == 'https://api.logfire.io'
assert GLOBAL_CONFIG.send_to_logfire is False
assert GLOBAL_CONFIG.project_name == 'test'
assert GLOBAL_CONFIG.console
assert GLOBAL_CONFIG.console.colors == 'never'
assert GLOBAL_CONFIG.console.include_timestamps is False
Expand Down Expand Up @@ -1546,3 +1545,15 @@ def test_collect_system_metrics_true():
)
):
logfire.configure(collect_system_metrics=True) # type: ignore


def test_unknown_kwargs():
with inline_snapshot.extra.raises(snapshot('TypeError: configure() got unexpected keyword arguments: foo, bar')):
logfire.configure(foo=1, bar=2) # type: ignore


def test_project_name_deprecated():
with inline_snapshot.extra.raises(
snapshot('DeprecationWarning: The `project_name` argument is deprecated and not needed.')
):
logfire.configure(project_name='foo') # type: ignore
8 changes: 4 additions & 4 deletions tests/test_logfire.py
Original file line number Diff line number Diff line change
Expand Up @@ -1253,10 +1253,10 @@ def test_format_attribute_added_after_pending_span_sent(exporter: TestExporter)
)


def check_project_name(expected_project_name: str) -> None:
def check_service_name(expected_service_name: str) -> None:
from logfire._internal.config import GLOBAL_CONFIG

assert GLOBAL_CONFIG.project_name == expected_project_name
assert GLOBAL_CONFIG.service_name == expected_service_name


@pytest.mark.parametrize(
Expand All @@ -1273,12 +1273,12 @@ def test_config_preserved_across_thread_or_process(
configure(
send_to_logfire=False,
console=False,
project_name='foobar!',
service_name='foobar!',
additional_metric_readers=[InMemoryMetricReader()],
)

with executor_factory() as executor:
executor.submit(check_project_name, 'foobar!')
executor.submit(check_service_name, 'foobar!')
executor.shutdown(wait=True)


Expand Down
4 changes: 2 additions & 2 deletions tests/test_secret_scrubbing.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ def callback(match: logfire.ScrubMatch): # pragma: no cover
with pytest.warns(
DeprecationWarning, match='The `scrubbing_callback` and `scrubbing_patterns` arguments are deprecated.'
):
logfire.configure(**config_kwargs, scrubbing_patterns=['my_pattern'], scrubbing_callback=callback)
logfire.configure(**config_kwargs, scrubbing_patterns=['my_pattern'], scrubbing_callback=callback) # type: ignore

config = logfire.DEFAULT_LOGFIRE_INSTANCE.config
assert config.scrubbing
Expand All @@ -344,7 +344,7 @@ def test_scrubbing_deprecated_args_combined_with_new_options():
ValueError,
match='Cannot specify `scrubbing` and `scrubbing_callback` or `scrubbing_patterns` at the same time.',
):
logfire.configure(scrubbing_patterns=['my_pattern'], scrubbing=logfire.ScrubbingOptions())
logfire.configure(scrubbing_patterns=['my_pattern'], scrubbing=logfire.ScrubbingOptions()) # type: ignore


@pytest.mark.skipif(sys.version_info[:2] < (3, 9), reason='f-string magic is not allowed in 3.8')
Expand Down