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

Feature: Implement defender metrics #70

Draft
wants to merge 8 commits into
base: feature/async
Choose a base branch
from

Conversation

KennethKnudsen97
Copy link
Contributor

@KennethKnudsen97 KennethKnudsen97 commented Dec 23, 2024

  • Implement proper error handling
  • Consider renaming Metric and Metrics.

@KennethKnudsen97
Copy link
Contributor Author

KennethKnudsen97 commented Dec 30, 2024

The initial structure for device custom metric takes up to much space as there is a lot of strings and vectors.
image

And the embassy pool size is 30K:
image

The structure I made was the simplest structure in order to serialize the struct into the json object AWS expects:

pub struct Metric {
    #[serde(rename = "hed")]
    pub header: Header,

    #[serde(rename = "cmet")]
    pub custom_metrics: Option<
        LinearMap<String<MAX_CUSTOM_METRICS_NAME>, Vec<CustomMetric, 1>, MAX_CUSTOM_METRICS>,
    >,
}

pub enum CustomMetric {
    Number(i64),
    NumberList(Vec<u64, MAX_METRICS>),
    StringList(Vec<String<LOCAL_INTERFACE_SIZE>, MAX_METRICS>),
    IpList(Vec<String<REMOTE_ADDR_SIZE>, MAX_METRICS>),
}

This is the structure from AWS documentation:

{
  "header": {
    "report_id": 1530304554,
    "version": "1.0"
  },
  "metrics": {
    "listening_tcp_ports": {
      "ports": [
        {
          "interface": "eth0",
          "port": 24800
        },
        {
          "interface": "eth0",
          "port": 22
        },
        {
          "interface": "eth0",
          "port": 53
        }
      ],
      "total": 3
    },
    "listening_udp_ports": {
      "ports": [
        {
          "interface": "eth0",
          "port": 5353
        },
        {
          "interface": "eth0",
          "port": 67
        }
      ],
      "total": 2
    },
    "network_stats": {
      "bytes_in": 29358693495,
      "bytes_out": 26485035,
      "packets_in": 10013573555,
      "packets_out": 11382615
    },
    "tcp_connections": {
      "established_connections": {
        "connections": [
          {
            "local_interface": "eth0",
            "local_port": 80,
            "remote_addr": "192.168.0.1:8000"
          },
          {
            "local_interface": "eth0",
            "local_port": 80,
            "remote_addr": "192.168.0.1:8000"
          }
        ],
        "total": 2
      }
    }
  },
  "custom_metrics": {
    "MyMetricOfType_Number": [
      {
        "number": 1
      }
    ],
    "MyMetricOfType_NumberList": [
      {
        "number_list": [
          1,
          2,
          3
        ]
      }
    ],
    "MyMetricOfType_StringList": [
      {
        "string_list": [
          "value_1",
          "value_2"
        ]
      }
    ],
    "MyMetricOfType_IpList": [
      {
        "ip_list": [
          "172.0.0.0",
          "172.0.0.10"
        ]
      }
    ]
  }
}

I will change the structure of Metric to take a generic custom metric so that the user of the library can decide how custom metric should look like. Then its up to the user to satisfy the json structure required for custom metrics.

@KennethKnudsen97
Copy link
Contributor Author

KennethKnudsen97 commented Jan 2, 2025

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:
LinearMap<String<24>, heapless::Vec<WifiMetric, 1>, 16>. Or the user could also manually impl serde serialize in order to follow the structure and keep the CustomMetric very simple:

 #[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
#[serde(rename="number")] the json structure would look like this:

{
    "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.

Copy link
Member

@MathiasKoch MathiasKoch left a 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.

src/defender_metrics/aws_types.rs Outdated Show resolved Hide resolved
src/defender_metrics/data_types.rs Outdated Show resolved Hide resolved
src/defender_metrics/data_types.rs Outdated Show resolved Hide resolved
src/defender_metrics/mod.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants