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

Add metrics parameter to logfire.configure() #444

Merged
merged 16 commits into from
Sep 26, 2024
Merged

Conversation

alexmojaki
Copy link
Contributor

@alexmojaki alexmojaki commented Sep 25, 2024

Internal discussion: https://pydantic.slack.com/archives/C05AF4A4WRM/p1727260278734989

This allows users to pass metrics=False to disable sending metrics entirely, since OTEL instrumentations often produce metrics where users only want/need the spans.

Alternatively users can pass logfire.MetricsOptions(...). For now the only option is additional_readers which replaces the existing top-level additional_metric_readers parameter of configure. In the future I want to add other options, including a way to filter specific metrics instead of just disabling them all, and a way to create metrics from spans in the SDK.

Copy link

codecov bot commented Sep 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (2304f95) to head (8ebb9d5).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #444   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          131       131           
  Lines         9718      9754   +36     
  Branches      1275      1285   +10     
=========================================
+ Hits          9718      9754   +36     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

cloudflare-workers-and-pages bot commented Sep 25, 2024

Deploying logfire-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8ebb9d5
Status: ✅  Deploy successful!
Preview URL: https://d47f7e4a.logfire-docs.pages.dev
Branch Preview URL: https://alex-metrics-options.logfire-docs.pages.dev

View logs

@alexmojaki alexmojaki marked this pull request as ready for review September 25, 2024 12:15
Base automatically changed from alex/advanced-options to main September 25, 2024 12:23
Copy link
Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

I know it's not the point here, but do you know why did we choose dataclasses instead of TypeDicts on the parameters for configure? It looks very weird to instantiate those dataclasses as a user.

@alexmojaki alexmojaki enabled auto-merge (squash) September 26, 2024 09:08
@alexmojaki alexmojaki merged commit 091bc6f into main Sep 26, 2024
11 checks passed
@alexmojaki alexmojaki deleted the alex/metrics-options branch September 26, 2024 09:11
@alexmojaki
Copy link
Contributor Author

#263 (comment)

It seems that PyCharm is handling a plain dict being passed to a TypedDict param more cleverly than I remember in terms of stuff like autocomplete. Maybe things have improved and this should be reconsidered.

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.

2 participants