Adding external plugin for ACLP-MR service#823
Adding external plugin for ACLP-MR service#823vivkumar-akamai wants to merge 4 commits intolinode:devfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds an external CLI plugin for the ACLP Metrics Read (ACLP-MR) service, enabling users to query monitoring metrics through the Linode CLI.
Key Changes:
- New plugin
monitor-metrics-get.pythat makes HTTP POST requests to ACLP Metric Read Service endpoints - Supports custom headers, request payload, timeout configuration, and CA certificate validation
- Implements argument parsing with mandatory fields validation for metrics queries
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "--cacert", "-c", | ||
| type=str, | ||
| help="Add ca certificate to validate server", | ||
| default=False |
There was a problem hiding this comment.
The default value for --cacert should be None or True, not False. Setting it to False will cause requests.post() to skip SSL certificate verification when no certificate is provided, which is a security risk. The verify parameter expects either a boolean or a path string.
| default=False | |
| default=None |
There was a problem hiding this comment.
Its server validation certs. I don't think it should be concern
There was a problem hiding this comment.
@vivkumar-akamai Could you elaborate a bit on this? I'd assume we'd always want to validate the remote certificate unless the user explicitly specifies otherwise but I might just be missing something
|
@vivkumar-akamai Be aware that this is the public GitHub site, |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def data_parser(args): | ||
| return args.data |
There was a problem hiding this comment.
[nitpick] The data_parser function is unnecessary as it only returns args.data without any transformation. Consider removing this function and accessing args.data directly on line 77.
yec-akamai
left a comment
There was a problem hiding this comment.
Sample Command
linode-cli monitor-metrics-get --url 'https://mr-devcloud.cloud-observability-dev.akadns.net/v2beta/monitor/services/firewall/metrics'
--header 'Authorization: '
--header 'Authentication-type: jwe'
--header 'Pragma: akamai-x-get-extracted-values'
--header 'Userid: 10001'
--header 'Content-Type: application/json'
--data '{"entity_ids":[1001], "relative_time_duration":{"unit":"min","value":60}, "metrics":
The current implementation looks not ideal from a CLI user experience. It's more like a curl command.
I wonder if you can build a plugin as the existing plugins in our codebase, i.e. linode-cli metadata. You can look for docs and examples under /plugins: https://github.com/linode/linode-cli/tree/dev/linodecli/plugins
📝 Description
This is linode CLI external plugin for ACLP-MR services. Here is how this requests going to looks like
Sample Command
linode-cli monitor-metrics-get --url 'https://mr-devcloud.cloud-observability-dev.akadns.net/v2beta/monitor/services/firewall/metrics'
--header 'Authorization: <redacted>'
--header 'Authentication-type: jwe'
--header 'Pragma: akamai-x-get-extracted-values'
--header 'Userid: 10001'
--header 'Content-Type: application/json'
--data '{"entity_ids":[1001], "relative_time_duration":{"unit":"min","value":60}, "metrics":[{"aggregate_function":"sum","name":"fw_ingress_bytes_accepted"}],"filters": [],
"associated_entity_region": "us-east", "time_granularity": {"value": 1, "unit": "days"}}'
['Authorization: Bearer <redacted>', 'Authentication-type: jwe', 'Pragma: akamai-x-get-extracted-values', 'Userid: 10001', 'Content-Type: application/json']
{'entity_ids': [1001], 'relative_time_duration': {'unit': 'min', 'value': 60}, 'metrics': [{'aggregate_function': 'sum', 'name': 'fw_ingress_bytes_accepted'}], 'filters': [], 'associated_entity_region': 'us-east', 'time_granularity': {'value': 1, 'unit': 'days'}}
Response
{
"data": {
"result": [],
"resultType": "matrix"
},
"isPartial": false,
"stats": {
"executionTimeMsec": 1,
"seriesFetched": "0"
},
"status": "success"
}
✔️ How to Test
What are the steps to reproduce the issue or verify the changes?
How do I run the relevant unit/integration tests?
📷 Preview
If applicable, include a screenshot or code snippet of this change. Otherwise, please remove this section.