Added export of connection basic metrics #3891

vladvildanov · 2025-12-19T12:28:49Z

Description of change

Please provide a description of the change here.

Pull Request check-list

Please make sure to review and check all of these items:

Do tests and lints pass with this change?
Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
Is the new or changed code fully tested?
Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
Is there an example added to the examples folder (if applicable)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Copilot

Pull request overview

This pull request adds export of connection basic metrics to the Redis client by refactoring the connection count metric from an UpDownCounter to an Observable Gauge and implementing an event-based system for collecting connection lifecycle metrics.

Key Changes

Refactored connection count metric from UpDownCounter to Observable Gauge with callback-based collection
Added event dispatching for connection lifecycle events (created, timeout relaxed, handoff)
Integrated event listeners to export metrics for maintenance notifications and connection operations
Updated error type formatting to include "other:" prefix for better categorization

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
redis/observability/recorder.py	Replaced `record_connection_count` with `init_connection_count` using callback pattern; updated parameter names
redis/observability/metrics.py	Changed connection count from UpDownCounter to Observable Gauge; updated parameter names
redis/observability/attributes.py	Added `DB_CLIENT_CONNECTION_NAME` attribute constant
redis/observability/providers.py	Added optional import handling for OpenTelemetry SDK
redis/event.py	Added new event classes and listener implementations for connection metrics
redis/connection.py	Added `get_connection_count` method and event dispatching in connection pools
redis/maint_notifications.py	Integrated event dispatcher and added event emission for maintenance operations
tests/test_observability/test_recorder.py	Updated mocks from `create_gauge` and removed `record_connection_count` tests
tests/test_observability/test_config.py	Removed unused telemetry option tests
tests/test_maint_notifications.py	Added comprehensive tests for event emission
tests/test_connection_pool.py	Added tests for connection creation event emission
tests/test_client.py	Added tests for connection count initialization

Comments suppressed due to low confidence (1)

redis/observability/recorder.py:109

The docstring example on line 109 is outdated. It still shows a string pool name being passed, but the function signature has been changed to accept a connection_pool object. Update the example to reflect the new signature.

    Example:
        >>> start = time.monotonic()
        >>> # ... create connection ...
        >>> record_connection_create_time('ConnectionPool<localhost:6379>', time.monotonic() - start)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-19T12:37:26Z

tests/test_observability/test_recorder.py

        return instrument_map.get(name, MagicMock())

    meter.create_counter.side_effect = create_counter_side_effect
+    meter.create_gauge.side_effect = create_gauge_side_effect


There is a mismatch between the test mocks and the implementation. The tests mock meter.create_gauge (lines 125, 732) but the implementation calls meter.create_observable_gauge (line 263 in redis/observability/metrics.py). This will cause the tests to fail because the mocked method doesn't match the actual method being called. Either update the tests to mock create_observable_gauge or update the implementation to use create_gauge.

Suggested change

meter.create_gauge.side_effect = create_gauge_side_effect

meter.create_gauge.side_effect = create_gauge_side_effect

meter.create_observable_gauge.side_effect = create_gauge_side_effect

Copilot · 2025-12-19T12:37:26Z

tests/test_observability/test_recorder.py

            }
            return instrument_map.get(name, MagicMock())

        mock_meter.create_counter.side_effect = create_counter_side_effect


There is a mismatch between the test mocks and the implementation. The tests mock meter.create_gauge but the implementation calls meter.create_observable_gauge (line 263 in redis/observability/metrics.py). This will cause the tests to fail because the mocked method doesn't match the actual method being called. Either update the tests to mock create_observable_gauge or update the implementation to use create_gauge.

Suggested change

mock_meter.create_counter.side_effect = create_counter_side_effect

mock_meter.create_counter.side_effect = create_counter_side_effect

# The RedisMetricsCollector uses create_observable_gauge in the implementation,

# so we need to mock that here to ensure the tests observe the correct behavior.

mock_meter.create_observable_gauge.side_effect = create_gauge_side_effect

# Keep create_gauge mocked as well in case it is used elsewhere.

Copilot · 2025-12-19T12:37:27Z

redis/maint_notifications.py

        self.connection.update_current_socket_timeout(self.config.relaxed_timeout)

-    def handle_maintenance_completed_notification(self):
+        if kwargs.get('notification', None) is not None:


The check kwargs.get('notification', None) is not None is redundant. The get method with a default of None already returns None if the key is missing, so checking is not None achieves the same result as just checking the truthiness. Consider simplifying to if kwargs.get('notification'): or better yet, use a more explicit parameter instead of **kwargs.

Copilot · 2025-12-19T12:37:27Z

redis/maint_notifications.py

        self.connection.update_current_socket_timeout(-1)
        self.connection.maintenance_state = MaintenanceState.NONE
+
+        if kwargs.get('notification', None) is not None:


The check kwargs.get('notification', None) is not None is redundant. The get method with a default of None already returns None if the key is missing, so checking is not None achieves the same result as just checking the truthiness. Consider simplifying to if kwargs.get('notification'): or better yet, use a more explicit parameter instead of **kwargs.

Copilot · 2025-12-19T12:37:27Z

redis/observability/metrics.py


        Args:
-            pool_name: Connection pool name
+            connection_name: Connection pool name


The docstring on line 436 incorrectly describes connection_name as "Connection pool name", but based on the parameter name and usage context, it should be "Connection name" or "Connection identifier". This is inconsistent with the actual parameter name change from pool_name to connection_name.

Suggested change

connection_name: Connection pool name

connection_name: Connection name

Copilot · 2025-12-19T12:37:28Z

redis/connection.py

 from .backoff import NoBackoff
 from .credentials import CredentialProvider, UsernamePasswordCredentialProvider
-from .event import AfterConnectionReleasedEvent, EventDispatcher, OnErrorEvent, OnMaintenanceNotificationEvent
+from .event import AfterConnectionReleasedEvent, EventDispatcher, OnErrorEvent, OnMaintenanceNotificationEvent, \


Import of 'OnMaintenanceNotificationEvent' is not used.

Suggested change

from .event import AfterConnectionReleasedEvent, EventDispatcher, OnErrorEvent, OnMaintenanceNotificationEvent, \

from .event import AfterConnectionReleasedEvent, EventDispatcher, OnErrorEvent, \

Copilot · 2025-12-19T12:37:28Z

redis/event.py

 from dataclasses import dataclass
 from enum import Enum
-from typing import Dict, List, Optional, Type, Union
+from typing import Dict, List, Optional, Type, Union, Callable


Import of 'Callable' is not used.

Suggested change

from typing import Dict, List, Optional, Type, Union, Callable

from typing import Dict, List, Optional, Type, Union

Copilot · 2025-12-19T12:37:29Z

redis/observability/metrics.py

 import logging
 import time
-from typing import Any, Dict, Optional
+from typing import Any, Dict, Optional, Callable, List


Import of 'Dict' is not used.
Import of 'Any' is not used.
Import of 'List' is not used.

Suggested change

from typing import Any, Dict, Optional, Callable, List

from typing import Optional, Callable

Copilot · 2025-12-19T12:37:29Z

redis/observability/recorder.py


 import time
-from typing import Optional
+from typing import Optional, Callable


Import of 'Callable' is not used.

Suggested change

from typing import Optional, Callable

from typing import Optional

Copilot · 2025-12-19T12:37:29Z

tests/test_client.py

+    AfterPooledConnectionsInstantiationEvent,
+    ClientType,
+    EventDispatcher,
+    EventListenerInterface,
+    InitializeConnectionCountObservability,


Import of 'AfterPooledConnectionsInstantiationEvent' is not used.
Import of 'ClientType' is not used.
Import of 'InitializeConnectionCountObservability' is not used.

Suggested change

AfterPooledConnectionsInstantiationEvent,

ClientType,

EventDispatcher,

EventListenerInterface,

InitializeConnectionCountObservability,

EventDispatcher,

EventListenerInterface,

Added export of connection basic metrics

0ff8046

vladvildanov requested review from Copilot and petyaslavova December 19, 2025 12:31

Copilot started reviewing on behalf of vladvildanov December 19, 2025 12:31 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

Added new error category attribute

02ef20c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Added export of connection basic metrics #3891

Added export of connection basic metrics #3891

vladvildanov commented Dec 19, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	meter.create_gauge.side_effect = create_gauge_side_effect
	meter.create_gauge.side_effect = create_gauge_side_effect
	meter.create_observable_gauge.side_effect = create_gauge_side_effect

-        mock_meter.create_counter.side_effect = create_counter_side_effect
+        mock_meter.create_counter.side_effect = create_counter_side_effect
+        # The RedisMetricsCollector uses create_observable_gauge in the implementation,
+        # so we need to mock that here to ensure the tests observe the correct behavior.
+        mock_meter.create_observable_gauge.side_effect = create_gauge_side_effect
+        # Keep create_gauge mocked as well in case it is used elsewhere.

	connection_name: Connection pool name
	connection_name: Connection name

	from .event import AfterConnectionReleasedEvent, EventDispatcher, OnErrorEvent, OnMaintenanceNotificationEvent, \
	from .event import AfterConnectionReleasedEvent, EventDispatcher, OnErrorEvent, \

	from typing import Dict, List, Optional, Type, Union, Callable
	from typing import Dict, List, Optional, Type, Union

	from typing import Any, Dict, Optional, Callable, List
	from typing import Optional, Callable

	from typing import Optional, Callable
	from typing import Optional

Added export of connection basic metrics #3891

Are you sure you want to change the base?

Added export of connection basic metrics #3891

Conversation

vladvildanov commented Dec 19, 2025

Description of change

Pull Request check-list

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants