Skip to content

Commit 72cef16

Browse files
committed
fix: server perf affected by sentry + fix permission issues caused by carriers permission update
1 parent 8b57b41 commit 72cef16

File tree

4 files changed

+136
-20
lines changed

4 files changed

+136
-20
lines changed

apps/api/karrio/server/settings/apm.py

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,9 @@
6464
SENTRY_DSN = config("SENTRY_DSN", default=None)
6565
SENTRY_ENVIRONMENT = config("SENTRY_ENVIRONMENT", default=config("ENV", default="production"))
6666
SENTRY_RELEASE = config("SENTRY_RELEASE", default=config("VERSION", default=None))
67-
SENTRY_TRACES_SAMPLE_RATE = config("SENTRY_TRACES_SAMPLE_RATE", default=1.0, cast=float)
68-
SENTRY_PROFILES_SAMPLE_RATE = config("SENTRY_PROFILES_SAMPLE_RATE", default=1.0, cast=float)
67+
# Lower default sample rates for better performance (was 1.0/100%)
68+
SENTRY_TRACES_SAMPLE_RATE = config("SENTRY_TRACES_SAMPLE_RATE", default=0.1, cast=float) # 10% of transactions
69+
SENTRY_PROFILES_SAMPLE_RATE = config("SENTRY_PROFILES_SAMPLE_RATE", default=0.0, cast=float) # Disabled by default
6970
SENTRY_SEND_PII = config("SENTRY_SEND_PII", default=True, cast=bool)
7071
SENTRY_DEBUG = config("SENTRY_DEBUG", default=False, cast=bool)
7172

@@ -136,8 +137,8 @@ def _sentry_before_send_transaction(event, hint):
136137
integrations = [
137138
DjangoIntegration(
138139
transaction_style="url", # Use URL patterns for transaction names
139-
middleware_spans=True, # Create spans for middleware
140-
signals_spans=True, # Create spans for Django signals
140+
middleware_spans=False, # Disabled for performance (was True)
141+
signals_spans=False, # Disabled for performance (was True)
141142
),
142143
]
143144

@@ -182,10 +183,10 @@ def _sentry_before_send_transaction(event, hint):
182183
environment=SENTRY_ENVIRONMENT,
183184
release=SENTRY_RELEASE,
184185

185-
# Performance monitoring
186+
# Performance monitoring (lower sample rates for better performance)
186187
traces_sample_rate=SENTRY_TRACES_SAMPLE_RATE,
187-
profile_session_sample_rate=SENTRY_PROFILES_SAMPLE_RATE,
188-
profile_lifecycle="trace",
188+
# Only enable profiling if explicitly configured (disabled by default)
189+
**({"profile_session_sample_rate": SENTRY_PROFILES_SAMPLE_RATE, "profile_lifecycle": "trace"} if SENTRY_PROFILES_SAMPLE_RATE > 0 else {}),
189190

190191
# Privacy settings
191192
send_default_pii=SENTRY_SEND_PII,
@@ -200,16 +201,11 @@ def _sentry_before_send_transaction(event, hint):
200201
before_send=_sentry_before_send,
201202
before_send_transaction=_sentry_before_send_transaction,
202203

203-
# Additional options
204-
max_breadcrumbs=50, # Keep last 50 breadcrumbs for context
204+
# Additional options (reduced for performance)
205+
max_breadcrumbs=25, # Reduced from 50 for lower memory usage
205206
attach_stacktrace=True, # Attach stack traces to messages
206-
include_source_context=True, # Include source code in stack traces
207-
include_local_variables=True, # Include local variables in stack traces
208-
209-
# Set custom tags
210-
_experiments={
211-
"record_sql_params": True, # Record SQL query parameters
212-
},
207+
include_source_context=False, # Disabled for performance (was True)
208+
include_local_variables=False, # Disabled for performance (was True)
213209
)
214210

215211
# Set default tags that will be applied to all events
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
# Generated by Django migration for carrier permission groups
2+
3+
from django.db import migrations
4+
5+
6+
def setup_carrier_groups(apps, schema_editor):
7+
"""
8+
Create read_carriers and write_carriers permission groups and update
9+
existing organization user permissions to include read_carriers.
10+
11+
This migration addresses the permission regression where:
12+
1. read_carriers and write_carriers groups were added to ROLES_GROUPS
13+
2. But existing user ContextPermissions weren't updated with these groups
14+
"""
15+
Group = apps.get_model('user', 'Group')
16+
Permission = apps.get_model('auth', 'Permission')
17+
ContentType = apps.get_model('contenttypes', 'ContentType')
18+
ContextPermission = apps.get_model('iam', 'ContextPermission')
19+
20+
# Create the new permission groups if they don't exist
21+
read_carriers_group, _ = Group.objects.get_or_create(name='read_carriers')
22+
write_carriers_group, _ = Group.objects.get_or_create(name='write_carriers')
23+
24+
# Get providers content types for permissions
25+
try:
26+
providers_ct = ContentType.objects.filter(app_label='providers')
27+
28+
# Set up read_carriers with view permissions
29+
view_perms = Permission.objects.filter(
30+
content_type__in=providers_ct,
31+
codename__icontains='view'
32+
)
33+
if view_perms.exists():
34+
read_carriers_group.permissions.set(view_perms)
35+
36+
# Set up write_carriers with all providers permissions
37+
all_provider_perms = Permission.objects.filter(content_type__in=providers_ct)
38+
if all_provider_perms.exists():
39+
write_carriers_group.permissions.set(all_provider_perms)
40+
except Exception:
41+
# Providers app may not be installed yet
42+
pass
43+
44+
# Update all existing ContextPermissions that have manage_carriers to also include read_carriers
45+
manage_carriers_group = Group.objects.filter(name='manage_carriers').first()
46+
47+
if manage_carriers_group:
48+
# Find all context permissions that have manage_carriers
49+
context_perms_with_manage = ContextPermission.objects.filter(
50+
groups=manage_carriers_group
51+
)
52+
53+
# Add read_carriers and write_carriers to these context permissions
54+
for ctx_perm in context_perms_with_manage:
55+
ctx_perm.groups.add(read_carriers_group)
56+
ctx_perm.groups.add(write_carriers_group)
57+
58+
# Also update any OrganizationUser context permissions based on their roles
59+
# This ensures all users who should have read_carriers get it
60+
try:
61+
OrganizationUser = apps.get_model('orgs', 'OrganizationUser')
62+
org_user_ct = ContentType.objects.get_for_model(OrganizationUser)
63+
64+
# Roles that should have read_carriers (all roles)
65+
org_user_context_perms = ContextPermission.objects.filter(
66+
content_type=org_user_ct
67+
)
68+
69+
for ctx_perm in org_user_context_perms:
70+
# All organization users should have read_carriers by default
71+
ctx_perm.groups.add(read_carriers_group)
72+
except Exception:
73+
# Orgs app may not be installed
74+
pass
75+
76+
77+
def reverse_migration(apps, schema_editor):
78+
"""Reverse migration - remove the new groups from context permissions."""
79+
Group = apps.get_model('user', 'Group')
80+
ContextPermission = apps.get_model('iam', 'ContextPermission')
81+
82+
read_carriers_group = Group.objects.filter(name='read_carriers').first()
83+
write_carriers_group = Group.objects.filter(name='write_carriers').first()
84+
85+
if read_carriers_group:
86+
for ctx_perm in ContextPermission.objects.filter(groups=read_carriers_group):
87+
ctx_perm.groups.remove(read_carriers_group)
88+
89+
if write_carriers_group:
90+
for ctx_perm in ContextPermission.objects.filter(groups=write_carriers_group):
91+
ctx_perm.groups.remove(write_carriers_group)
92+
93+
94+
class Migration(migrations.Migration):
95+
96+
dependencies = [
97+
('iam', '0001_initial'),
98+
('user', '0004_group'),
99+
]
100+
101+
operations = [
102+
migrations.RunPython(setup_carrier_groups, reverse_migration),
103+
]

modules/graph/karrio/server/graph/schemas/base/types.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,7 +1369,6 @@ def resolve_list_legacy(
13691369
@staticmethod
13701370
@utils.utils.error_wrapper
13711371
@utils.authentication_required
1372-
@utils.authorization_required(["read_carriers"])
13731372
def resolve(
13741373
info,
13751374
id: str,
@@ -1382,7 +1381,6 @@ def resolve(
13821381
@staticmethod
13831382
@utils.utils.error_wrapper
13841383
@utils.authentication_required
1385-
@utils.authorization_required(["read_carriers"])
13861384
def resolve_list(
13871385
info,
13881386
filter: typing.Optional[inputs.CarrierFilter] = strawberry.UNSET,

modules/sdk/karrio/core/utils/tracing.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,24 @@
2121

2222
Trace = typing.Callable[[typing.Any, str], typing.Any]
2323

24+
# Shared thread pool executor for trace recording
25+
# Using a single executor avoids the overhead of creating new thread pools per trace
26+
_trace_executor: typing.Optional[futures.ThreadPoolExecutor] = None
27+
_trace_executor_lock = __import__("threading").Lock()
28+
29+
30+
def _get_trace_executor() -> futures.ThreadPoolExecutor:
31+
"""Get or create the shared trace executor (lazy initialization, thread-safe)."""
32+
global _trace_executor
33+
if _trace_executor is None:
34+
with _trace_executor_lock:
35+
if _trace_executor is None:
36+
_trace_executor = futures.ThreadPoolExecutor(
37+
max_workers=4,
38+
thread_name_prefix="karrio_trace",
39+
)
40+
return _trace_executor
41+
2442

2543
# =============================================================================
2644
# Telemetry Abstraction Layer
@@ -380,8 +398,9 @@ def _save():
380398
metadata=metadata,
381399
)
382400

383-
promise = futures.ThreadPoolExecutor(max_workers=1)
384-
self.inner_recordings.update({promise.submit(_save): data})
401+
# Use shared executor to avoid creating new thread pools per trace
402+
executor = _get_trace_executor()
403+
self.inner_recordings.update({executor.submit(_save): data})
385404

386405
# Add telemetry breadcrumb for APM context
387406
self._telemetry.add_breadcrumb(

0 commit comments

Comments
 (0)