-
Notifications
You must be signed in to change notification settings - Fork 509
Consolidate initialization and kwargs passing for AgentOps client #728
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,5 @@ | ||||||||||
| from typing import List, Optional | ||||||||||
| import os | ||||||||||
| from typing import Any, List, Optional | ||||||||||
| from uuid import UUID | ||||||||||
|
|
||||||||||
| from .log_config import logger | ||||||||||
|
|
@@ -16,6 +17,8 @@ def __init__(self): | |||||||||
| self.auto_start_session: bool = True | ||||||||||
| self.skip_auto_end_session: bool = False | ||||||||||
| self.env_data_opt_out: bool = False | ||||||||||
| self.exporter: Optional[Any] = None | ||||||||||
| self.exporter_endpoint: Optional[str] = None | ||||||||||
|
|
||||||||||
| def configure( | ||||||||||
| self, | ||||||||||
|
|
@@ -30,6 +33,9 @@ def configure( | |||||||||
| auto_start_session: Optional[bool] = None, | ||||||||||
| skip_auto_end_session: Optional[bool] = None, | ||||||||||
| env_data_opt_out: Optional[bool] = None, | ||||||||||
| exporter: Optional[Any] = None, | ||||||||||
| exporter_endpoint: Optional[str] = None, | ||||||||||
| **kwargs, | ||||||||||
| ): | ||||||||||
| if api_key is not None: | ||||||||||
| try: | ||||||||||
|
|
@@ -72,3 +78,11 @@ def configure( | |||||||||
|
|
||||||||||
| if env_data_opt_out is not None: | ||||||||||
| self.env_data_opt_out = env_data_opt_out | ||||||||||
|
|
||||||||||
| if exporter is not None: | ||||||||||
| self.exporter = exporter | ||||||||||
|
|
||||||||||
| if exporter_endpoint is not None: | ||||||||||
| self.exporter_endpoint = exporter_endpoint | ||||||||||
| elif os.environ.get("AGENTOPS_EXPORTER_ENDPOINT") is not None: | ||||||||||
| self.exporter_endpoint = os.environ.get("AGENTOPS_EXPORTER_ENDPOINT") | ||||||||||
|
Comment on lines
+87
to
+88
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Environment variable access is not type-safe - 📝 Committable Code Suggestion
Suggested change
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| import os | ||
| import pytest | ||
| from unittest.mock import MagicMock, patch | ||
| from opentelemetry.sdk.trace.export import SpanExporter | ||
|
|
||
| import agentops | ||
| from agentops import Client | ||
| from agentops.singleton import clear_singletons | ||
|
|
||
|
|
||
| class TestCustomExporterConfig: | ||
| def setup_method(self): | ||
| self.api_key = "11111111-1111-4111-8111-111111111111" | ||
| clear_singletons() | ||
|
|
||
| def teardown_method(self): | ||
| """Clean up after each test""" | ||
| agentops.end_all_sessions() | ||
| clear_singletons() | ||
|
|
||
| def test_custom_exporter(self, mock_req): | ||
| """Test that a custom exporter can be used""" | ||
| # Create a mock exporter | ||
| mock_exporter = MagicMock(spec=SpanExporter) | ||
|
|
||
| # Initialize agentops with the mock exporter | ||
| # This will fail until the implementation is complete | ||
| agentops.init(api_key=self.api_key, exporter=mock_exporter, auto_start_session=True) | ||
|
|
||
| # Verify that the mock exporter was used | ||
| session = Client()._safe_get_session() | ||
| assert session is not None | ||
|
|
||
| # This assertion will fail until the implementation is complete | ||
| # The expected behavior is that the custom exporter is used instead of the default | ||
| assert session._otel_exporter == mock_exporter | ||
|
|
||
| # Clean up | ||
| agentops.end_session("Success") | ||
|
|
||
| def test_exporter_endpoint(self, mock_req): | ||
| """Test that exporter_endpoint is correctly configured""" | ||
| # Initialize agentops with a custom exporter_endpoint | ||
| custom_endpoint = "https://custom.endpoint/api" | ||
|
|
||
| # This will fail until the implementation is complete | ||
| agentops.init(api_key=self.api_key, exporter_endpoint=custom_endpoint, auto_start_session=True) | ||
|
|
||
| # Verify that the exporter_endpoint was correctly configured | ||
| session = Client()._safe_get_session() | ||
| assert session is not None | ||
|
|
||
| # These assertions will fail until the implementation is complete | ||
| # The expected behavior is that the custom endpoint is stored in the config | ||
| assert session.config.exporter_endpoint == custom_endpoint | ||
|
|
||
| # The SessionExporter should use the custom endpoint for its endpoint property | ||
| # The endpoint property in SessionExporter should be updated to use config.exporter_endpoint if set | ||
| full_endpoint = f"{custom_endpoint}/v2/create_events" | ||
| assert session._otel_exporter.endpoint == full_endpoint | ||
|
|
||
| # Clean up | ||
| agentops.end_session("Success") | ||
|
|
||
| def test_environment_variable_exporter_endpoint(self, mock_req, monkeypatch): | ||
| """Test that exporter_endpoint from environment variable is correctly configured""" | ||
| # Set environment variable | ||
| custom_endpoint = "https://env.endpoint/api" | ||
| monkeypatch.setenv("AGENTOPS_EXPORTER_ENDPOINT", custom_endpoint) | ||
|
|
||
| # Initialize agentops without explicitly setting exporter_endpoint | ||
| # This will fail until the implementation is complete | ||
| agentops.init(api_key=self.api_key, auto_start_session=True) | ||
|
|
||
| # Verify that the exporter_endpoint from env var was correctly configured | ||
| session = Client()._safe_get_session() | ||
| assert session is not None | ||
|
|
||
| # These assertions will fail until the implementation is complete | ||
| # The expected behavior is that the environment variable is used for the endpoint | ||
| assert session.config.exporter_endpoint == custom_endpoint | ||
|
|
||
| # The SessionExporter should use the environment variable endpoint | ||
| full_endpoint = f"{custom_endpoint}/v2/create_events" | ||
| assert session._otel_exporter.endpoint == full_endpoint | ||
|
|
||
| # Clean up | ||
| agentops.end_session("Success") | ||
|
|
||
| def test_kwargs_passing(self, mock_req): | ||
| """Test that additional kwargs are passed through the initialization chain""" | ||
| # Initialize agentops with additional kwargs | ||
| # This will fail until the implementation is complete | ||
| agentops.init(api_key=self.api_key, auto_start_session=True, custom_param1="value1", custom_param2=42) | ||
|
|
||
| # Verify session was created | ||
| session = Client()._safe_get_session() | ||
| assert session is not None | ||
|
|
||
| # The expected behavior is that the kwargs are passed through the initialization chain | ||
| # and stored in the configuration object | ||
|
|
||
| # Clean up | ||
| agentops.end_session("Success") | ||
|
|
||
| def test_custom_exporter_with_endpoint(self, mock_req): | ||
| """Test that a custom exporter can be used with a custom endpoint""" | ||
| # Create a mock exporter | ||
| mock_exporter = MagicMock(spec=SpanExporter) | ||
| custom_endpoint = "https://custom.endpoint/api" | ||
|
|
||
| # Initialize agentops with both custom exporter and endpoint | ||
| # This will fail until the implementation is complete | ||
| agentops.init( | ||
| api_key=self.api_key, exporter=mock_exporter, exporter_endpoint=custom_endpoint, auto_start_session=True | ||
| ) | ||
|
|
||
| # Verify that the mock exporter was used and endpoint was set | ||
| session = Client()._safe_get_session() | ||
| assert session is not None | ||
|
|
||
| # These assertions will fail until the implementation is complete | ||
| # The expected behavior is that the custom exporter is used and endpoint is set | ||
| assert session._otel_exporter == mock_exporter | ||
| assert session.config.exporter_endpoint == custom_endpoint | ||
|
|
||
| # Clean up | ||
| agentops.end_session("Success") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
exporterparameter acceptsAnytype without validation, which could lead to runtime errors. Should validate the exporter has required methods/interface.📝 Committable Code Suggestion