Skip to content

Emit configuration span after logfire.configure()#1904

Open
Kludex wants to merge 6 commits intomainfrom
emit-configuration-span
Open

Emit configuration span after logfire.configure()#1904
Kludex wants to merge 6 commits intomainfrom
emit-configuration-span

Conversation

@Kludex
Copy link
Copy Markdown
Member

@Kludex Kludex commented May 4, 2026

Summary

  • Adds a Logfire configured log span emitted at the end of LogfireConfig._initialize, after providers/exporters are wired up but outside the suppress_instrumentation() block so it actually gets exported.
  • Span attributes: logfire_version, package_versions (from collect_package_info()), and logfire_config (a shallow dict of _LogfireConfigData fields). token and api_key are redacted to [REDACTED]; the scrubber catches anything else by default.
  • This fills a long-standing gap - collect_package_info was defined and tested but never actually emitted anywhere (a commented-out resource attribute in config.py hints at an earlier abandoned design).

Implementation notes

  • Used a manual field walk instead of dataclasses.asdict(self) because the latter deepcopys nested values, and additional_span_processors / log_record_processors carry _thread.lock objects that fail to pickle.
  • In tests, an autouse _disable_configured_span fixture replaces _emit_configuration_span with a no-op so existing snapshot tests aren't disturbed and trace/span IDs from IncrementalIdGenerator don't shift. A real_emit_configuration_span fixture exposes the original for the dedicated test_configuration_span_emitted test.

Test plan

  • make lint typecheck clean
  • New test_configuration_span_emitted covers attributes and token redaction
  • Updated test_exit_open_spans_exports_suspended_generator_span_before_shutdown to expect the new span
  • Full test suite passes (excluding 5 pre-existing failures in test_configure.py / test_cli.py caused by a stale local credentials file, unrelated to this change)

AI Disclaimer

This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.

Adds a "Logfire configured" log span at the end of `LogfireConfig._initialize`
containing the SDK version, installed package versions, and the active
configuration (with `token` and `api_key` redacted). Useful for diagnosing
"why is my data not showing up" issues from a single span in the UI.
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 4, 2026

Deploying logfire-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3ea2771
Status: ✅  Deploy successful!
Preview URL: https://5c2a85db.logfire-docs.pages.dev
Branch Preview URL: https://emit-configuration-span.logfire-docs.pages.dev

View logs

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 3 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@adriangb
Copy link
Copy Markdown
Member

adriangb commented May 4, 2026

  1. Why does this have to be a span instead of sending it out of band? Is it useful to query this data? Is this compatible with any other backend?
  2. How do we correlate this with other telemetry from the same SDK instance?
  3. How does this play with sampling? Do we force this one span to never be sampled? Presumably that's impossible if there's e.g. an otel collector doing head sampling.

@dmontagu
Copy link
Copy Markdown
Contributor

dmontagu commented May 4, 2026

  1. Why does this have to be a span instead of sending it out of band? Is it useful to query this data? Is this compatible with any other backend?

I believe it is useful to query this data for a variety of reasons — debugging logfire configuration, knowing when services were started, etc. Obviously better if you could be confident it was never sampled out but I'd prefer to have these spans and have it "misbehave" (i.e., just sometimes be missing these spans) if you have sampling enabled.

Not sure why it would be incompatible with other backends if it uses otel; obviously sending data out-of-band using another API is incompatible with other backends. I feel like making it opt-out of this behavior (rather than opt-in) is reasonable, but making other out-of-band telemetry opt-out rather than opt-in seems harder to justify.

  1. How do we correlate this with other telemetry from the same SDK instance?

We have the service.instance.id resource attribute that I would assume works for this?

  1. How does this play with sampling? Do we force this one span to never be sampled? Presumably that's impossible if there's e.g. an otel collector doing head sampling.

Presumably plays poorly but I feel like it's better to have than not. We could make it possible to opt out but I'd suggest we have it opt-in by default.

@adriangb
Copy link
Copy Markdown
Member

adriangb commented May 5, 2026

If we go forward with this please lets cherry pick information from _LogfireConfigData explicitly, not make a copy of the dict and rely on redaction.

Kludex added 2 commits May 6, 2026 16:35
Add `AdvancedOptions.emit_configuration_span` flag (default `False`) and
replace the dynamic config-dict serialization with an explicit allowlist of
non-sensitive flags. Identity and secrets - token, api_key, service_name,
service_version, environment, base_url - are never sent.
Comment thread logfire/_internal/config.py Outdated
Kludex added 2 commits May 6, 2026 16:52
…N` env var

Adds an `emit_configuration_span` ConfigParam so the flag can be set via env
var or pyproject.toml without passing an explicit `advanced=` argument -
useful for enabling on demo apps without touching the configure() call.
Promote the flag from `AdvancedOptions` to a first-class `configure()`
keyword argument plus a `_LogfireConfigData` field, matching the pattern
used by `distributed_tracing`. The `LOGFIRE_EMIT_CONFIGURATION_SPAN`
env var now drives the field directly instead of going through
`AdvancedOptions`.
@Kludex Kludex requested a review from alexmojaki May 6, 2026 15:02
@Kludex Kludex requested a review from alexmojaki May 6, 2026 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants