Updates on metrics

Author: rakshil14-2

awscrt/aws_iot_metrics.py

@@ -63,32 +63,36 @@ class _MetricsFeatureId(str, Enum):
 # Feature Value Constants
 
 
-class _MetricsProtocolVersionValue(str, Enum):
-    """Protocol version values for metrics encoding.
+def _protocol_version_metrics_value(protocol):
+    """Map protocol version to its single-character metrics value.
 
-    Maps MQTT protocol versions to their single-character metric representations.
+    Mapping: MQTT311->3, MQTT5->5.
     """
-    MQTT311 = "3"
-    MQTT5 = "5"
+    mapping = {
+        "MQTT311": "3",
+        "MQTT5": "5",
+    }
+    return mapping.get(protocol)
 
 
-class _MetricsSocketImplementationValue(str, Enum):
-    """Socket implementation values for metrics encoding.
+def _socket_implementation_metrics_value():
+    """Detect the socket implementation and return its single-character metrics value.
 
-    Maps the underlying platform socket layer to its metric representation.
-    POSIX covers macOS and Linux; WINSOCK covers Windows.
+    Mapping: Windows (WINSOCK)->B, all other platforms (POSIX)->A.
     """
-    POSIX = "A"
-    WINSOCK = "B"
+    if sys.platform == "win32":
+        return "B"
+    return "A"
 
 
-class _MetricsHttpProxyTypeValue(str, Enum):
-    """HTTP proxy type values for metrics encoding.
+def _http_proxy_type_metrics_value(proxy_options):
+    """Map proxy options to the single-character metrics value for proxy type.
 
-    Indicates whether the proxy connection uses plain HTTP or HTTPS (TLS).
+    Mapping: HTTPS (has tls_connection_options)->B, HTTP->A.
     """
-    HTTP = "A"
-    HTTPS = "B"
+    if getattr(proxy_options, 'tls_connection_options', None) is not None:
+        return "B"
+    return "A"
 
 # Mappings from existing enums to metrics values
 
@@ -200,16 +204,6 @@ def _tls_cipher_preference_metrics_value(pref):
     return mapping.get(pref)
 
 
-def _detect_socket_implementation():
-    """Detect the socket implementation based on the current platform.
-
-    Returns _MetricsSocketImplementationValue.WINSOCK on Windows,
-    _MetricsSocketImplementationValue.POSIX on all other platforms
-    (macOS, Linux).
-    """
-    if sys.platform == "win32":
-        return _MetricsSocketImplementationValue.WINSOCK
-    return _MetricsSocketImplementationValue.POSIX
 
 
 # MQTT5 encoding list
@@ -277,18 +271,15 @@ def _get_encoded_feature_list(client_options):
                 features.append(f"{_MetricsFeatureId.INBOUND_TOPIC_ALIAS_BEHAVIOR.value}/{val}")
 
     # F: protocol_version - MQTT5 always uses client options
-    features.append(f"{_MetricsFeatureId.PROTOCOL_VERSION.value}/{_MetricsProtocolVersionValue.MQTT5.value}")
+    features.append(f"{_MetricsFeatureId.PROTOCOL_VERSION.value}/{_protocol_version_metrics_value('MQTT5')}")
 
     # G: socket_implementation - Detect based on platform
-    features.append(f"{_MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_detect_socket_implementation().value}")
+    features.append(f"{_MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_socket_implementation_metrics_value()}")
 
     # H: http_proxy_type - Determine based on whether proxy uses TLS
     if client_options.http_proxy_options is not None:
-        proxy_type = _MetricsHttpProxyTypeValue.HTTPS if getattr(
-            client_options.http_proxy_options,
-            'tls_connection_options',
-            None) is not None else _MetricsHttpProxyTypeValue.HTTP
-        features.append(f"{_MetricsFeatureId.HTTP_PROXY_TYPE.value}/{proxy_type.value}")
+        val = _http_proxy_type_metrics_value(client_options.http_proxy_options)
+        features.append(f"{_MetricsFeatureId.HTTP_PROXY_TYPE.value}/{val}")
 
     # I: certificate_source - Would need to be tracked from TLS context setup. This is set at a IoT SDK level
 
@@ -329,14 +320,13 @@ def _get_encoded_feature_list_mqtt3(proxy_options, tls_ctx=None):
         str: The encoded feature list string.
     """
     features = [
-        f"{_MetricsFeatureId.PROTOCOL_VERSION.value}/{_MetricsProtocolVersionValue.MQTT311.value}",
-        f"{_MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_detect_socket_implementation().value}"
+        f"{_MetricsFeatureId.PROTOCOL_VERSION.value}/{_protocol_version_metrics_value('MQTT311')}",
+        f"{_MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_socket_implementation_metrics_value()}"
     ]
     # H: http_proxy_type - Determine based on whether proxy uses TLS
     if proxy_options is not None:
-        proxy_type = _MetricsHttpProxyTypeValue.HTTPS if getattr(
-            proxy_options, 'tls_connection_options', None) is not None else _MetricsHttpProxyTypeValue.HTTP
-        features.append(f"{_MetricsFeatureId.HTTP_PROXY_TYPE.value}/{proxy_type.value}")
+        val = _http_proxy_type_metrics_value(proxy_options)
+        features.append(f"{_MetricsFeatureId.HTTP_PROXY_TYPE.value}/{val}")
 
     # J: tls_cipher_preference - security policy
     if tls_ctx is not None:

awscrt/mqtt5.py

@@ -1821,9 +1821,7 @@ def __init__(self, client_options: ClientOptions):
             socket_options = SocketOptions()
 
         # Handle metrics configuration
-        enable_metrics = True
         if client_options.disable_metrics:
-            enable_metrics = False
             self._metrics = None
         else:
             self._metrics = _create_metrics_mqtt5(client_options)
@@ -1879,7 +1877,7 @@ def __init__(self, client_options: ClientOptions):
                                                  client_options.ack_timeout_sec,
                                                  client_options.topic_aliasing_options,
                                                  websocket_is_none,
-                                                 enable_metrics,
+                                                 not client_options.disable_metrics,
                                                  self._metrics,
                                                  core)
 

source/mqtt5_client.c

@@ -841,10 +841,6 @@ bool aws_py_metrics_parse(PyObject *metrics_py, struct aws_mqtt_iot_metrics *out
     PyObject *library_name_py = PyObject_GetAttrString(metrics_py, "library_name");
     out_metrics->library_name = aws_byte_cursor_from_pyunicode(library_name_py);
     Py_XDECREF(library_name_py);
-    if (!out_metrics->library_name.ptr) {
-        PyErr_SetString(PyExc_TypeError, "metrics.library_name must be str type");
-        return false;
-    }
 
     PyObject *metadata_entries_py = PyObject_GetAttrString(metrics_py, "metadata_entries");
 
@@ -853,11 +849,6 @@ bool aws_py_metrics_parse(PyObject *metrics_py, struct aws_mqtt_iot_metrics *out
         if (count > 0) {
             struct aws_mqtt_metadata_entry *entries =
                 aws_mem_calloc(aws_py_get_allocator(), (size_t)count, sizeof(struct aws_mqtt_metadata_entry));
-            if (!entries) {
-                Py_XDECREF(metadata_entries_py);
-                PyErr_SetAwsLastError();
-                return false;
-            }
 
             for (Py_ssize_t i = 0; i < count; ++i) {
                 PyObject *entry_py = PyList_GetItem(metadata_entries_py, i);
@@ -869,13 +860,6 @@ bool aws_py_metrics_parse(PyObject *metrics_py, struct aws_mqtt_iot_metrics *out
 
                 Py_XDECREF(key_py);
                 Py_XDECREF(value_py);
-
-                if (!entries[i].key.ptr || !entries[i].value.ptr) {
-                    Py_XDECREF(metadata_entries_py);
-                    aws_mem_release(aws_py_get_allocator(), entries);
-                    PyErr_SetString(PyExc_TypeError, "metadata_entries items must have str key and value");
-                    return false;
-                }
             }
             out_metrics->metadata_count = (size_t)count;
             out_metrics->metadata_entries = entries;

source/mqtt5_client.h

@@ -10,11 +10,17 @@
 #include <aws/mqtt/mqtt.h>
 
 /**
- * Parse a Python metrics object (with library_name and metadata_entries attrs)
- * into an aws_mqtt_iot_metrics struct. On success, returns true and populates
- * out_metrics. The caller must call aws_py_metrics_clean_up() when done.
+ * Parse a Python AWSIoTMetrics object into a C aws_mqtt_iot_metrics struct.
  *
- * On failure, returns false and a Python error has been set.
+ * WARNING: This function calls AWS_ZERO_STRUCT on out_metrics, which
+ * unconditionally zeroes all fields. The caller must pass a pointer to an
+ * uninitialized (or already cleaned-up) struct. If out_metrics currently owns
+ * heap-allocated memory (e.g. a previous metadata_entries array), that memory
+ * will be leaked because the pointer is overwritten without being freed.
+ *
+ * On success the caller is responsible for calling aws_py_metrics_clean_up()
+ * to release any memory allocated here (metadata_entries).
+ * 
  */
 bool aws_py_metrics_parse(PyObject *metrics_py, struct aws_mqtt_iot_metrics *out_metrics);
 

test/test_aws_iot_metrics.py

@@ -8,9 +8,9 @@
     AWSIoTMetrics,
     IoTMetricsMetadata,
     _MetricsFeatureId,
-    _MetricsProtocolVersionValue,
-    _MetricsSocketImplementationValue,
-    _MetricsHttpProxyTypeValue,
+    _protocol_version_metrics_value,
+    _socket_implementation_metrics_value,
+    _http_proxy_type_metrics_value,
     IOT_SDK_METRICS_FEATURE_VERSION,
     _get_encoded_feature_list,
     _get_encoded_feature_list_mqtt3,
@@ -31,9 +31,7 @@
 
 
 def _expected_socket_value():
-    if sys.platform == "win32":
-        return _MetricsSocketImplementationValue.WINSOCK
-    return _MetricsSocketImplementationValue.POSIX
+    return _socket_implementation_metrics_value()
 
 
 class TestMinimalOptionsEncoding(NativeResourceTest):
@@ -45,17 +43,17 @@ def test_mqtt5_minimal(self):
 
         result = _get_encoded_feature_list(options)
 
-        self.assertIn(f"{_MetricsFeatureId.PROTOCOL_VERSION.value}/{_MetricsProtocolVersionValue.MQTT5.value}", result)
-        self.assertIn(f"{_MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_expected_socket_value().value}", result)
+        self.assertIn(f"{_MetricsFeatureId.PROTOCOL_VERSION.value}/{_protocol_version_metrics_value('MQTT5')}", result)
+        self.assertIn(f"{_MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_expected_socket_value()}", result)
         parts = result.split(",")
         self.assertEqual(2, len(parts))
 
     def test_mqtt3_minimal(self):
         """MQTT3 with no proxy and no TLS should only have protocol version and socket."""
         result = _get_encoded_feature_list_mqtt3(proxy_options=None, tls_ctx=None)
 
-        self.assertIn(f"{_MetricsFeatureId.PROTOCOL_VERSION.value}/{_MetricsProtocolVersionValue.MQTT311.value}", result)
-        self.assertIn(f"{_MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_expected_socket_value().value}", result)
+        self.assertIn(f"{_MetricsFeatureId.PROTOCOL_VERSION.value}/{_protocol_version_metrics_value('MQTT311')}", result)
+        self.assertIn(f"{_MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_expected_socket_value()}", result)
         parts = result.split(",")
         self.assertEqual(2, len(parts))