Skip to content

Commit 87c1c12

Browse files
CopilotnikhilNava
andcommitted
Fix ObservabilityHostingManager.configure to accept MiddlewareSet instead of ChannelAdapter
ChannelAdapter is an ABC (CloudAdapter extends ChannelServiceAdapter extends ChannelAdapter). The adapter HAS-A MiddlewareSet (composition) — it is not one. The configure() method only needs the middleware registration object, so accept MiddlewareSet directly. Users pass adapter.middleware_set. Rename parameter from 'adapter' to 'middleware_set' for clarity. Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>
1 parent a297348 commit 87c1c12

File tree

2 files changed

+36
-35
lines changed

2 files changed

+36
-35
lines changed

libraries/microsoft-agents-a365-observability-hosting/microsoft_agents_a365/observability/hosting/middleware/observability_hosting_manager.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class ObservabilityHostingManager:
3333
Example:
3434
.. code-block:: python
3535
36-
ObservabilityHostingManager.configure(adapter, ObservabilityHostingOptions(
36+
ObservabilityHostingManager.configure(adapter.middleware_set, ObservabilityHostingOptions(
3737
enable_output_logging=True,
3838
))
3939
"""
@@ -46,25 +46,26 @@ def __init__(self) -> None:
4646
@classmethod
4747
def configure(
4848
cls,
49-
adapter: MiddlewareSet,
49+
middleware_set: MiddlewareSet,
5050
options: ObservabilityHostingOptions,
5151
) -> ObservabilityHostingManager:
52-
"""Configure the singleton instance and register middleware on the adapter.
52+
"""Configure the singleton instance and register middleware.
5353
5454
Subsequent calls after the first are no-ops and return the existing instance.
5555
5656
Args:
57-
adapter: The middleware set to register middleware on.
57+
middleware_set: The middleware set to register middleware on
58+
(e.g., ``adapter.middleware_set``).
5859
options: Configuration options controlling which middleware to enable.
5960
6061
Returns:
6162
The singleton :class:`ObservabilityHostingManager` instance.
6263
6364
Raises:
64-
TypeError: If *adapter* or *options* is ``None``.
65+
TypeError: If *middleware_set* or *options* is ``None``.
6566
"""
66-
if adapter is None:
67-
raise TypeError("adapter must not be None")
67+
if middleware_set is None:
68+
raise TypeError("middleware_set must not be None")
6869
if options is None:
6970
raise TypeError("options must not be None")
7071

@@ -78,11 +79,11 @@ def configure(
7879
instance = cls()
7980

8081
if options.enable_baggage:
81-
adapter.use(BaggageMiddleware())
82+
middleware_set.use(BaggageMiddleware())
8283
logger.info("[ObservabilityHostingManager] BaggageMiddleware registered.")
8384

8485
if options.enable_output_logging:
85-
adapter.use(OutputLoggingMiddleware())
86+
middleware_set.use(OutputLoggingMiddleware())
8687
logger.info("[ObservabilityHostingManager] OutputLoggingMiddleware registered.")
8788

8889
logger.info(

tests/observability/hosting/middleware/test_observability_hosting_manager.py

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,71 +26,71 @@ def _reset_singleton():
2626

2727
def test_configure_returns_instance():
2828
"""configure() should return an ObservabilityHostingManager instance."""
29-
adapter = MagicMock()
30-
instance = ObservabilityHostingManager.configure(adapter, ObservabilityHostingOptions())
29+
middleware_set = MagicMock()
30+
instance = ObservabilityHostingManager.configure(middleware_set, ObservabilityHostingOptions())
3131
assert isinstance(instance, ObservabilityHostingManager)
3232

3333

3434
def test_configure_is_singleton():
3535
"""Subsequent configure() calls should return the same instance."""
36-
adapter = MagicMock()
36+
middleware_set = MagicMock()
3737
options = ObservabilityHostingOptions()
38-
first = ObservabilityHostingManager.configure(adapter, options)
39-
second = ObservabilityHostingManager.configure(adapter, options)
38+
first = ObservabilityHostingManager.configure(middleware_set, options)
39+
second = ObservabilityHostingManager.configure(middleware_set, options)
4040
assert first is second
4141

4242

4343
def test_configure_registers_baggage_middleware_by_default():
4444
"""By default, BaggageMiddleware should be registered."""
45-
adapter = MagicMock()
46-
ObservabilityHostingManager.configure(adapter, ObservabilityHostingOptions())
45+
middleware_set = MagicMock()
46+
ObservabilityHostingManager.configure(middleware_set, ObservabilityHostingOptions())
4747

48-
# The adapter.use should have been called once (only BaggageMiddleware by default)
49-
assert adapter.use.call_count == 1
50-
registered = adapter.use.call_args_list[0][0][0]
48+
# The middleware_set.use should have been called once (only BaggageMiddleware by default)
49+
assert middleware_set.use.call_count == 1
50+
registered = middleware_set.use.call_args_list[0][0][0]
5151
assert isinstance(registered, BaggageMiddleware)
5252

5353

5454
def test_configure_registers_both_middlewares():
5555
"""When output logging is enabled, both middlewares should be registered."""
56-
adapter = MagicMock()
56+
middleware_set = MagicMock()
5757
options = ObservabilityHostingOptions(enable_baggage=True, enable_output_logging=True)
58-
ObservabilityHostingManager.configure(adapter, options)
58+
ObservabilityHostingManager.configure(middleware_set, options)
5959

60-
assert adapter.use.call_count == 2
61-
registered_types = [c[0][0] for c in adapter.use.call_args_list]
60+
assert middleware_set.use.call_count == 2
61+
registered_types = [c[0][0] for c in middleware_set.use.call_args_list]
6262
assert isinstance(registered_types[0], BaggageMiddleware)
6363
assert isinstance(registered_types[1], OutputLoggingMiddleware)
6464

6565

6666
def test_configure_disables_baggage():
6767
"""When baggage is disabled, only output logging should be registered (if enabled)."""
68-
adapter = MagicMock()
68+
middleware_set = MagicMock()
6969
options = ObservabilityHostingOptions(enable_baggage=False, enable_output_logging=True)
70-
ObservabilityHostingManager.configure(adapter, options)
70+
ObservabilityHostingManager.configure(middleware_set, options)
7171

72-
assert adapter.use.call_count == 1
73-
registered = adapter.use.call_args_list[0][0][0]
72+
assert middleware_set.use.call_count == 1
73+
registered = middleware_set.use.call_args_list[0][0][0]
7474
assert isinstance(registered, OutputLoggingMiddleware)
7575

7676

7777
def test_configure_disables_all():
7878
"""When both are disabled, no middleware should be registered."""
79-
adapter = MagicMock()
79+
middleware_set = MagicMock()
8080
options = ObservabilityHostingOptions(enable_baggage=False, enable_output_logging=False)
81-
ObservabilityHostingManager.configure(adapter, options)
81+
ObservabilityHostingManager.configure(middleware_set, options)
8282

83-
adapter.use.assert_not_called()
83+
middleware_set.use.assert_not_called()
8484

8585

86-
def test_configure_raises_on_none_adapter():
87-
"""configure() should raise TypeError when adapter is None."""
88-
with pytest.raises(TypeError, match="adapter must not be None"):
86+
def test_configure_raises_on_none_middleware_set():
87+
"""configure() should raise TypeError when middleware_set is None."""
88+
with pytest.raises(TypeError, match="middleware_set must not be None"):
8989
ObservabilityHostingManager.configure(None, ObservabilityHostingOptions())
9090

9191

9292
def test_configure_raises_on_none_options():
9393
"""configure() should raise TypeError when options is None."""
94-
adapter = MagicMock()
94+
middleware_set = MagicMock()
9595
with pytest.raises(TypeError, match="options must not be None"):
96-
ObservabilityHostingManager.configure(adapter, None)
96+
ObservabilityHostingManager.configure(middleware_set, None)

0 commit comments

Comments
 (0)