ROB-1441 atlas mongodb toolset #557
Conversation
|
""" WalkthroughA new MongoDB Atlas toolset integration is introduced, providing tools for interacting with the Atlas API to retrieve alerts, events, slow queries, and logs. Supporting instruction and import changes are made, and the toolset is registered for use. Minor formatting adjustments are also applied to an unrelated context dictionary. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Holmes
participant MongoDBAtlasToolset
participant MongoDB Atlas API
User->>Holmes: Requests Atlas MongoDB info (alerts, slow queries, events, logs)
Holmes->>MongoDBAtlasToolset: Invokes appropriate tool (e.g., ReturnProjectAlerts)
MongoDBAtlasToolset->>MongoDB Atlas API: Authenticated API request
MongoDB Atlas API-->>MongoDBAtlasToolset: Returns API response (data or error)
MongoDBAtlasToolset-->>Holmes: Formats and returns tool result
Holmes-->>User: Presents results or error message
""" ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
holmes/plugins/toolsets/atlas_mongodb/instructions.jinja2 (1)
4-4: Fix typo in instructions."collscan" appears to be a typo or needs clarification.
If "collscan" refers to collection scans, consider writing it as "collection scan queries" for clarity:
-* For performance issues or collscan queries use atlas_return_project_slow_queries to see a list of slow queries. YOU MUST check this for all processes of the project. ALWAYS show the query in the result for every slow query. +* For performance issues or collection scan queries use atlas_return_project_slow_queries to see a list of slow queries. YOU MUST check this for all processes of the project. ALWAYS show the query in the result for every slow query.holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py (3)
31-31: Fix typo in description.- description: str = "The MongoDB Atlas API allows access to Mongodb projects and processes. You can find logs, alerts, events, slow quereies and various metrics to understand the state of Mongodb projects." + description: str = "The MongoDB Atlas API allows access to Mongodb projects and processes. You can find logs, alerts, events, slow queries and various metrics to understand the state of Mongodb projects."
150-150: Consider consistent tool naming.The tool name includes "processes" which doesn't match the naming pattern of other tools.
- name: str = "atlas_return_project_processes_slow_queries" + name: str = "atlas_return_project_slow_queries"
74-76: Provide example configuration for better user guidance.def get_example_config(self) -> Dict[str, Any]: - return {} + return { + "public_key": "your-atlas-public-key", + "private_key": "your-atlas-private-key", + "project_id": "your-project-id" + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
holmes/core/conversations.py(1 hunks)holmes/plugins/toolsets/__init__.py(2 hunks)holmes/plugins/toolsets/atlas_mongodb/instructions.jinja2(1 hunks)holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py(1 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py
[refactor] 22-22: Too few public methods (0/2)
(R0903)
[refactor] 85-100: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 196-214: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 251-263: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🪛 GitHub Actions: Build and test HolmesGPT
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py
[error] 239-239: mypy: Incompatible types in assignment (expression has type "int", variable has type "datetime") [assignment]
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (3.12)
🔇 Additional comments (2)
holmes/plugins/toolsets/__init__.py (1)
33-33: LGTM!The MongoDB Atlas toolset is properly imported and registered following the existing patterns.
Also applies to: 76-76
holmes/core/conversations.py (1)
267-269: Formatting change only.This reformatting improves readability without affecting functionality.
# Conflicts: # holmes/plugins/toolsets/__init__.py
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py (3)
143-144: Use consistent error handling pattern.Also applies to: 173-174, 211-211, 219-219, 259-260, 267-268, 302-303
201-201: Fix incorrect tool name reference.The data message references "atlas_return_events_type_from_project" but it should be the correct tool name.
225-225: Fix typo in class name.Also applies to: 47-47
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py (1)
holmes/core/tools.py (7)
CallablePrerequisite(307-308)Tool(110-157)ToolParameter(104-107)Toolset(320-462)StructuredToolResult(29-52)ToolResultStatus(23-26)_load_llm_instructions(457-462)
🪛 Pylint (3.3.7)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py
[refactor] 22-22: Too few public methods (0/2)
(R0903)
[refactor] 85-100: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 196-214: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 250-262: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build (3.12)
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
- GitHub Check: build (3.12)
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
- GitHub Check: llm_evals
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py (1)
145-147: Usedata=instead oferror=for error payloads (consistency with base pattern)Previous review already flagged this. Stick to one convention:
status=ERROR+data=<error message>.- error=f"Exception {self.name}: {str(e)}", + data=f"Exception {self.name}: {str(e)}",Apply in all highlighted locations.
Also applies to: 175-177, 214-215, 262-263, 270-271
🧹 Nitpick comments (5)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py (5)
31-33: Fix typo and inconsistent capitalization in public description
"slow quereies"→"slow queries"
Also, keep the product name consistently capitalised (MongoDB, notMongodb).- description: str = "The MongoDB Atlas API allows access to Mongodb projects and processes. You can find logs, alerts, events, slow quereies and various metrics to understand the state of Mongodb projects." + description: str = ( + "The MongoDB Atlas API allows access to MongoDB projects and processes. " + "You can find logs, alerts, events, slow queries and various metrics to understand " + "the state of MongoDB projects." + )
77-79: Provide a concrete example configurationAn empty dict defeats the purpose of
get_example_config(). Supply the keys a
user must set.- def get_example_config(self) -> Dict[str, Any]: - return {} + def get_example_config(self) -> Dict[str, Any]: + return { + "public_key": "ATLAS_PUBLIC_KEY", + "private_key": "ATLAS_PRIVATE_KEY", + "project_id": "64e7f8…", + }
84-104: Simplifyreturn_result; unreachable else branch
response.raise_for_status()raises for non-2xx codes, so the subsequent
if/elsealready guaranteesresponse.okis true; the else is dead code.
Also, the “error” path should probably usedata=noterror=per the agreed
pattern.- response.raise_for_status() - if response.ok: - … - else: - return StructuredToolResult( - status=ToolResultStatus.ERROR, - error=f"Failed {self.name}.\n{response.text}", - return_code=response.status_code, - params=params, - ) + response.raise_for_status() + res = response.json() + return StructuredToolResult( + status=ToolResultStatus.SUCCESS if res.get(field, []) else ToolResultStatus.NO_DATA, + data=res, + params=params, + )
155-161: Parameter description typos and clarity
- “procecess” → “processes”
- Avoid telling the caller which tool to call in a parameter description
(that belongs in overall instructions).- description="Combination of host and port that serves the MongoDB process. call tool atlas_return_project_processes tool to get host+port of project procecess.", + description="Combination of host and port that serves the MongoDB process (format: <hostname>:<port>).",
253-265: Remove redundantelseafter a guaranteed return
returninside theif response.okalready exits; theelsesection can be
unindented one level to improve readability (pylint R1705).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py (1)
holmes/core/tools.py (7)
CallablePrerequisite(307-308)Tool(110-157)ToolParameter(104-107)Toolset(320-462)StructuredToolResult(29-52)ToolResultStatus(23-26)_load_llm_instructions(457-462)
🪛 Pylint (3.3.7)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py
[refactor] 22-22: Too few public methods (0/2)
(R0903)
[refactor] 88-103: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 199-217: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 253-265: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build (3.10)
- GitHub Check: build (3.12)
- GitHub Check: build (3.11)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
- GitHub Check: llm_evals
🔇 Additional comments (1)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py (1)
241-252: ValidatestartDateformat for log endpointAtlas
getHostLogsexpectsstartDateISO-8601 (YYYY-MM-DDTHH:MM:SSZ)
or milliseconds since epoch, not seconds. Passing a plainintof seconds
may yield 400 errors.Request verification against the live API or docs before merging.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py (8)
31-31: Fix typo and inconsistent capitalization.- description: str = "The MongoDB Atlas API allows access to Mongodb projects and processes. You can find logs, alerts, events, slow quereies and various metrics to understand the state of Mongodb projects." + description: str = "The MongoDB Atlas API allows access to MongoDB projects and processes. You can find logs, alerts, events, slow queries and various metrics to understand the state of MongoDB projects."
37-37: Session object is created at import-time – make it instance-scoped.-_session: requests.Session = PrivateAttr(default=requests.Session()) +_session: requests.Session = PrivateAttr(default_factory=requests.Session)
47-48: Fix typo in class name.The class name
ReturnLogsForProcessInPorjecthas a typo. It should beReturnLogsForProcessInProject.- ReturnLogsForProcessInPorject(toolset=self), + ReturnLogsForProcessInProject(toolset=self),
71-75: Return explicit error messages from prerequisite callable.- except Exception: + except Exception as e: logging.exception( "Invalid Atlas config. Failed to set up MongoDBAtlas toolset" ) - return False, "Invalid Atlas config" + return False, f"Invalid Atlas config: {e}"
124-127: Use consistent error handling pattern.- data=f"Exception {self.name}: {str(e)}", + error=f"Exception {self.name}: {str(e)}",
168-168: Don't mutate caller'sparamsdict.- process_id=params.pop("process_id", ""), + process_id=params.get("process_id", ""),
204-204: Fix incorrect tool name reference.- data = f"last 4 hours eventTypeName and # of occurrences list: {events_counter} \n to get more information about a given eventTypeName call atlas_return_events_type_from_project" + data = f"last 4 hours eventTypeName and # of occurrences list: {events_counter} \n to get more information about a given eventTypeName call atlas_return_events_type_from_project with eventType parameter"
228-228: Fix typo in class name.-class ReturnLogsForProcessInPorject(MongoDBAtlasBaseTool): +class ReturnLogsForProcessInProject(MongoDBAtlasBaseTool):
🧹 Nitpick comments (3)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py (3)
88-103: Simplify conditional logic by removing unnecessary else block.The
elseblock is unnecessary after thereturnstatement.def return_result( self, response: requests.Response, params: Any, field: str = "results" ) -> StructuredToolResult: response.raise_for_status() if response.ok: res = response.json() return StructuredToolResult( status=ToolResultStatus.SUCCESS if res.get(field, []) else ToolResultStatus.NO_DATA, data=res, params=params, ) - else: - return StructuredToolResult( - status=ToolResultStatus.ERROR, - error=f"Failed {self.name}.\n{response.text}", - return_code=response.status_code, - params=params, - ) + return StructuredToolResult( + status=ToolResultStatus.ERROR, + error=f"Failed {self.name}.\n{response.text}", + return_code=response.status_code, + params=params, + )
199-217: Simplify conditional logic by removing unnecessary else block.response.raise_for_status() if response.ok: res = response.json() events_counter = Counter( [event.get("eventTypeName") for event in res.get("results", [])] ) data = f"last 4 hours eventTypeName and # of occurrences list: {events_counter} \n to get more information about a given eventTypeName call atlas_return_events_type_from_project with eventType parameter" status = ( ToolResultStatus.SUCCESS if events_counter else ToolResultStatus.NO_DATA ) return StructuredToolResult(status=status, data=data, params=params) - else: - return StructuredToolResult( - status=ToolResultStatus.ERROR, - error=f"Failed {self.name}. \n{response.text}", - return_code=response.status_code, - params=params, - ) + return StructuredToolResult( + status=ToolResultStatus.ERROR, + error=f"Failed {self.name}. \n{response.text}", + return_code=response.status_code, + params=params, + )
253-265: Simplify conditional logic by removing unnecessary else block.response.raise_for_status() if response.ok: with gzip.GzipFile(fileobj=io.BytesIO(response.content)) as gz: text_data = gz.read().decode("utf-8") return StructuredToolResult( status=ToolResultStatus.SUCCESS, data=text_data, params=params ) - else: - return StructuredToolResult( - status=ToolResultStatus.ERROR, - error=f"Failed {self.name}. \n{response.text}", - return_code=response.status_code, - params=params, - ) + return StructuredToolResult( + status=ToolResultStatus.ERROR, + error=f"Failed {self.name}. \n{response.text}", + return_code=response.status_code, + params=params, + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py(1 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
holmes/plugins/toolsets/atlas_mongodb/mongodb_atlas.py
[refactor] 22-22: Too few public methods (0/2)
(R0903)
[refactor] 88-103: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 199-217: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 253-265: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
- GitHub Check: build (3.11)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
- GitHub Check: llm_evals
create new mongodb atlas tool for holmes.
supports