-
Notifications
You must be signed in to change notification settings - Fork 74
Feature: Monitoring MCP Enhancements #61
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
base: main
Are you sure you want to change the base?
Feature: Monitoring MCP Enhancements #61
Conversation
a709d2d to
8f47eeb
Compare
8f47eeb to
df40bf4
Compare
|
I dont mind the change in structure (i.e. splitting the server up into metrics vs alarms), but im wondering if it would just be simpler for us to keep the normal structure of a server intact and separate them in their files by regions? It's my preferred way of splitting things up like that because in any code editor you could just collapse all of the alarms section if you only wanted to look at metrics. I typically always collapse every single piece of code that im not actively working on E.g. server.py OR we dont even have to worry about the difference between alarms and metrics. I think of our compute server where we have instances, images, and vnic attachments all together |
| statistic: StatisticType = Field( | ||
| "mean", description="The statistic to use for the metric" | ||
| ), | ||
| resolution: str = Field( |
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.
Would it make sense for us to pass in a Literal enum here instead?
| "The default resolution is 1m (one minute).", | ||
| examples=["1m", "5m", "1h", "1d"], | ||
| ), | ||
| resource_group: str | None = Field( |
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.
Does this parameter get used anywhere?
| "ocid1.instance.oc1.phx.anyhqljt6vnpo4icbvfcrphq2vkdmivmlbhupdw46nth7scky6rbigwjc7ea" | ||
| ], | ||
| ), | ||
| metric: str = Field( |
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.
This goes for every parameter for this function, but if you pass in a default value (in the case here it's "InstanceMetadataRequests"), it's best to wrap the parameter type in Optional. So in this case it would be
metric: Optional[str] = Field(...)
| "oci_compute", | ||
| description="The source service or application to use when searching for metric data points" | ||
| "to aggregate.", | ||
| examples=["oci_compute"], |
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.
How are any namespaces besides this supposed to be determined?
# Conflicts: # src/oci-monitoring-mcp-server/oracle/oci_monitoring_mcp_server/tests/test_monitoring_tools.py
df40bf4 to
87a11c5
Compare
Description
This PR enhances the monitoring MCP server. This separates the "Alarms" and the "Metrics" into their own folders. The following tools are now available within the Monitoring MCP Server:
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
You can ask specific or generic questions related to metrics or alarms
ocid1.instance.oc1.phx....can you list the VolumeReadOps for the last week?Checklist: