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.

Sign up for GitHub

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

Jump to bottom

Turn off metrics for uWSGI #2720

Merged
merged 4 commits into from Feb 9, 2024
Merged

Turn off metrics for uWSGI #2720

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aba5bd6
Turn off metrics for uWSGI
sentrivana Feb 9, 2024
db21c49
mypy
sentrivana Feb 9, 2024
326d6eb
add an override
sentrivana Feb 9, 2024
d0e5720
also add to experiments typedict
sentrivana Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 22 additions & 9 deletions sentry_sdk/client.py
View file
Open in desktop
Expand Up @@ -249,15 +249,28 @@ def _capture_envelope(envelope):

self.metrics_aggregator = None # type: Optional[MetricsAggregator]
experiments = self.options.get("_experiments", {})
if experiments.get("enable_metrics", True):
from sentry_sdk.metrics import MetricsAggregator

self.metrics_aggregator = MetricsAggregator(
capture_func=_capture_envelope,
enable_code_locations=bool(
experiments.get("metric_code_locations", True)
),
)
if experiments.get("enable_metrics", True) or experiments.get(
"force_enable_metrics", False
):
try:
import uwsgi # type: ignore
except ImportError:
uwsgi = None

if uwsgi is not None and not experiments.get(
"force_enable_metrics", False
):
logger.warning("Metrics currently not supported with uWSGI.")

else:
from sentry_sdk.metrics import MetricsAggregator

self.metrics_aggregator = MetricsAggregator(
capture_func=_capture_envelope,
enable_code_locations=bool(
experiments.get("metric_code_locations", True)
),
)
Comment on lines +255 to +273
Copy link
Member

Choose a reason for hiding this comment

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

Kinda a nitpick, but couldn't we get rid of this additional if/else branching simply by moving the if part to an else block associated with the try statement, and by moving the else portion under except ImportError? Something like this:

Suggested change
try:
import uwsgi # type: ignore
except ImportError:
uwsgi = None
if uwsgi is not None and not experiments.get(
"force_enable_metrics", False
):
logger.warning("Metrics currently not supported with uWSGI.")
else:
from sentry_sdk.metrics import MetricsAggregator
self.metrics_aggregator = MetricsAggregator(
capture_func=_capture_envelope,
enable_code_locations=bool(
experiments.get("metric_code_locations", True)
),
)
try:
import uwsgi as _ # type: ignore
except ImportError:
from sentry_sdk.metrics import MetricsAggregator
self.metrics_aggregator = MetricsAggregator(
capture_func=_capture_envelope,
enable_code_locations=bool(
experiments.get("metric_code_locations", True)
),
)
else:
if not experiments.get(
"force_enable_metrics", False
):
logger.warning("Metrics currently not supported with uWSGI.")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure this would work but I'd have to recheck this locally first and I'm kinda eager to get this hotfix out :) This code will go back to how it was anyway as soon as we figure out an actual fix.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha, fair enough. Does not matter if this change is only going to be temporary


max_request_body_size = ("always", "never", "small", "medium")
if self.options["max_request_body_size"] not in max_request_body_size:
Expand Down