-
Notifications
You must be signed in to change notification settings - Fork 3
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
Feature: Implement defender metrics #70
base: feature/async
Are you sure you want to change the base?
Conversation
KennethKnudsen97
commented
Dec 23, 2024
•
edited
Loading
edited
- Implement proper error handling
- Consider renaming Metric and Metrics.
The new structure is this: #[derive(Debug, Serialize)]
pub struct Metric<C: Serialize> {
#[serde(rename = "hed")]
pub header: Header,
#[serde(rename = "cmet")]
pub custom_metrics: C,
} Now the user have to make sure that the custom metric they make will be serialized to follow aws json structure. The user could do this by wrapping the metric in linearmap and vector: #[derive(Debug)]
struct WifiMetric {
signal_strength: u8,
}
impl Serialize for WifiMetric {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
let mut outer = serializer.serialize_struct("WifiMetricWrapper", 1)?;
// Define the type we want to wrap our signal_strength field in
#[derive(Serialize)]
struct Number {
number: u8,
}
let number = Number {
number: self.signal_strength,
};
// Serialize number and wrap in array
outer.serialize_field("MyMetricOfType_Number", &[number])?;
outer.end()
}
} Here we have WifiMetric with a field called signal_strength, if we just derived serialization and use {
"hed": {
"rid": 123123123123,
"v": "1.0"
},
"cmet": {
"number": 23
}
} This is missing the name of the metric and should be wrapped in an array like this: {
"hed": {
"rid": 123123123123,
"v": "1.0"
},
"cmet": {
"MyMetricOfType_Number": [
{
"number": 23
}
]
}
} But by using the custom serialize implementation we can wrap the simple Wifimetric with the name and an array to follow the structure. Implementing serde serialize is not the easiest so maybe we can do some magic with rust macros just like ShadowState. |
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.
Awesome starting point!
Really nice to see how fast a feature like this can be implemented!
Dropped a few comments and observations, let's go over them together if there is anything you disagree with or something I missed.
Also, I think the general Metric
struct can be improved a bit, but I think you might be right that it requires a custom derive macro.