Skip to content

Commit 3ffae8f

Browse files
ahoblitzclaudeholtskinner
authored
perf: optimize logging performance and modernize string formatting (#411)
- Replace f-strings with `%` formatting in logging calls for lazy evaluation - Convert `.format()` to f-strings in `server/models.py` `__repr__` methods - Add `test_venv/` to `.gitignore` - Improves performance by avoiding string formatting when log levels are disabled --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Holt Skinner <[email protected]> Co-authored-by: Holt Skinner <[email protected]>
1 parent 5912b28 commit 3ffae8f

File tree

15 files changed

+106
-77
lines changed

15 files changed

+106
-77
lines changed

.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ __pycache__
66
.pytest_cache
77
.ruff_cache
88
.venv
9+
test_venv/
910
coverage.xml
1011
.nox
11-
spec.json
12+
spec.json

.ruff.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ ignore = [
3030
"ANN003",
3131
"ANN401",
3232
"TRY003",
33-
"G004",
3433
"TRY201",
3534
"FIX002",
3635
]

src/a2a/client/auth/interceptor.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,9 @@ async def intercept(
6262
):
6363
headers['Authorization'] = f'Bearer {credential}'
6464
logger.debug(
65-
f"Added Bearer token for scheme '{scheme_name}' (type: {scheme_def.type})."
65+
"Added Bearer token for scheme '%s' (type: %s).",
66+
scheme_name,
67+
scheme_def.type,
6668
)
6769
http_kwargs['headers'] = headers
6870
return request_payload, http_kwargs
@@ -74,7 +76,9 @@ async def intercept(
7476
):
7577
headers['Authorization'] = f'Bearer {credential}'
7678
logger.debug(
77-
f"Added Bearer token for scheme '{scheme_name}' (type: {scheme_def.type})."
79+
"Added Bearer token for scheme '%s' (type: %s).",
80+
scheme_name,
81+
scheme_def.type,
7882
)
7983
http_kwargs['headers'] = headers
8084
return request_payload, http_kwargs
@@ -83,7 +87,8 @@ async def intercept(
8387
case APIKeySecurityScheme(in_=In.header):
8488
headers[scheme_def.name] = credential
8589
logger.debug(
86-
f"Added API Key Header for scheme '{scheme_name}'."
90+
"Added API Key Header for scheme '%s'.",
91+
scheme_name,
8792
)
8893
http_kwargs['headers'] = headers
8994
return request_payload, http_kwargs

src/a2a/server/apps/jsonrpc/jsonrpc_app.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -233,9 +233,13 @@ def _generate_error_response(
233233
)
234234
logger.log(
235235
log_level,
236-
f'Request Error (ID: {request_id}): '
237-
f"Code={error_resp.error.code}, Message='{error_resp.error.message}'"
238-
f'{", Data=" + str(error_resp.error.data) if error_resp.error.data else ""}',
236+
"Request Error (ID: %s): Code=%s, Message='%s'%s",
237+
request_id,
238+
error_resp.error.code,
239+
error_resp.error.message,
240+
', Data=' + str(error_resp.error.data)
241+
if error_resp.error.data
242+
else '',
239243
)
240244
return JSONResponse(
241245
error_resp.model_dump(mode='json', exclude_none=True),
@@ -422,7 +426,7 @@ async def _process_non_streaming_request(
422426
)
423427
case _:
424428
logger.error(
425-
f'Unhandled validated request type: {type(request_obj)}'
429+
'Unhandled validated request type: %s', type(request_obj)
426430
)
427431
error = UnsupportedOperationError(
428432
message=f'Request type {type(request_obj).__name__} is unknown.'
@@ -497,8 +501,10 @@ async def _handle_get_agent_card(self, request: Request) -> JSONResponse:
497501
"""
498502
if request.url.path == PREV_AGENT_CARD_WELL_KNOWN_PATH:
499503
logger.warning(
500-
f"Deprecated agent card endpoint '{PREV_AGENT_CARD_WELL_KNOWN_PATH}' accessed. "
501-
f"Please use '{AGENT_CARD_WELL_KNOWN_PATH}' instead. This endpoint will be removed in a future version."
504+
"Deprecated agent card endpoint '%s' accessed. "
505+
"Please use '%s' instead. This endpoint will be removed in a future version.",
506+
PREV_AGENT_CARD_WELL_KNOWN_PATH,
507+
AGENT_CARD_WELL_KNOWN_PATH,
502508
)
503509

504510
card_to_serve = self.agent_card

src/a2a/server/events/event_consumer.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ async def consume_one(self) -> Event:
6262
InternalError(message='Agent did not return any response')
6363
) from e
6464

65-
logger.debug(f'Dequeued event of type: {type(event)} in consume_one.')
65+
logger.debug('Dequeued event of type: %s in consume_one.', type(event))
6666

6767
self.queue.task_done()
6868

@@ -95,7 +95,7 @@ async def consume_all(self) -> AsyncGenerator[Event]:
9595
self.queue.dequeue_event(), timeout=self._timeout
9696
)
9797
logger.debug(
98-
f'Dequeued event of type: {type(event)} in consume_all.'
98+
'Dequeued event of type: %s in consume_all.', type(event)
9999
)
100100
self.queue.task_done()
101101
logger.debug(

src/a2a/server/events/event_queue.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ async def enqueue_event(self, event: Event) -> None:
5454
logger.warning('Queue is closed. Event will not be enqueued.')
5555
return
5656

57-
logger.debug(f'Enqueuing event of type: {type(event)}')
57+
logger.debug('Enqueuing event of type: %s', type(event))
5858

5959
# Make sure to use put instead of put_nowait to avoid blocking the event loop.
6060
await self.queue.put(event)
@@ -103,13 +103,13 @@ async def dequeue_event(self, no_wait: bool = False) -> Event:
103103
logger.debug('Attempting to dequeue event (no_wait=True).')
104104
event = self.queue.get_nowait()
105105
logger.debug(
106-
f'Dequeued event (no_wait=True) of type: {type(event)}'
106+
'Dequeued event (no_wait=True) of type: %s', type(event)
107107
)
108108
return event
109109

110110
logger.debug('Attempting to dequeue event (waiting).')
111111
event = await self.queue.get()
112-
logger.debug(f'Dequeued event (waited) of type: {type(event)}')
112+
logger.debug('Dequeued event (waited) of type: %s', type(event))
113113
return event
114114

115115
def task_done(self) -> None:
@@ -193,7 +193,9 @@ async def clear_events(self, clear_child_queues: bool = True) -> None:
193193
while True:
194194
event = self.queue.get_nowait()
195195
logger.debug(
196-
f'Discarding unprocessed event of type: {type(event)}, content: {event}'
196+
'Discarding unprocessed event of type: %s, content: %s',
197+
type(event),
198+
event,
197199
)
198200
self.queue.task_done()
199201
cleared_count += 1
@@ -211,7 +213,8 @@ async def clear_events(self, clear_child_queues: bool = True) -> None:
211213

212214
if cleared_count > 0:
213215
logger.debug(
214-
f'Cleared {cleared_count} unprocessed events from EventQueue.'
216+
'Cleared %d unprocessed events from EventQueue.',
217+
cleared_count,
215218
)
216219

217220
# Clear all child queues (lock released before awaiting child tasks)

src/a2a/server/models.py

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,9 @@ def task_metadata(cls) -> Mapped[dict[str, Any] | None]:
147147
@override
148148
def __repr__(self) -> str:
149149
"""Return a string representation of the task."""
150-
repr_template = (
151-
'<{CLS}(id="{ID}", context_id="{CTX_ID}", status="{STATUS}")>'
152-
)
153-
return repr_template.format(
154-
CLS=self.__class__.__name__,
155-
ID=self.id,
156-
CTX_ID=self.context_id,
157-
STATUS=self.status,
150+
return (
151+
f'<{self.__class__.__name__}(id="{self.id}", '
152+
f'context_id="{self.context_id}", status="{self.status}")>'
158153
)
159154

160155

@@ -188,12 +183,9 @@ class TaskModel(TaskMixin, base): # type: ignore
188183
@override
189184
def __repr__(self) -> str:
190185
"""Return a string representation of the task."""
191-
repr_template = '<TaskModel[{TABLE}](id="{ID}", context_id="{CTX_ID}", status="{STATUS}")>'
192-
return repr_template.format(
193-
TABLE=table_name,
194-
ID=self.id,
195-
CTX_ID=self.context_id,
196-
STATUS=self.status,
186+
return (
187+
f'<TaskModel[{table_name}](id="{self.id}", '
188+
f'context_id="{self.context_id}", status="{self.status}")>'
197189
)
198190

199191
# Set a dynamic name for better debugging
@@ -221,11 +213,9 @@ class PushNotificationConfigMixin:
221213
@override
222214
def __repr__(self) -> str:
223215
"""Return a string representation of the push notification config."""
224-
repr_template = '<{CLS}(task_id="{TID}", config_id="{CID}")>'
225-
return repr_template.format(
226-
CLS=self.__class__.__name__,
227-
TID=self.task_id,
228-
CID=self.config_id,
216+
return (
217+
f'<{self.__class__.__name__}(task_id="{self.task_id}", '
218+
f'config_id="{self.config_id}")>'
229219
)
230220

231221

@@ -241,11 +231,9 @@ class PushNotificationConfigModel(PushNotificationConfigMixin, base): # type: i
241231
@override
242232
def __repr__(self) -> str:
243233
"""Return a string representation of the push notification config."""
244-
repr_template = '<PushNotificationConfigModel[{TABLE}](task_id="{TID}", config_id="{CID}")>'
245-
return repr_template.format(
246-
TABLE=table_name,
247-
TID=self.task_id,
248-
CID=self.config_id,
234+
return (
235+
f'<PushNotificationConfigModel[{table_name}]('
236+
f'task_id="{self.task_id}", config_id="{self.config_id}")>'
249237
)
250238

251239
PushNotificationConfigModel.__name__ = (

src/a2a/server/request_handlers/default_request_handler.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,9 @@ def _validate_task_id_match(self, task_id: str, event_task_id: str) -> None:
244244
"""Validates that agent-generated task ID matches the expected task ID."""
245245
if task_id != event_task_id:
246246
logger.error(
247-
f'Agent generated task_id={event_task_id} does not match the RequestContext task_id={task_id}.'
247+
'Agent generated task_id=%s does not match the RequestContext task_id=%s.',
248+
event_task_id,
249+
task_id,
248250
)
249251
raise ServerError(
250252
InternalError(message='Task ID mismatch in agent response')

src/a2a/server/tasks/base_push_notification_sender.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ async def send_notification(self, task: Task) -> None:
4444

4545
if not all(results):
4646
logger.warning(
47-
f'Some push notifications failed to send for task_id={task.id}'
47+
'Some push notifications failed to send for task_id=%s', task.id
4848
)
4949

5050
async def _dispatch_notification(
@@ -62,11 +62,13 @@ async def _dispatch_notification(
6262
)
6363
response.raise_for_status()
6464
logger.info(
65-
f'Push-notification sent for task_id={task.id} to URL: {url}'
65+
'Push-notification sent for task_id=%s to URL: %s', task.id, url
6666
)
6767
except Exception:
6868
logger.exception(
69-
f'Error sending push-notification for task_id={task.id} to URL: {url}.'
69+
'Error sending push-notification for task_id=%s to URL: %s.',
70+
task.id,
71+
url,
7072
)
7173
return False
7274
return True

src/a2a/server/tasks/database_push_notification_config_store.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ def __init__(
7878
The key must be a URL-safe base64-encoded 32-byte key.
7979
"""
8080
logger.debug(
81-
f'Initializing DatabasePushNotificationConfigStore with existing engine, table: {table_name}'
81+
'Initializing DatabasePushNotificationConfigStore with existing engine, table: %s',
82+
table_name,
8283
)
8384
self.engine = engine
8485
self.async_session_maker = async_sessionmaker(
@@ -235,7 +236,9 @@ async def set_info(
235236
async with self.async_session_maker.begin() as session:
236237
await session.merge(db_config)
237238
logger.debug(
238-
f'Push notification config for task {task_id} with config id {config_to_save.id} saved/updated.'
239+
'Push notification config for task %s with config id %s saved/updated.',
240+
task_id,
241+
config_to_save.id,
239242
)
240243

241244
async def get_info(self, task_id: str) -> list[PushNotificationConfig]:
@@ -280,9 +283,13 @@ async def delete_info(
280283

281284
if result.rowcount > 0:
282285
logger.info(
283-
f'Deleted {result.rowcount} push notification config(s) for task {task_id}.'
286+
'Deleted %s push notification config(s) for task %s.',
287+
result.rowcount,
288+
task_id,
284289
)
285290
else:
286291
logger.warning(
287-
f'Attempted to delete push notification config for task {task_id} with config_id: {config_id} that does not exist.'
292+
'Attempted to delete push notification config for task %s with config_id: %s that does not exist.',
293+
task_id,
294+
config_id,
288295
)

0 commit comments

Comments
 (0)