Don't configure automatically #219
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
samuelcolvin
approved these changes
May 28, 2024
| @@ -1228,3 +1238,7 @@ def sanitize_project_name(name: str) -> str: | |||
|
|
|||
| def default_project_name(): | |||
| return sanitize_project_name(os.path.basename(os.getcwd())) | |||
|
|
|||
|
|
|||
| class LogfireNotConfiguredWarning(UserWarning): | |||
There was a problem hiding this comment.
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.
No, the behaviour does actually change. We can have a deprecation period (1 week?) if you want but I'm not sure it's worth it. |
Deploying logfire-docs with
|
| Latest commit: |
172a742
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d34b3a64.logfire-docs.pages.dev |
| Branch Preview URL: | https://alex-no-auto-configure.logfire-docs.pages.dev |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #194
Step 2 there isn't done but I realize now that it's not really a blocker, and there's strong demand for this change.
The implementation here is simple but not ideal. In particular
logfire.spanetc will still do a lot of processing which will slow things down and maybe even raise errors, it's just that the actual OTEL span will be a no-op. But makinglogfire.spana proper no-op is nontrivial since it still needs to return a context manager with methods, and that work overlaps with #146. Optimizing this can be done later, and the same strategy can also be applied to checking if the span/log will be sampled out or if instrumentation is currently suppressed.logfire.instrument_<module>will also apply instrumentation including patching, but (1) that's necessary to allow callingconfigurelater and having it all just work and (2) that's how OTEL works anyway.