Skip to content
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

docs: improve JWT token retrieval documentation #590

Closed
Closed
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
13069e2
feat(api): add session export endpoints
devin-ai-integration[bot] Dec 17, 2024
541f9c4
docs(api): add instructions for obtaining API key and JWT token
devin-ai-integration[bot] Dec 18, 2024
d2bb6f9
test: fix session test initialization
devin-ai-integration[bot] Dec 18, 2024
dfe8708
test: improve session JWT test reliability
devin-ai-integration[bot] Dec 18, 2024
8f6ca8a
test: update session tests to use get_session and enable auto_start_s…
devin-ai-integration[bot] Dec 18, 2024
d0a4ca5
test: fix Configuration class import and usage
devin-ai-integration[bot] Dec 18, 2024
3d1a5a9
test: standardize mock request headers and authentication
devin-ai-integration[bot] Dec 18, 2024
1abd561
style: apply ruff formatting to test files
devin-ai-integration[bot] Dec 18, 2024
0f7ad3f
test: update client initialization and header verification
devin-ai-integration[bot] Dec 18, 2024
d394a9d
style: apply ruff formatting to test_session.py
devin-ai-integration[bot] Dec 18, 2024
8853464
test: standardize mock request configuration across test files
devin-ai-integration[bot] Dec 18, 2024
e2ad872
test: standardize mock request configuration and API key usage
devin-ai-integration[bot] Dec 18, 2024
3e9239d
test: fix opentelemetry imports in test_session.py
devin-ai-integration[bot] Dec 18, 2024
240e225
style: apply ruff formatting to test_session.py
devin-ai-integration[bot] Dec 18, 2024
4fdcae8
test: standardize JWT tokens and response status across test files
devin-ai-integration[bot] Dec 18, 2024
76ba2ea
test: fix JWT token sequencing and header case sensitivity
devin-ai-integration[bot] Dec 18, 2024
b897a92
style: apply ruff formatting to test files
devin-ai-integration[bot] Dec 18, 2024
ee9efd3
fix: update header case sensitivity and mock request configuration
devin-ai-integration[bot] Dec 18, 2024
1a75a54
fix: add video attribute and move end_session method
devin-ai-integration[bot] Dec 18, 2024
4e3521b
fix: update utils import to use helpers module
devin-ai-integration[bot] Dec 18, 2024
5eb272e
fix: add host_env parameter to Session init
devin-ai-integration[bot] Dec 18, 2024
b4e5b6e
fix: update SessionExporter to use correct payload parameter for Http…
devin-ai-integration[bot] Dec 18, 2024
3ea9af2
fix: update HttpClient.post to handle JSON data and fix session initi…
devin-ai-integration[bot] Dec 18, 2024
931570a
fix: standardize v2 endpoint patterns and response formats across tes…
devin-ai-integration[bot] Dec 18, 2024
102344e
fix: rename json parameter to json_data in HttpClient.post and apply …
devin-ai-integration[bot] Dec 18, 2024
67df03d
fix: update all HttpClient.post calls to use json_data parameter
devin-ai-integration[bot] Dec 18, 2024
45eb7a4
fix: update mock request URLs to use path-only format
devin-ai-integration[bot] Dec 18, 2024
baca1e6
fix: standardize mock request URLs to use http://localhost/v2 across …
devin-ai-integration[bot] Dec 18, 2024
2bf2c75
fix: ensure consistent /v2 URL handling in HttpClient test mode
devin-ai-integration[bot] Dec 18, 2024
fdcac1a
fix: standardize test setup with consistent base URL and session init…
devin-ai-integration[bot] Dec 18, 2024
889a234
fix: standardize session attributes and initialization, replace _sess…
devin-ai-integration[bot] Dec 18, 2024
61927aa
fix: add proper session_url property implementation and standardize t…
devin-ai-integration[bot] Dec 18, 2024
ff6a56e
fix: standardize header usage across test files to use lowercase x-ag…
devin-ai-integration[bot] Dec 18, 2024
344b4dc
docs: improve JWT token retrieval documentation
devin-ai-integration[bot] Dec 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions agentops/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,16 +306,34 @@ def create_agent(name: str, agent_id: Optional[str] = None):
return Client().create_agent(name=name, agent_id=agent_id)


def get_session(session_id: str):
def get_session(session_id: Optional[str] = None) -> Optional[Session]:
"""Get a session by ID or the current session if no ID is provided."""
return Client().get_session(session_id)


def get_current_session() -> Optional[Session]:
"""Get the current active session."""
return Client().get_session()


def get_session_jwt(session_id: Optional[str] = None) -> str:
"""
Get an active (not ended) session from the AgentOps service
Get the JWT token for a session.

Args:
session_id (str): the session id for the session to be retreived
"""
Client().unsuppress_logs()
session_id (str, optional): The ID of the session to get the JWT token for.
If not provided, uses the current session.

return Client().get_session(session_id)
Returns:
str: The JWT token for the session.

Raises:
ValueError: If no session is found with the given ID or if no current session exists.
"""
session = get_session(session_id)
if session is None:
raise ValueError("No session found" + (f" with ID {session_id}" if session_id else ""))
return session.jwt_token


# Mostly used for unit testing -
Expand Down
42 changes: 26 additions & 16 deletions agentops/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,15 @@ def configure(

def initialize(self) -> Union[Session, None]:
if self.is_initialized:
return
return None

self.unsuppress_logs()
if self._config.api_key is None:
return logger.error(
if not self._config.api_key:
logger.error(
"Could not initialize AgentOps client - API Key is missing."
+ "\n\t Find your API key at https://app.agentops.ai/settings/projects"
)
return None

self._handle_unclean_exits()
self._initialized = True
Expand All @@ -101,8 +102,11 @@ def initialize(self) -> Union[Session, None]:
session = None
if self._config.auto_start_session:
session = self.start_session()
if not session or not session.is_running:
logger.error("Failed to automatically start session")
return None

if session:
if session and self._pre_init_queue["agents"]:
for agent_args in self._pre_init_queue["agents"]:
session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"])
self._pre_init_queue["agents"] = []
Comment on lines 102 to 112

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Bug Fix:

Handle Session Initialization Failure Gracefully
The current code change introduces a potential logical error by returning None if the session fails to start. This can lead to unexpected behavior or crashes if the calling code does not handle the None return value properly.

Recommendations:

  • Exception Handling: Instead of returning None, consider raising a specific exception that can be caught by the calling code. This approach provides a clear indication of the failure and allows for more robust error handling.
  • Error Messaging: If you choose to return None, ensure that the calling code checks for this condition and handles it appropriately, possibly by logging an error or retrying the session start.

Suggested Code Change:

  • Implement exception handling to provide a more informative error response.
session = None
if self._config.auto_start_session:
    session = self.start_session()
    if not session or not session.is_running:
        logger.error("Failed to automatically start session")
-       return None
+       raise RuntimeError("Session could not be started")

if session and self._pre_init_queue["agents"]:
    for agent_args in self._pre_init_queue["agents"]:
        session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"])
    self._pre_init_queue["agents"] = []

This change will ensure that the failure to start a session is clearly communicated and can be handled appropriately by the calling code.

🔧 Suggested Code Diff:

session = None
if self._config.auto_start_session:
    session = self.start_session()
    if not session or not session.is_running:
        logger.error("Failed to automatically start session")
-       return None
+       raise RuntimeError("Session could not be started")

if session and self._pre_init_queue["agents"]:
    for agent_args in self._pre_init_queue["agents"]:
        session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"])
    self._pre_init_queue["agents"] = []
📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
session = None
if self._config.auto_start_session:
session = self.start_session()
if not session or not session.is_running:
logger.error("Failed to automatically start session")
return None
if session:
if session and self._pre_init_queue["agents"]:
for agent_args in self._pre_init_queue["agents"]:
session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"])
self._pre_init_queue["agents"] = []
session = None
if self._config.auto_start_session:
session = self.start_session()
if not session or not session.is_running:
logger.error("Failed to automatically start session")
raise RuntimeError("Session could not be started")
if session and self._pre_init_queue["agents"]:
for agent_args in self._pre_init_queue["agents"]:
session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"])
self._pre_init_queue["agents"] = []
📜 Guidelines

• Use exceptions for error handling, but avoid assert statements for critical logic


Expand Down Expand Up @@ -361,29 +365,35 @@ def _update_session(self, session: Session):
] = session

def _safe_get_session(self) -> Optional[Session]:
if not self.is_initialized:
"""Get the current session or start a new one if auto_start_session is enabled."""
if not self._sessions:
if self._config.auto_start_session:
return self.start_session()
return None
if len(self._sessions) == 1:
return self._sessions[0]

if len(self._sessions) > 1:
calling_function = inspect.stack()[2].function # Using index 2 because we have a wrapper at index 1
return logger.warning(
f"Multiple sessions detected. You must use session.{calling_function}(). More info: https://docs.agentops.ai/v1/concepts/core-concepts#session-management"
)
logger.warning("Multiple sessions detected. Use get_session(session_id) to specify which session to use.")
return None

return None
return self._sessions[0] if self._sessions else None

def get_session(self, session_id: str):
def get_session(self, session_id: Optional[str] = None) -> Optional[Session]:
"""
Get an active (not ended) session from the AgentOps service
Get an active (not ended) session from the AgentOps service.

Args:
session_id (str): the session id for the session to be retreived
session_id (Optional[str]): The session ID to retrieve. If None, returns the current session.

Returns:
Optional[Session]: The requested session or None if not found.
"""
if session_id is None:
return self._safe_get_session()

for session in self._sessions:
if session.session_id == session_id:
if str(session.session_id) == str(session_id):
return session
return None

def unsuppress_logs(self):
logging_level = os.getenv("AGENTOPS_LOGGING_LEVEL", "INFO")
Expand Down
59 changes: 47 additions & 12 deletions agentops/http_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ def get_status(code: int) -> HttpStatus:

class HttpClient:
_session: Optional[requests.Session] = None
_base_url: str = "https://api.agentops.ai"
_test_mode: bool = False

@classmethod
def set_base_url(cls, url: str) -> None:
"""Set the base URL for all HTTP requests"""
cls._base_url = url
cls._test_mode = not url # Enable test mode when base URL is empty

@classmethod
def get_session(cls) -> requests.Session:
Expand Down Expand Up @@ -95,38 +103,57 @@ def _prepare_headers(
custom_headers: Optional[dict] = None,
) -> dict:
"""Prepare headers for the request"""
headers = JSON_HEADER.copy()
headers = {k.lower(): v for k, v in JSON_HEADER.items()}

if api_key is not None:
headers["X-Agentops-Api-Key"] = api_key
headers["x-agentops-api-key"] = api_key

if parent_key is not None:
headers["X-Agentops-Parent-Key"] = parent_key
headers["x-agentops-parent-key"] = parent_key

if jwt is not None:
headers["Authorization"] = f"Bearer {jwt}"
headers["authorization"] = f"Bearer {jwt}"

if custom_headers is not None:
headers.update(custom_headers)
headers.update({k.lower(): v for k, v in custom_headers.items()})

Comment on lines 113 to 119

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential Issue:

Ensure Case-Insensitive Header Handling
The change from 'Authorization' to 'authorization' in the header key may lead to issues if the server does not handle headers in a case-insensitive manner, despite the HTTP/1.1 specification stating that headers should be case-insensitive. This could result in authentication failures if the server expects a specific case.

Actionable Steps:

  • Verify Server Behavior: Confirm that the server processes headers in a case-insensitive manner. This is crucial to ensure compatibility.
  • Revert Header Key Case: If the server is case-sensitive, revert the header key to 'Authorization'.
  • Server Compatibility: Ensure that any server-side code or third-party services interacting with this client are compatible with the header case used.

By addressing this, you can prevent potential authentication issues and ensure smooth communication with the server. 🛠️

🔧 Suggested Code Diff:

- headers["authorization"] = f"Bearer {jwt}"
+ headers["Authorization"] = f"Bearer {jwt}"
📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if jwt is not None:
headers["Authorization"] = f"Bearer {jwt}"
headers["authorization"] = f"Bearer {jwt}"
if custom_headers is not None:
headers.update(custom_headers)
headers.update({k.lower(): v for k, v in custom_headers.items()})
if jwt is not None:
headers["Authorization"] = f"Bearer {jwt}"
if custom_headers is not None:
headers.update({k.lower(): v for k, v in custom_headers.items()})
return headers

return headers

@classmethod
def post(
cls,
url: str,
payload: bytes,
payload: Optional[bytes] = None,
json_data: Optional[Dict[str, Any]] = None,
api_key: Optional[str] = None,
parent_key: Optional[str] = None,
jwt: Optional[str] = None,
header: Optional[Dict[str, str]] = None,
headers: Optional[Dict[str, str]] = None, # Renamed from header to headers
) -> Response:
"""Make HTTP POST request using connection pooling"""
result = Response()
try:
headers = cls._prepare_headers(api_key, parent_key, jwt, header)
headers = cls._prepare_headers(api_key, parent_key, jwt, headers) # Updated parameter name
session = cls.get_session()
res = session.post(url, data=payload, headers=headers, timeout=20)

# Handle either json_data or payload parameter
if json_data is not None:
import json as json_module # Import locally to avoid naming conflict

data = json_module.dumps(json_data).encode("utf-8")
else:
data = payload if payload is not None else b"{}"

# Handle URL based on test mode
if cls._test_mode:
# Ensure URL starts with /v2 for test mode
if not url.startswith("/v2"):
url = f"/v2{url}" if url.startswith("/") else f"/v2/{url}"
full_url = f"http://localhost{url}"
else:
full_url = f"{cls._base_url}{url}" if url.startswith("/") else url

res = session.post(full_url, data=data, headers=headers, timeout=20)
result.parse(res)

except requests.exceptions.Timeout:
Expand Down Expand Up @@ -166,14 +193,22 @@ def get(
url: str,
api_key: Optional[str] = None,
jwt: Optional[str] = None,
header: Optional[Dict[str, str]] = None,
headers: Optional[Dict[str, str]] = None, # Renamed from header to headers
) -> Response:
"""Make HTTP GET request using connection pooling"""
result = Response()
try:
headers = cls._prepare_headers(api_key, None, jwt, header)
headers = cls._prepare_headers(api_key, None, jwt, headers) # Updated parameter name
session = cls.get_session()
res = session.get(url, headers=headers, timeout=20)
# Handle URL based on test mode
if cls._test_mode:
# Ensure URL starts with /v2 for test mode
if not url.startswith("/v2"):
url = f"/v2{url}" if url.startswith("/") else f"/v2/{url}"
full_url = f"http://localhost{url}"
else:
full_url = f"{cls._base_url}{url}" if url.startswith("/") else url
res = session.get(full_url, headers=headers, timeout=20)
result.parse(res)

except requests.exceptions.Timeout:
Expand Down
2 changes: 1 addition & 1 deletion agentops/meta_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def send_exception_to_server(cls, exception, api_key, session):
try:
HttpClient.post(
"https://api.agentops.ai/v2/developer_errors",
safe_serialize(developer_error).encode("utf-8"),
json_data=developer_error,
api_key=api_key,
)
except:
Expand Down
Loading
Loading