From 2fe26badd4fd27e03bc94a679a8359b5306c5e56 Mon Sep 17 00:00:00 2001 From: Rakshil Modi Date: Tue, 12 May 2026 13:36:35 -0700 Subject: [PATCH 1/8] adding metrics metadata Update submodule added function to track features lint making public module fix CI errors Added TLS feature at CRT layer lint making aws_iot_metric public updating doc fix docs error Introduce Opt-out for Metrics updating doc_string updating submodules Test cases changing public attributes to private changed to public attributes --- awscrt/__init__.py | 2 + awscrt/_aws_iot_metrics.py | 16 - awscrt/aws_iot_metrics.py | 483 ++++++++++++++++++++++++++ awscrt/io.py | 6 +- awscrt/mqtt.py | 13 +- awscrt/mqtt5.py | 28 +- docsrc/source/api/aws_iot_metrics.rst | 6 + docsrc/source/index.rst | 1 + source/mqtt5_client.c | 64 +++- source/mqtt_client_connection.c | 55 ++- test/test_aws_iot_metrics.py | 425 ++++++++++++++++++++++ test/test_mqtt.py | 8 +- test/test_mqtt5.py | 10 +- 13 files changed, 1059 insertions(+), 58 deletions(-) delete mode 100644 awscrt/_aws_iot_metrics.py create mode 100644 awscrt/aws_iot_metrics.py create mode 100644 docsrc/source/api/aws_iot_metrics.rst create mode 100644 test/test_aws_iot_metrics.py diff --git a/awscrt/__init__.py b/awscrt/__init__.py index dbe9eb2c8..68e23bf54 100644 --- a/awscrt/__init__.py +++ b/awscrt/__init__.py @@ -3,6 +3,7 @@ from weakref import WeakSet + __all__ = [ 'aio', 'auth', @@ -15,6 +16,7 @@ 'mqtt_request_response', 's3', 'websocket', + 'aws_iot_metrics', ] __version__ = '1.0.0.dev0' diff --git a/awscrt/_aws_iot_metrics.py b/awscrt/_aws_iot_metrics.py deleted file mode 100644 index d2925f8d4..000000000 --- a/awscrt/_aws_iot_metrics.py +++ /dev/null @@ -1,16 +0,0 @@ -# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -# SPDX-License-Identifier: Apache-2.0. - -from dataclasses import dataclass - - -@dataclass -class AWSIoTMetrics: - """ - Configuration for IoT SDK metrics that are embedded in MQTT Connect Packet username field. - - Args: - library_name (str): The SDK library name (e.g., "IoTDeviceSDK/Python") - - """ - library_name: str = "IoTDeviceSDK/Python" diff --git a/awscrt/aws_iot_metrics.py b/awscrt/aws_iot_metrics.py new file mode 100644 index 000000000..261075b0c --- /dev/null +++ b/awscrt/aws_iot_metrics.py @@ -0,0 +1,483 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0. + +from dataclasses import dataclass +from typing import List, Optional +from enum import Enum +import sys + + +@dataclass +class IoTMetricsMetadata: + """A key-value pair for IoT SDK metrics metadata. + + Metadata entries are appended to the MQTT CONNECT packet username field + as part of the Metadata query parameter. + + Args: + key (str): The metadata key (e.g., "IoTSDKVersion", "IoTSDKFeature", "CRTVersion") + value (str): The metadata value + """ + key: str + value: str + + +@dataclass +class AWSIoTMetrics: + """ + Configuration for IoT SDK metrics that are embedded in MQTT Connect Packet username field. + + Args: + library_name (str): The SDK library name (e.g., "IoTDeviceSDK/Python") + metadata_entries (Optional[List[IoTMetricsMetadata]]): Optional list for storing key-value pairs of metadata + + """ + library_name: str = "IoTDeviceSDK/Python" + metadata_entries: Optional[List[IoTMetricsMetadata]] = None + + +# Metrics Version Constant +IOT_SDK_METRICS_FEATURE_VERSION = 1 + +# Feature ID Constants + + +class MetricsFeatureId(str, Enum): + """Feature IDs for IoT SDK metrics tracking. + + Each ID is a single character used to encode feature usage in the metrics + string with the format "ID/Value". IDs are assigned sequentially and never + reused to ensure historical data consistency across SDK versions. + """ + RETRY_JITTER_MODE = "A" + SESSION_BEHAVIOR = "B" + OFFLINE_QUEUE_BEHAVIOR = "C" + OUTBOUND_TOPIC_ALIAS_BEHAVIOR = "D" + INBOUND_TOPIC_ALIAS_BEHAVIOR = "E" + PROTOCOL_VERSION = "F" + SOCKET_IMPLEMENTATION = "G" + HTTP_PROXY_TYPE = "H" + CERTIFICATE_SOURCE = "I" + TLS_CIPHER_PREFERENCE = "J" + MINIMUM_TLS_VERSION = "K" + +# Feature Value Constants + + +class MetricsProtocolVersionValue(str, Enum): + """Protocol version values for metrics encoding. + + Maps MQTT protocol versions to their single-character metric representations. + """ + MQTT311 = "3" + MQTT5 = "5" + + +class MetricsSocketImplementationValue(str, Enum): + """Socket implementation values for metrics encoding. + + Maps the underlying platform socket layer to its metric representation. + POSIX covers macOS and Linux; WINSOCK covers Windows. + """ + POSIX = "A" + WINSOCK = "B" + + +class MetricsHttpProxyTypeValue(str, Enum): + """HTTP proxy type values for metrics encoding. + + Indicates whether the proxy connection uses plain HTTP or HTTPS (TLS). + """ + HTTP = "A" + HTTPS = "B" + +# Mappings from existing enums to metrics values + + +def _retry_jitter_metrics_value(mode): + """Map ExponentialBackoffJitterMode to its single-character metrics value. + + Mapping: NONE->A, FULL->B, DECORRELATED->C. + Returns None for DEFAULT. + """ + from awscrt.mqtt5 import ExponentialBackoffJitterMode + mapping = { + ExponentialBackoffJitterMode.NONE: "A", + ExponentialBackoffJitterMode.FULL: "B", + ExponentialBackoffJitterMode.DECORRELATED: "C", + } + return mapping.get(mode) + + +def _client_session_behavior_metrics_value(behavior): + """Map ClientSessionBehaviorType to its single-character metrics value. + + Mapping: CLEAN->A, REJOIN_POST_SUCCESS->B, REJOIN_ALWAYS->C. + Returns None for DEFAULT. + """ + from awscrt.mqtt5 import ClientSessionBehaviorType + mapping = { + ClientSessionBehaviorType.CLEAN: "A", + ClientSessionBehaviorType.REJOIN_POST_SUCCESS: "B", + ClientSessionBehaviorType.REJOIN_ALWAYS: "C", + } + return mapping.get(behavior) + + +def _client_operation_queue_behavior_metrics_value(behavior): + """Map ClientOperationQueueBehaviorType to its single-character metrics value. + + Mapping: FAIL_NON_QOS1_PUBLISH_ON_DISCONNECT->A, + FAIL_QOS0_PUBLISH_ON_DISCONNECT->B, FAIL_ALL_ON_DISCONNECT->C. + Returns None for DEFAULT. + """ + from awscrt.mqtt5 import ClientOperationQueueBehaviorType + mapping = { + ClientOperationQueueBehaviorType.FAIL_NON_QOS1_PUBLISH_ON_DISCONNECT: "A", + ClientOperationQueueBehaviorType.FAIL_QOS0_PUBLISH_ON_DISCONNECT: "B", + ClientOperationQueueBehaviorType.FAIL_ALL_ON_DISCONNECT: "C", + } + return mapping.get(behavior) + + +def _outbound_topic_alias_behavior_metrics_value(behavior): + """Map OutboundTopicAliasBehaviorType to its single-character metrics value. + + Mapping: MANUAL->A, LRU->B, DISABLED->C. + Returns None for DEFAULT. + """ + from awscrt.mqtt5 import OutboundTopicAliasBehaviorType + mapping = { + OutboundTopicAliasBehaviorType.MANUAL: "A", + OutboundTopicAliasBehaviorType.LRU: "B", + OutboundTopicAliasBehaviorType.DISABLED: "C", + } + return mapping.get(behavior) + + +def _inbound_topic_alias_behavior_metrics_value(behavior): + """Map InboundTopicAliasBehaviorType to its single-character metrics value. + + Mapping: ENABLED->A, DISABLED->B. + Returns None for DEFAULT. + """ + from awscrt.mqtt5 import InboundTopicAliasBehaviorType + mapping = { + InboundTopicAliasBehaviorType.ENABLED: "A", + InboundTopicAliasBehaviorType.DISABLED: "B", + } + return mapping.get(behavior) + + +def _minimum_tls_version_metrics_value(version): + """Map TlsVersion to its single-character metrics value. + + Mapping: SSLv3->A, TLSv1->B, TLSv1_1->C, TLSv1_2->D, TLSv1_3->E. + Returns None for DEFAULT. + """ + from awscrt.io import TlsVersion + mapping = { + TlsVersion.SSLv3: "A", + TlsVersion.TLSv1: "B", + TlsVersion.TLSv1_1: "C", + TlsVersion.TLSv1_2: "D", + TlsVersion.TLSv1_3: "E", + } + return mapping.get(version) + + +def _tls_cipher_preference_metrics_value(pref): + """Map TlsCipherPref to its single-character metrics value. + + Mapping: PQ_TLSv1_0_2021_05->A, PQ_DEFAULT->B, TLSv1_2_2025_07->C. + Returns None for DEFAULT. + """ + from awscrt.io import TlsCipherPref + mapping = { + TlsCipherPref.PQ_TLSv1_0_2021_05: "A", + TlsCipherPref.PQ_DEFAULT: "B", + TlsCipherPref.TLSv1_2_2025_07: "C", + } + 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 +def _get_encoded_feature_list(client_options): + """Generates the encoded feature list string for metrics from MQTT5 ClientOptions. + + Format: "ID/Value,ID/Value,..." + Example: "A/B,C/A,F/5,G/A" means retry_jitter_mode=FULL, offline_queue_behavior= + FAIL_NON_QOS1_PUBLISH_ON_DISCONNECT, protocol=MQTT5, socket=POSIX. + + MQTT5 connections always include: + - F (protocol_version): set to MQTT5 + - G (socket_implementation): detected from platform (POSIX or WINSOCK) + + Conditionally includes (only when the option is explicitly set and not DEFAULT): + - A (retry_jitter_mode): from client_options.retry_jitter_mode + - B (session_behavior): from client_options.session_behavior + - C (offline_queue_behavior): from client_options.offline_queue_behavior + - D (outbound_topic_alias_behavior): from topic_aliasing_options.outbound_behavior + - E (inbound_topic_alias_behavior): from topic_aliasing_options.inbound_behavior + - H (http_proxy_type): HTTP or HTTPS based on proxy TLS settings + - J (tls_cipher_preference): mapped from TlsCipherPref on the TLS context + - K (minimum_tls_version): mapped from TlsVersion on the TLS context + + Feature I (certificate_source) is set at the IoT SDK level, not here. + + Args: + client_options: MQTT5 ClientOptions dataclass. + Returns: + str: The encoded feature list string. + """ + + features = [] + + # A: retry_jitter_mode + if client_options.retry_jitter_mode is not None: + val = _retry_jitter_metrics_value(client_options.retry_jitter_mode) + if val: + features.append(f"{MetricsFeatureId.RETRY_JITTER_MODE.value}/{val}") + + # B: session_behavior + if client_options.session_behavior is not None: + val = _client_session_behavior_metrics_value(client_options.session_behavior) + if val: + features.append(f"{MetricsFeatureId.SESSION_BEHAVIOR.value}/{val}") + + # C: offline_queue_behavior + if client_options.offline_queue_behavior is not None: + val = _client_operation_queue_behavior_metrics_value(client_options.offline_queue_behavior) + if val: + features.append(f"{MetricsFeatureId.OFFLINE_QUEUE_BEHAVIOR.value}/{val}") + + # D: outbound_topic_alias_behavior + if client_options.topic_aliasing_options is not None: + if client_options.topic_aliasing_options.outbound_behavior is not None: + val = _outbound_topic_alias_behavior_metrics_value(client_options.topic_aliasing_options.outbound_behavior) + if val: + features.append(f"{MetricsFeatureId.OUTBOUND_TOPIC_ALIAS_BEHAVIOR.value}/{val}") + + # E: inbound_topic_alias_behavior + if client_options.topic_aliasing_options is not None: + if client_options.topic_aliasing_options.inbound_behavior is not None: + val = _inbound_topic_alias_behavior_metrics_value(client_options.topic_aliasing_options.inbound_behavior) + if val: + 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}") + + # G: socket_implementation - Detect based on platform + features.append(f"{MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_detect_socket_implementation().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}") + + # I: certificate_source - Would need to be tracked from TLS context setup. This is set at a IoT SDK level + + # J: tls_cipher_preference - security policy + if client_options.tls_ctx is not None: + val = _tls_cipher_preference_metrics_value(client_options.tls_ctx.cipher_pref) + if val: + features.append(f"{MetricsFeatureId.TLS_CIPHER_PREFERENCE.value}/{val}") + + # K: minimum_tls_version - The minimum TLS version set on TLSContextOptions + if client_options.tls_ctx is not None: + val = _minimum_tls_version_metrics_value(client_options.tls_ctx.min_tls_ver) + if val: + features.append(f"{MetricsFeatureId.MINIMUM_TLS_VERSION.value}/{val}") + + return ",".join(features) + +# MQTT3 encoding list + + +def _get_encoded_feature_list_mqtt3(proxy_options, tls_ctx=None): + """ + Generates the encoded feature list string for metrics from MQTT3 connection options. + Format: "ID/Value,ID/Value..." + + MQTT3 connections always include: + - F (protocol_version): set to MQTT311 + - G (socket_implementation): detected from platform (POSIX or WINSOCK) + + Conditionally includes: + - H (http_proxy_type): HTTP or HTTPS based on proxy TLS settings + - J (tls_cipher_preference): mapped from TlsCipherPref on the TLS context + - K (minimum_tls_version): mapped from TlsVersion on the TLS context + + Args: + proxy_options: Optional HttpProxyOptions from the Connection. + tls_ctx: Optional ClientTlsContext used by the connection. + Returns: + str: The encoded feature list string. + """ + features = [ + f"{MetricsFeatureId.PROTOCOL_VERSION.value}/{MetricsProtocolVersionValue.MQTT311.value}", + f"{MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_detect_socket_implementation().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}") + + # J: tls_cipher_preference - security policy + if tls_ctx is not None: + val = _tls_cipher_preference_metrics_value(tls_ctx.cipher_pref) + if val: + features.append(f"{MetricsFeatureId.TLS_CIPHER_PREFERENCE.value}/{val}") + + # K: minimum_tls_version - the minimum TLS version set on TLSContextOptions + if tls_ctx is not None: + val = _minimum_tls_version_metrics_value(tls_ctx.min_tls_ver) + if val: + features.append(f"{MetricsFeatureId.MINIMUM_TLS_VERSION.value}/{val}") + + return ",".join(features) + + +def _merge_feature_lists(crt_features, user_features): + """Merge CRT-generated features with user-provided (IoT SDK) features. + + When both lists contain the same feature ID, the user-provided value + takes precedence. The result is sorted alphabetically by feature ID. + + Args: + crt_features (str): CRT-generated feature list. + user_features (str): User-provided feature list from the IoT SDK. + May be empty string if no SDK features are provided. + Returns: + str: The merged feature list string, sorted by feature ID. + """ + merged = {} + # Parse CRT Features + for pair in crt_features.split(","): + if "/" in pair: + fid, val = pair.split("/", 1) + merged[fid] = val + + for pair in user_features.split(","): + if "/" in pair: + fid, val = pair.split("/", 1) + merged[fid] = val + return ",".join(f"{k}/{v}" for k, v in sorted(merged.items())) + +# Metrics creation + + +def create_metrics(user_metrics, crt_feature_list): + """Create the final AWSIoTMetrics object by merging CRT and user-provided data. + + Applies the following rules to produce the final metrics: + + 1. library_name: Uses the value from user_metrics if provided, + otherwise defaults to "IoTDeviceSDK/Python". + 2. CRTVersion: Automatically set to the current awscrt + package version. Cannot be overridden by user input. + 3. IoTSDKMetricsVersion: Always set to the current + IOT_SDK_METRICS_FEATURE_VERSION constant. + 4. IoTSDKFeature: If the user-provided metrics version + matches IOT_SDK_METRICS_FEATURE_VERSION, the CRT feature list is + merged with the user's IoTSDKFeature (user values take precedence + for duplicate feature IDs). Otherwise, only CRT features are used. + 5. Any additional user metadata entries (other than CRTVersion, + IoTSDKMetricsVersion, IoTSDKFeature) are passed through unchanged. + + Args: + user_metrics : Metrics configuration from + the IoT SDK. May be None if no SDK-level metrics are provided. + crt_feature_list : Encoded CRT feature list string generated + by _get_encoded_feature_list or _get_encoded_feature_list_mqtt3. + Returns: + AWSIoTMetrics: The final metrics object ready to be embedded in the + MQTT CONNECT packet username field. + """ + + from awscrt import __version__ as crt_version + + final_metrics = AWSIoTMetrics( + library_name=user_metrics.library_name if user_metrics else "IoTDeviceSDK/Python" + ) + + # CRTVERSION: not modifiable by user, automatically set + metadata = {"CRTVersion": crt_version} + + # Extract user_metadata from IoT SDK + user_metrics_version = None + user_feature = "" + if user_metrics and user_metrics.metadata_entries: + for entry in user_metrics.metadata_entries: + if entry.key == "IoTSDKMetricsVersion": + user_metrics_version = entry.value + elif entry.key == "IoTSDKFeature": + user_feature = entry.value + elif entry.key != "CRTVersion": + metadata[entry.key] = entry.value + + # Merge features: if version matches, merge CRT + SDK; otherwise CRT only + if (user_metrics_version is not None and user_metrics_version.isdigit() and int( + user_metrics_version) == IOT_SDK_METRICS_FEATURE_VERSION and user_feature): + metadata["IoTSDKFeature"] = _merge_feature_lists(crt_feature_list, user_feature) + else: + metadata["IoTSDKFeature"] = _merge_feature_lists(crt_feature_list, "") + + # Always set current metrics version + metadata["IoTSDKMetricsVersion"] = str(IOT_SDK_METRICS_FEATURE_VERSION) + + final_metrics.metadata_entries = [IoTMetricsMetadata(key=k, value=v) for k, v in metadata.items()] + return final_metrics + + +def create_metrics_mqtt5(client_options): + """Create the final AWSIoTMetrics object for an MQTT5 client. + + Generates the CRT feature list from the full set of MQTT5 ClientOptions + + Args: + client_options: MQTT5 ClientOptions dataclass containing all + connection configuration and optional user metrics. + Returns: + AWSIoTMetrics: The final metrics object with merged CRT and SDK features. + """ + crt_feature_list = _get_encoded_feature_list(client_options) + return create_metrics(client_options.metrics, crt_feature_list) + + +def create_metrics_mqtt3(user_metrics=None, proxy_options=None, tls_ctx=None): + """ + Creates the final AWSIoTMetrics object for an MQTT3 connection. + + Generates the CRT feature list from the MQTT3 connection parameters + + Args: + user_metrics : Optional metrics configuration + provided by the IoT SDK. If None, defaults are used. + proxy_options : Optional HTTP proxy options + from the Connection, used to determine proxy type feature. + tls_ctx : Optional TLS context from the + connection, used to determine cipher preference and minimum TLS + version features. + Returns: + AWSIoTMetrics: The final metrics object with merged CRT and SDK features. + """ + crt_feature_list = _get_encoded_feature_list_mqtt3(proxy_options, tls_ctx) + return create_metrics(user_metrics, crt_feature_list) diff --git a/awscrt/io.py b/awscrt/io.py index 4d36ba69f..cfb340ecc 100644 --- a/awscrt/io.py +++ b/awscrt/io.py @@ -606,12 +606,16 @@ class ClientTlsContext(NativeResource): Args: options (TlsContextOptions): Configuration options. """ - __slots__ = () + __slots__ = ('min_tls_ver', 'cipher_pref') def __init__(self, options): assert isinstance(options, TlsContextOptions) super().__init__() + + self.min_tls_ver = options.min_tls_ver + self.cipher_pref = options.cipher_pref + self._binding = _awscrt.client_tls_ctx_new( options.min_tls_ver.value, options.cipher_pref.value, diff --git a/awscrt/mqtt.py b/awscrt/mqtt.py index d3c3921d5..6ea9df7f6 100644 --- a/awscrt/mqtt.py +++ b/awscrt/mqtt.py @@ -16,7 +16,7 @@ from awscrt.io import ClientBootstrap, ClientTlsContext, SocketOptions from dataclasses import dataclass from awscrt.mqtt5 import Client as Mqtt5Client -from awscrt._aws_iot_metrics import AWSIoTMetrics +from awscrt.aws_iot_metrics import AWSIoTMetrics, IoTMetricsMetadata, create_metrics_mqtt3 class QoS(IntEnum): @@ -332,7 +332,9 @@ class Connection(NativeResource): proxy_options (Optional[awscrt.http.HttpProxyOptions]): Optional proxy options for all connections. - enable_metrics (bool): Enable IoT SDK metrics in MQTT CONNECT packet username field, including SDK name, version, and platform. Default to True. + disable_metrics (bool): Disable IoT SDK metrics in MQTT CONNECT packet username field, including SDK name, version, and platform. Default to False. + + metrics (Optional[AWSIoTMetrics]) : Optional metrics configuration for IoT SDK metrics reporting. If provided, the CRT will use the given metrics. If None, a default AWSIoTMetrics will be created. """ def __init__(self, @@ -359,7 +361,8 @@ def __init__(self, on_connection_success=None, on_connection_failure=None, on_connection_closed=None, - enable_metrics=True, + disable_metrics=False, + metrics=None, ): assert isinstance(client, Client) or isinstance(client, Mqtt5Client) @@ -412,8 +415,8 @@ def __init__(self, self.password = password self.socket_options = socket_options if socket_options else SocketOptions() self.proxy_options = proxy_options if proxy_options else websocket_proxy_options - if enable_metrics: - self._metrics = AWSIoTMetrics() + if not disable_metrics: + self._metrics = create_metrics_mqtt3(metrics, self.proxy_options, self.client.tls_ctx) else: self._metrics = None diff --git a/awscrt/mqtt5.py b/awscrt/mqtt5.py index bc20581e5..8f0da1997 100644 --- a/awscrt/mqtt5.py +++ b/awscrt/mqtt5.py @@ -5,7 +5,7 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0. -from typing import Any, Callable, Union +from typing import Any, Callable, Optional, Union import _awscrt from concurrent.futures import Future from enum import IntEnum @@ -15,7 +15,7 @@ from dataclasses import dataclass from collections.abc import Sequence from inspect import signature -from awscrt._aws_iot_metrics import AWSIoTMetrics +from awscrt.aws_iot_metrics import AWSIoTMetrics, IoTMetricsMetadata, create_metrics_mqtt5 class QoS(IntEnum): @@ -1371,7 +1371,8 @@ class ClientOptions: on_lifecycle_event_connection_success_fn (Callable[[LifecycleConnectSuccessData],]): Callback for Lifecycle Event Connection Success. on_lifecycle_event_connection_failure_fn (Callable[[LifecycleConnectFailureData],]): Callback for Lifecycle Event Connection Failure. on_lifecycle_event_disconnection_fn (Callable[[LifecycleDisconnectData],]): Callback for Lifecycle Event Disconnection. - enable_metrics (bool): Enable IoT SDK metrics in MQTT CONNECT packet username field, including SDK name, version, and platform. Default to True. + disable_metrics (bool): Disable IoT SDK metrics in MQTT CONNECT packet username field, including SDK name, version, and platform. Default to False. + metrics (Optional[AWSIoTMetrics]) : Optional metrics configuration for IoT SDK metrics reporting. If provided, the CRT will use the given metrics. If None, a default AWSIoTMetrics will be created. """ host_name: str @@ -1399,7 +1400,8 @@ class ClientOptions: on_lifecycle_event_connection_success_fn: Callable[[LifecycleConnectSuccessData], None] = None on_lifecycle_event_connection_failure_fn: Callable[[LifecycleConnectFailureData], None] = None on_lifecycle_event_disconnection_fn: Callable[[LifecycleDisconnectData], None] = None - enable_metrics: bool = True + disable_metrics: bool = False + metrics: Optional[AWSIoTMetrics] = None def _check_callback(callback): @@ -1428,7 +1430,7 @@ def __init__(self, client_options: ClientOptions): self._on_lifecycle_connection_failure_cb = _check_callback( client_options.on_lifecycle_event_connection_failure_fn) self._on_lifecycle_disconnection_cb = _check_callback(client_options.on_lifecycle_event_disconnection_fn) - self._enable_metrics = client_options.enable_metrics + self._disable_metrics = client_options.disable_metrics def _ws_handshake_transform(self, http_request_binding, http_headers_binding, native_userdata): if self._ws_handshake_transform_cb is None: @@ -1776,7 +1778,7 @@ def __init__( keep_alive_secs: int, ack_timeout_secs: int, clean_session: int, - enable_metrics: bool): + disable_metrics: bool): self.host_name = host_name self.port = port self.client_id = "" if client_id is None else client_id @@ -1787,7 +1789,7 @@ def __init__( self.keep_alive_secs: int = 1200 if keep_alive_secs is None else keep_alive_secs self.ack_timeout_secs: int = 0 if ack_timeout_secs is None else ack_timeout_secs self.clean_session: bool = True if clean_session is None else clean_session - self.enable_metrics: bool = True if enable_metrics is None else enable_metrics + self.disable_metrics: bool = False if disable_metrics is None else disable_metrics class Client(NativeResource): @@ -1819,8 +1821,8 @@ def __init__(self, client_options: ClientOptions): socket_options = SocketOptions() # Handle metrics configuration - if client_options.enable_metrics: - self._metrics = AWSIoTMetrics() + if not client_options.disable_metrics: + self._metrics = create_metrics_mqtt5(client_options) else: self._metrics = None @@ -1875,8 +1877,8 @@ def __init__(self, client_options: ClientOptions): client_options.ack_timeout_sec, client_options.topic_aliasing_options, websocket_is_none, - client_options.enable_metrics, - self._metrics.library_name if self._metrics else None, + not client_options.disable_metrics, + self._metrics, core) # Store the options for adapter @@ -1892,7 +1894,7 @@ def __init__(self, client_options: ClientOptions): ack_timeout_secs=client_options.ack_timeout_sec, clean_session=( client_options.session_behavior < ClientSessionBehaviorType.REJOIN_ALWAYS if client_options.session_behavior else True), - enable_metrics=client_options.enable_metrics) + disable_metrics=client_options.disable_metrics) def start(self): """Notifies the MQTT5 client that you want it maintain connectivity to the configured endpoint. @@ -2146,5 +2148,5 @@ def new_connection(self, on_connection_interrupted=None, on_connection_resumed=N websocket_proxy_options=None, websocket_handshake_transform=None, proxy_options=None, - enable_metrics=self.adapter_options.enable_metrics + disable_metrics=self.adapter_options.disable_metrics ) diff --git a/docsrc/source/api/aws_iot_metrics.rst b/docsrc/source/api/aws_iot_metrics.rst new file mode 100644 index 000000000..56148c54b --- /dev/null +++ b/docsrc/source/api/aws_iot_metrics.rst @@ -0,0 +1,6 @@ +awscrt.aws_iot_metrics +====================== + +.. automodule:: awscrt.aws_iot_metrics + :members: + diff --git a/docsrc/source/index.rst b/docsrc/source/index.rst index bbac435c3..5d0a510be 100644 --- a/docsrc/source/index.rst +++ b/docsrc/source/index.rst @@ -12,6 +12,7 @@ API Reference :maxdepth: 2 api/auth + api/aws_iot_metrics api/checksums api/common api/crypto diff --git a/source/mqtt5_client.c b/source/mqtt5_client.c index 6018b565c..f8faa0431 100644 --- a/source/mqtt5_client.c +++ b/source/mqtt5_client.c @@ -890,12 +890,12 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { PyObject *is_websocket_none_py; PyObject *client_core_py; /* Metrics */ - PyObject *is_metrics_enabled_py; /* optional enable metrics */ - struct aws_byte_cursor metrics_library_name; /* optional IoT SDK metrics username */ + PyObject *is_metrics_enabled_py; /* optional enable metrics */ + PyObject *metrics_py; /* optional AWSIoTMetrics object */ if (!PyArg_ParseTuple( args, - "Os#IOOOOz#Oz#z#OOOOOOOOOz*Oz#OOOz#z*z#OOOOOOOOOOOOOOz#O", + "Os#IOOOOz#Oz#z#OOOOOOOOOz*Oz#OOOz#z*z#OOOOOOOOOOOOOOOO", /* O */ &self_py, /* s */ &host_name.ptr, /* # */ &host_name.len, @@ -953,8 +953,7 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { /* Metrics */ /* O */ &is_metrics_enabled_py, - /* z */ &metrics_library_name.ptr, - /* # */ &metrics_library_name.len, + /* O */ &metrics_py, /* O */ &client_core_py)) { return NULL; @@ -975,6 +974,9 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { AWS_ZERO_STRUCT(tls_options); struct aws_mqtt5_user_property *user_properties_tmp = NULL; struct aws_mqtt5_user_property *will_user_properties_tmp = NULL; + struct aws_mqtt_metadata_entry *metadata_entries = NULL; + PyObject *library_name_py = NULL; + PyObject *metadata_entries_py = NULL; struct aws_mqtt5_client_options client_options; AWS_ZERO_STRUCT(client_options); @@ -1321,8 +1323,47 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { /* METRICS */ struct aws_mqtt_iot_metrics metrics_tmp; AWS_ZERO_STRUCT(metrics_tmp); - if (PyObject_IsTrue(is_metrics_enabled_py)) { - metrics_tmp.library_name = metrics_library_name; + + if (PyObject_IsTrue(is_metrics_enabled_py) && metrics_py != Py_None) { + library_name_py = PyObject_GetAttrString(metrics_py, "library_name"); + metrics_tmp.library_name = aws_byte_cursor_from_pyunicode(library_name_py); + if (!metrics_tmp.library_name.ptr) { + PyErr_SetString(PyExc_TypeError, "metrics.library_name must be str type"); + goto done; + } + + metadata_entries_py = PyObject_GetAttrString(metrics_py, "metadata_entries"); + + if (metadata_entries_py && metadata_entries_py != Py_None && PyList_Check(metadata_entries_py)) { + Py_ssize_t count = PyList_Size(metadata_entries_py); + if (count > 0) { + metadata_entries = + aws_mem_calloc(aws_py_get_allocator(), (size_t)count, sizeof(struct aws_mqtt_metadata_entry)); + if (!metadata_entries) { + PyErr_SetAwsLastError(); + goto done; + } + + for (Py_ssize_t i = 0; i < count; ++i) { + PyObject *entry_py = PyList_GetItem(metadata_entries_py, i); + PyObject *key_py = PyObject_GetAttrString(entry_py, "key"); + PyObject *value_py = PyObject_GetAttrString(entry_py, "value"); + + metadata_entries[i].key = aws_byte_cursor_from_pyunicode(key_py); + metadata_entries[i].value = aws_byte_cursor_from_pyunicode(value_py); + + Py_XDECREF(key_py); + Py_XDECREF(value_py); + + if (!metadata_entries[i].key.ptr || !metadata_entries[i].value.ptr) { + PyErr_SetString(PyExc_TypeError, "metadata_entries items must have str key and value"); + goto done; + } + } + metrics_tmp.metadata_count = (size_t)count; + metrics_tmp.metadata_entries = metadata_entries; + } + } client_options.metrics = &metrics_tmp; } @@ -1368,10 +1409,19 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { } PyBuffer_Release(&will_payload_stack); PyBuffer_Release(&will_correlation_data_stack); + + /* Cleanup metrics */ + Py_XDECREF(library_name_py); + Py_XDECREF(metadata_entries_py); + if (metadata_entries) { + aws_mem_release(allocator, metadata_entries); + } + if (success) { return capsule; } Py_XDECREF(capsule); + return NULL; } diff --git a/source/mqtt_client_connection.c b/source/mqtt_client_connection.c index d9dcbac6e..ff31dbf56 100644 --- a/source/mqtt_client_connection.c +++ b/source/mqtt_client_connection.c @@ -447,18 +447,55 @@ static bool s_set_metrics(struct aws_mqtt_client_connection *connection, PyObjec bool success = false; + struct aws_mqtt_iot_metrics metrics_tmp; + AWS_ZERO_STRUCT(metrics_tmp); + PyObject *metadata_entries_py = NULL; + struct aws_mqtt_metadata_entry *metadata_entries = NULL; + size_t metadata_count = 0; + PyObject *library_name_py = PyObject_GetAttrString(metrics, "library_name"); - struct aws_byte_cursor library_name = aws_byte_cursor_from_pyunicode(library_name_py); - if (!library_name.ptr) { + metrics_tmp.library_name = aws_byte_cursor_from_pyunicode(library_name_py); + if (!metrics_tmp.library_name.ptr) { PyErr_SetString(PyExc_TypeError, "metrics.library_name must be str type"); goto done; } - struct aws_mqtt_iot_metrics metrics_struct = { - .library_name = library_name, - }; + metadata_entries_py = PyObject_GetAttrString(metrics, "metadata_entries"); + + if (metadata_entries_py && metadata_entries_py != Py_None && PyList_Check(metadata_entries_py)) { + Py_ssize_t count = PyList_Size(metadata_entries_py); + if (count > 0) { + metadata_entries = + aws_mem_calloc(aws_py_get_allocator(), (size_t)count, sizeof(struct aws_mqtt_metadata_entry)); + if (!metadata_entries) { + PyErr_SetAwsLastError(); + goto done; + } + + for (Py_ssize_t i = 0; i < count; ++i) { + PyObject *entry_py = PyList_GetItem(metadata_entries_py, i); + PyObject *key_py = PyObject_GetAttrString(entry_py, "key"); + PyObject *value_py = PyObject_GetAttrString(entry_py, "value"); + + metadata_entries[i].key = aws_byte_cursor_from_pyunicode(key_py); + metadata_entries[i].value = aws_byte_cursor_from_pyunicode(value_py); + + Py_XDECREF(key_py); + Py_XDECREF(value_py); - if (aws_mqtt_client_connection_set_metrics(connection, &metrics_struct)) { + if (!metadata_entries[i].key.ptr || !metadata_entries[i].value.ptr) { + PyErr_SetString(PyExc_TypeError, "metadata_entries items must have str key and value"); + goto done; + } + } + metadata_count = (size_t)count; + } + } + + metrics_tmp.metadata_count = metadata_count; + metrics_tmp.metadata_entries = metadata_entries; + + if (aws_mqtt_client_connection_set_metrics(connection, &metrics_tmp)) { PyErr_SetAwsLastError(); goto done; } @@ -466,7 +503,11 @@ static bool s_set_metrics(struct aws_mqtt_client_connection *connection, PyObjec success = true; done: - Py_DECREF(library_name_py); + Py_XDECREF(library_name_py); + Py_XDECREF(metadata_entries_py); + if (metadata_entries) { + aws_mem_release(aws_py_get_allocator(), metadata_entries); + } return success; } diff --git a/test/test_aws_iot_metrics.py b/test/test_aws_iot_metrics.py new file mode 100644 index 000000000..98a68d8b4 --- /dev/null +++ b/test/test_aws_iot_metrics.py @@ -0,0 +1,425 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0. + +import sys +import unittest +from test import NativeResourceTest +from awscrt.aws_iot_metrics import ( + AWSIoTMetrics, + IoTMetricsMetadata, + MetricsFeatureId, + MetricsProtocolVersionValue, + MetricsSocketImplementationValue, + MetricsHttpProxyTypeValue, + IOT_SDK_METRICS_FEATURE_VERSION, + _get_encoded_feature_list, + _get_encoded_feature_list_mqtt3, + _merge_feature_lists, + create_metrics, +) +from awscrt.mqtt5 import ( + ClientOptions, + ExponentialBackoffJitterMode, + ClientSessionBehaviorType, + ClientOperationQueueBehaviorType, + OutboundTopicAliasBehaviorType, + InboundTopicAliasBehaviorType, + TopicAliasingOptions, +) +from awscrt.io import ClientTlsContext, TlsContextOptions, TlsConnectionOptions, TlsVersion, TlsCipherPref +from awscrt.http import HttpProxyOptions + + +def _expected_socket_value(): + if sys.platform == "win32": + return MetricsSocketImplementationValue.WINSOCK + return MetricsSocketImplementationValue.POSIX + + +class TestMinimalOptionsEncoding(NativeResourceTest): + """Test encoding with minimal/default options.""" + + def test_mqtt5_minimal(self): + """MQTT5 with all defaults should only have protocol version and socket.""" + options = ClientOptions(host_name="localhost", port=8883) + + 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) + 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) + parts = result.split(",") + self.assertEqual(2, len(parts)) + + def test_default_enum_values_omitted(self): + """DEFAULT enum values should not appear in the encoded list.""" + options = ClientOptions( + host_name="localhost", + port=8883, + retry_jitter_mode=ExponentialBackoffJitterMode.DEFAULT, + session_behavior=ClientSessionBehaviorType.DEFAULT, + offline_queue_behavior=ClientOperationQueueBehaviorType.DEFAULT, + ) + + result = _get_encoded_feature_list(options) + + self.assertNotIn(f"{MetricsFeatureId.RETRY_JITTER_MODE.value}/", result) + self.assertNotIn(f"{MetricsFeatureId.SESSION_BEHAVIOR.value}/", result) + self.assertNotIn(f"{MetricsFeatureId.OFFLINE_QUEUE_BEHAVIOR.value}/", result) + + +class TestOptionsWithMultipleNonDefaultFeaturesEncoding(NativeResourceTest): + """Test encoding with multiple explicitly-set features.""" + + def test_all_mqtt5_features_set(self): + """MQTT5 with all features explicitly set to non-default values.""" + proxy = HttpProxyOptions(host_name="proxy.example.com", port=8080) + + tls_options = TlsContextOptions() + tls_options.min_tls_ver = TlsVersion.TLSv1_2 + cipher_pref = TlsCipherPref.PQ_DEFAULT + if cipher_pref.is_supported(): + tls_options.cipher_pref = cipher_pref + tls_ctx = ClientTlsContext(tls_options) + + options = ClientOptions( + host_name="localhost", + port=8883, + retry_jitter_mode=ExponentialBackoffJitterMode.FULL, + session_behavior=ClientSessionBehaviorType.CLEAN, + offline_queue_behavior=ClientOperationQueueBehaviorType.FAIL_ALL_ON_DISCONNECT, + topic_aliasing_options=TopicAliasingOptions( + outbound_behavior=OutboundTopicAliasBehaviorType.LRU, + inbound_behavior=InboundTopicAliasBehaviorType.ENABLED, + ), + http_proxy_options=proxy, + tls_ctx=tls_ctx, + ) + + result = _get_encoded_feature_list(options) + + self.assertIn("A/B", result) # FULL + self.assertIn("B/A", result) # CLEAN + self.assertIn("C/C", result) # FAIL_ALL + self.assertIn("D/B", result) # LRU + self.assertIn("E/A", result) # ENABLED + self.assertIn("F/5", result) # MQTT5 + self.assertIn("H/A", result) # HTTP proxy (no TLS on proxy) + self.assertIn("K/D", result) # TLSv1_2 + if cipher_pref.is_supported(): + self.assertIn("J/B", result) # PQ_DEFAULT + + def test_alternate_values(self): + """MQTT5 with alternate non-default values.""" + tls_options = TlsContextOptions() + tls_options.min_tls_ver = TlsVersion.TLSv1_3 + cipher_pref = TlsCipherPref.PQ_TLSv1_0_2021_05 + if cipher_pref.is_supported(): + tls_options.cipher_pref = cipher_pref + tls_ctx = ClientTlsContext(tls_options) + + proxy_tls_ctx = ClientTlsContext(TlsContextOptions()) + proxy_tls_conn_options = TlsConnectionOptions(proxy_tls_ctx) + proxy = HttpProxyOptions( + host_name="proxy.example.com", port=443, + tls_connection_options=proxy_tls_conn_options) + + options = ClientOptions( + host_name="localhost", + port=8883, + retry_jitter_mode=ExponentialBackoffJitterMode.DECORRELATED, + session_behavior=ClientSessionBehaviorType.REJOIN_ALWAYS, + offline_queue_behavior=ClientOperationQueueBehaviorType.FAIL_NON_QOS1_PUBLISH_ON_DISCONNECT, + topic_aliasing_options=TopicAliasingOptions( + outbound_behavior=OutboundTopicAliasBehaviorType.MANUAL, + inbound_behavior=InboundTopicAliasBehaviorType.DISABLED, + ), + http_proxy_options=proxy, + tls_ctx=tls_ctx, + ) + + result = _get_encoded_feature_list(options) + + self.assertIn("A/C", result) # DECORRELATED + self.assertIn("B/C", result) # REJOIN_ALWAYS + self.assertIn("C/A", result) # FAIL_NON_QOS1 + self.assertIn("D/A", result) # MANUAL + self.assertIn("E/B", result) # DISABLED + self.assertIn("F/5", result) # MQTT5 + self.assertIn("H/B", result) # HTTPS proxy + self.assertIn("K/E", result) # TLSv1_3 + if cipher_pref.is_supported(): + self.assertIn("J/A", result) # PQ_TLSv1_0_2021_05 + + def test_mqtt3_with_proxy_and_tls(self): + """MQTT3 with HTTPS proxy and TLS context.""" + tls_options = TlsContextOptions() + tls_options.min_tls_ver = TlsVersion.TLSv1_2 + cipher_pref = TlsCipherPref.TLSv1_2_2025_07 + if cipher_pref.is_supported(): + tls_options.cipher_pref = cipher_pref + tls_ctx = ClientTlsContext(tls_options) + + proxy_tls_ctx = ClientTlsContext(TlsContextOptions()) + proxy_tls_conn_options = TlsConnectionOptions(proxy_tls_ctx) + proxy = HttpProxyOptions( + host_name="proxy.example.com", port=443, + tls_connection_options=proxy_tls_conn_options) + + result = _get_encoded_feature_list_mqtt3(proxy_options=proxy, tls_ctx=tls_ctx) + + self.assertIn("F/3", result) + self.assertIn("H/B", result) # HTTPS + self.assertIn("K/D", result) # TLSv1_2 + if cipher_pref.is_supported(): + self.assertIn("J/C", result) # TLSv1_2_2025_07 + + +class TestMergeFeatureLists(NativeResourceTest): + """Test feature list merging logic.""" + + def test_user_overrides_crt(self): + """User features take precedence over CRT features for same feature ID.""" + result = _merge_feature_lists("A/B,F/5", "A/C") + # User's A/C overwrites CRT's A/B + self.assertEqual("A/C,F/5", result) + + def test_user_overrides_multiple_crt_features(self): + """User can override multiple CRT features at once.""" + result = _merge_feature_lists("A/B,F/5,G/A,K/D", "A/C,F/3,K/E") + # User overrides A, F, K; G remains from CRT + self.assertEqual("A/C,F/3,G/A,K/E", result) + + def test_empty_user_features(self): + result = _merge_feature_lists("F/5,G/A", "") + self.assertEqual("F/5,G/A", result) + + def test_empty_crt_features(self): + result = _merge_feature_lists("", "A/B") + self.assertEqual("A/B", result) + + def test_sorted_output(self): + result = _merge_feature_lists("G/A,F/5", "A/B") + self.assertEqual("A/B,F/5,G/A", result) + + def test_disjoint_features(self): + result = _merge_feature_lists("F/5,G/A", "I/A,K/D") + self.assertEqual("F/5,G/A,I/A,K/D", result) + + def test_both_empty(self): + result = _merge_feature_lists("", "") + self.assertEqual("", result) + + +class TestCreateMetricsWithDefaultOptions(NativeResourceTest): + """Test create_metrics with no user metrics or default user metrics.""" + + def test_none_user_metrics(self): + result = create_metrics(None, "F/5,G/A") + + self.assertEqual("IoTDeviceSDK/Python", result.library_name) + metadata_dict = {e.key: e.value for e in result.metadata_entries} + self.assertIn("CRTVersion", metadata_dict) + self.assertEqual("F/5,G/A", metadata_dict["IoTSDKFeature"]) + self.assertEqual(str(IOT_SDK_METRICS_FEATURE_VERSION), metadata_dict["IoTSDKMetricsVersion"]) + + def test_empty_user_metrics(self): + user = AWSIoTMetrics() + result = create_metrics(user, "F/5,G/A") + + self.assertEqual("IoTDeviceSDK/Python", result.library_name) + metadata_dict = {e.key: e.value for e in result.metadata_entries} + self.assertEqual("F/5,G/A", metadata_dict["IoTSDKFeature"]) + + def test_version_always_set(self): + result = create_metrics(None, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + self.assertEqual(str(IOT_SDK_METRICS_FEATURE_VERSION), metadata_dict["IoTSDKMetricsVersion"]) + + +class TestCreateMetricsWithUserFeaturesMerged(NativeResourceTest): + """Test that user features are merged when version matches.""" + + def test_user_feature_added(self): + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="IoTSDKMetricsVersion", value="1"), + IoTMetricsMetadata(key="IoTSDKFeature", value="I/A"), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertIn("I/A", metadata_dict["IoTSDKFeature"]) + self.assertIn("F/5", metadata_dict["IoTSDKFeature"]) + self.assertIn("G/A", metadata_dict["IoTSDKFeature"]) + + def test_user_feature_overrides_crt(self): + """User feature takes precedence over CRT feature for same ID.""" + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="IoTSDKMetricsVersion", value="1"), + IoTMetricsMetadata(key="IoTSDKFeature", value="F/3,I/B"), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertIn("F/3", metadata_dict["IoTSDKFeature"]) + self.assertNotIn("F/5", metadata_dict["IoTSDKFeature"]) + self.assertIn("I/B", metadata_dict["IoTSDKFeature"]) + + def test_empty_user_feature_string(self): + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="IoTSDKMetricsVersion", value="1"), + IoTMetricsMetadata(key="IoTSDKFeature", value=""), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertEqual("F/5,G/A", metadata_dict["IoTSDKFeature"]) + + +class TestCreateMetricsWithVersionMismatch(NativeResourceTest): + """Test that user features are ignored when version doesn't match.""" + + def test_higher_version(self): + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="IoTSDKMetricsVersion", value="99"), + IoTMetricsMetadata(key="IoTSDKFeature", value="I/A"), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertNotIn("I/A", metadata_dict["IoTSDKFeature"]) + self.assertIn("F/5", metadata_dict["IoTSDKFeature"]) + + def test_lower_version(self): + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="IoTSDKMetricsVersion", value="0"), + IoTMetricsMetadata(key="IoTSDKFeature", value="I/A"), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertNotIn("I/A", metadata_dict["IoTSDKFeature"]) + + def test_non_numeric_version(self): + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="IoTSDKMetricsVersion", value="abc"), + IoTMetricsMetadata(key="IoTSDKFeature", value="I/A"), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertNotIn("I/A", metadata_dict["IoTSDKFeature"]) + + def test_no_version_set(self): + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="IoTSDKFeature", value="I/A"), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertNotIn("I/A", metadata_dict["IoTSDKFeature"]) + + +class TestCreateMetricsCRTVersionNotModifiable(NativeResourceTest): + """Test that CRTVersion cannot be overridden by user.""" + + def test_user_cannot_override_crt_version(self): + from awscrt import __version__ as actual_version + + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="CRTVersion", value="fake_version"), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertEqual(actual_version, metadata_dict["CRTVersion"]) + self.assertNotEqual("fake_version", metadata_dict["CRTVersion"]) + + def test_empty_crt_version_overridden(self): + from awscrt import __version__ as actual_version + + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="CRTVersion", value=""), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertEqual(actual_version, metadata_dict["CRTVersion"]) + + +class TestCreateMetricsPreservesOtherUserMetadata(NativeResourceTest): + """Test that non-reserved user metadata keys are preserved.""" + + def test_sdk_version_preserved(self): + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="IoTSDKVersion", value="2.0.0"), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertEqual("2.0.0", metadata_dict["IoTSDKVersion"]) + + def test_custom_key_preserved(self): + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="CustomKey", value="custom_value"), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertEqual("custom_value", metadata_dict["CustomKey"]) + + def test_mixed_metadata(self): + user = AWSIoTMetrics() + user.metadata_entries = [ + IoTMetricsMetadata(key="CRTVersion", value="should_be_ignored"), + IoTMetricsMetadata(key="IoTSDKVersion", value="2.0.0"), + IoTMetricsMetadata(key="CustomKey", value="val"), + ] + + result = create_metrics(user, "F/5,G/A") + metadata_dict = {e.key: e.value for e in result.metadata_entries} + + self.assertNotEqual("should_be_ignored", metadata_dict["CRTVersion"]) + self.assertEqual("2.0.0", metadata_dict["IoTSDKVersion"]) + self.assertEqual("val", metadata_dict["CustomKey"]) + + def test_custom_library_name(self): + user = AWSIoTMetrics(library_name="MyCustomSDK/1.0") + + result = create_metrics(user, "F/5,G/A") + + self.assertEqual("MyCustomSDK/1.0", result.library_name) + + +if __name__ == '__main__': + unittest.main() diff --git a/test/test_mqtt.py b/test/test_mqtt.py index af8acb6d9..cf86369bb 100644 --- a/test/test_mqtt.py +++ b/test/test_mqtt.py @@ -630,7 +630,7 @@ def _test_mqtt311_direct_connect_basic_auth(self): port=input_port, username=input_username, password=input_password, - enable_metrics=False) + disable_metrics=True) connection.connect().result(TIMEOUT) connection.disconnect().result(TIMEOUT) @@ -762,7 +762,7 @@ def sign_function(transform_args, **kwargs): password=input_password, use_websockets=True, websocket_handshake_transform=sign_function, - enable_metrics=False) + disable_metrics=True) connection.connect().result(TIMEOUT) connection.disconnect().result(TIMEOUT) @@ -853,7 +853,7 @@ def _test_mqtt311_direct_connect_basic_auth_metrics_enabled(self): bootstrap = ClientBootstrap(elg, resolver) client = Client(bootstrap, None) - # Create connection with enable_metrics=True explicitly + # Create connection with disable_metrics=False explicitly # This should fail because metrics appends to username, corrupting basic auth connection = Connection( client=client, @@ -862,7 +862,7 @@ def _test_mqtt311_direct_connect_basic_auth_metrics_enabled(self): port=input_port, username=input_username, password=input_password, - enable_metrics=True) + disable_metrics=False) # Connection should fail because metrics corrupts the username for basic auth exception_occurred = False diff --git a/test/test_mqtt5.py b/test/test_mqtt5.py index 1e6dbd080..216fe8b51 100644 --- a/test/test_mqtt5.py +++ b/test/test_mqtt5.py @@ -218,7 +218,7 @@ def _test_direct_connect_minimum(self): client_options = mqtt5.ClientOptions( host_name=input_host_name, - enable_metrics=False + disable_metrics=True ) callbacks = Mqtt5TestCallbacks() client = self._create_client(client_options=client_options, callbacks=callbacks) @@ -245,7 +245,7 @@ def _test_direct_connect_basic_auth(self): host_name=input_host_name, port=input_port, connect_options=connect_options, - enable_metrics=False + disable_metrics=True ) callbacks = Mqtt5TestCallbacks() client = self._create_client(client_options=client_options, callbacks=callbacks) @@ -464,7 +464,7 @@ def _test_websocket_connect_basic_auth(self): host_name=input_host_name, port=input_port, connect_options=connect_options, - enable_metrics=False + disable_metrics=True ) callbacks = Mqtt5TestCallbacks() client_options.websocket_handshake_transform = callbacks.ws_handshake_transform @@ -2249,13 +2249,13 @@ def _test_direct_connect_basic_auth_metrics_enabled(self): host_name=input_host_name, port=input_port, connect_options=connect_options, - enable_metrics=True + disable_metrics=False ) callbacks = Mqtt5TestCallbacks() client = self._create_client(client_options=client_options, callbacks=callbacks) # Verify metrics are enabled - self.assertTrue(client_options.enable_metrics) + self.assertFalse(client_options.disable_metrics) client.start() # Connection should fail because metrics corrupts the username for basic auth From 6c107e111de1df08ac675faccc30c47ccd60bc34 Mon Sep 17 00:00:00 2001 From: Rakshil Modi Date: Tue, 19 May 2026 11:54:35 -0700 Subject: [PATCH 2/8] update doc string format --- awscrt/aws_iot_metrics.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/awscrt/aws_iot_metrics.py b/awscrt/aws_iot_metrics.py index 261075b0c..90b39698b 100644 --- a/awscrt/aws_iot_metrics.py +++ b/awscrt/aws_iot_metrics.py @@ -24,8 +24,7 @@ class IoTMetricsMetadata: @dataclass class AWSIoTMetrics: - """ - Configuration for IoT SDK metrics that are embedded in MQTT Connect Packet username field. + """Configuration for IoT SDK metrics that are embedded in MQTT Connect Packet username field. Args: library_name (str): The SDK library name (e.g., "IoTDeviceSDK/Python") @@ -311,8 +310,7 @@ def _get_encoded_feature_list(client_options): def _get_encoded_feature_list_mqtt3(proxy_options, tls_ctx=None): - """ - Generates the encoded feature list string for metrics from MQTT3 connection options. + """Generates the encoded feature list string for metrics from MQTT3 connection options. Format: "ID/Value,ID/Value..." MQTT3 connections always include: @@ -359,7 +357,7 @@ def _merge_feature_lists(crt_features, user_features): """Merge CRT-generated features with user-provided (IoT SDK) features. When both lists contain the same feature ID, the user-provided value - takes precedence. The result is sorted alphabetically by feature ID. + takes precedence. The result is sorted by feature ID. Args: crt_features (str): CRT-generated feature list. @@ -463,8 +461,7 @@ def create_metrics_mqtt5(client_options): def create_metrics_mqtt3(user_metrics=None, proxy_options=None, tls_ctx=None): - """ - Creates the final AWSIoTMetrics object for an MQTT3 connection. + """Creates the final AWSIoTMetrics object for an MQTT3 connection. Generates the CRT feature list from the MQTT3 connection parameters From 00a436bc7a1fb5678eb7f9fc57145fe2be32b8b3 Mon Sep 17 00:00:00 2001 From: Rakshil Modi Date: Fri, 22 May 2026 15:46:34 -0700 Subject: [PATCH 3/8] publicAPi to private --- awscrt/aws_iot_metrics.py | 80 +++++++++++------------ awscrt/io.py | 6 +- awscrt/mqtt.py | 8 +-- awscrt/mqtt5.py | 8 +-- source/mqtt5_client.c | 108 +++++++++++++++++++------------- source/mqtt5_client.h | 16 +++++ source/mqtt_client_connection.c | 61 ++---------------- test/test_aws_iot_metrics.py | 66 +++++++++---------- 8 files changed, 168 insertions(+), 185 deletions(-) diff --git a/awscrt/aws_iot_metrics.py b/awscrt/aws_iot_metrics.py index 90b39698b..920a22f23 100644 --- a/awscrt/aws_iot_metrics.py +++ b/awscrt/aws_iot_metrics.py @@ -41,7 +41,7 @@ class AWSIoTMetrics: # Feature ID Constants -class MetricsFeatureId(str, Enum): +class _MetricsFeatureId(str, Enum): """Feature IDs for IoT SDK metrics tracking. Each ID is a single character used to encode feature usage in the metrics @@ -63,7 +63,7 @@ class MetricsFeatureId(str, Enum): # Feature Value Constants -class MetricsProtocolVersionValue(str, Enum): +class _MetricsProtocolVersionValue(str, Enum): """Protocol version values for metrics encoding. Maps MQTT protocol versions to their single-character metric representations. @@ -72,7 +72,7 @@ class MetricsProtocolVersionValue(str, Enum): MQTT5 = "5" -class MetricsSocketImplementationValue(str, Enum): +class _MetricsSocketImplementationValue(str, Enum): """Socket implementation values for metrics encoding. Maps the underlying platform socket layer to its metric representation. @@ -82,7 +82,7 @@ class MetricsSocketImplementationValue(str, Enum): WINSOCK = "B" -class MetricsHttpProxyTypeValue(str, Enum): +class _MetricsHttpProxyTypeValue(str, Enum): """HTTP proxy type values for metrics encoding. Indicates whether the proxy connection uses plain HTTP or HTTPS (TLS). @@ -203,13 +203,13 @@ def _tls_cipher_preference_metrics_value(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 + Returns _MetricsSocketImplementationValue.WINSOCK on Windows, + _MetricsSocketImplementationValue.POSIX on all other platforms (macOS, Linux). """ if sys.platform == "win32": - return MetricsSocketImplementationValue.WINSOCK - return MetricsSocketImplementationValue.POSIX + return _MetricsSocketImplementationValue.WINSOCK + return _MetricsSocketImplementationValue.POSIX # MQTT5 encoding list @@ -248,61 +248,61 @@ def _get_encoded_feature_list(client_options): if client_options.retry_jitter_mode is not None: val = _retry_jitter_metrics_value(client_options.retry_jitter_mode) if val: - features.append(f"{MetricsFeatureId.RETRY_JITTER_MODE.value}/{val}") + features.append(f"{_MetricsFeatureId.RETRY_JITTER_MODE.value}/{val}") # B: session_behavior if client_options.session_behavior is not None: val = _client_session_behavior_metrics_value(client_options.session_behavior) if val: - features.append(f"{MetricsFeatureId.SESSION_BEHAVIOR.value}/{val}") + features.append(f"{_MetricsFeatureId.SESSION_BEHAVIOR.value}/{val}") # C: offline_queue_behavior if client_options.offline_queue_behavior is not None: val = _client_operation_queue_behavior_metrics_value(client_options.offline_queue_behavior) if val: - features.append(f"{MetricsFeatureId.OFFLINE_QUEUE_BEHAVIOR.value}/{val}") + features.append(f"{_MetricsFeatureId.OFFLINE_QUEUE_BEHAVIOR.value}/{val}") # D: outbound_topic_alias_behavior if client_options.topic_aliasing_options is not None: if client_options.topic_aliasing_options.outbound_behavior is not None: val = _outbound_topic_alias_behavior_metrics_value(client_options.topic_aliasing_options.outbound_behavior) if val: - features.append(f"{MetricsFeatureId.OUTBOUND_TOPIC_ALIAS_BEHAVIOR.value}/{val}") + features.append(f"{_MetricsFeatureId.OUTBOUND_TOPIC_ALIAS_BEHAVIOR.value}/{val}") # E: inbound_topic_alias_behavior if client_options.topic_aliasing_options is not None: if client_options.topic_aliasing_options.inbound_behavior is not None: val = _inbound_topic_alias_behavior_metrics_value(client_options.topic_aliasing_options.inbound_behavior) if val: - features.append(f"{MetricsFeatureId.INBOUND_TOPIC_ALIAS_BEHAVIOR.value}/{val}") + 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}/{_MetricsProtocolVersionValue.MQTT5.value}") # 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}/{_detect_socket_implementation().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( + 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}") + None) is not None else _MetricsHttpProxyTypeValue.HTTP + features.append(f"{_MetricsFeatureId.HTTP_PROXY_TYPE.value}/{proxy_type.value}") # I: certificate_source - Would need to be tracked from TLS context setup. This is set at a IoT SDK level # J: tls_cipher_preference - security policy if client_options.tls_ctx is not None: - val = _tls_cipher_preference_metrics_value(client_options.tls_ctx.cipher_pref) + val = _tls_cipher_preference_metrics_value(client_options.tls_ctx._cipher_pref) if val: - features.append(f"{MetricsFeatureId.TLS_CIPHER_PREFERENCE.value}/{val}") + features.append(f"{_MetricsFeatureId.TLS_CIPHER_PREFERENCE.value}/{val}") # K: minimum_tls_version - The minimum TLS version set on TLSContextOptions if client_options.tls_ctx is not None: - val = _minimum_tls_version_metrics_value(client_options.tls_ctx.min_tls_ver) + val = _minimum_tls_version_metrics_value(client_options.tls_ctx._min_tls_ver) if val: - features.append(f"{MetricsFeatureId.MINIMUM_TLS_VERSION.value}/{val}") + features.append(f"{_MetricsFeatureId.MINIMUM_TLS_VERSION.value}/{val}") return ",".join(features) @@ -329,26 +329,26 @@ 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}/{_MetricsProtocolVersionValue.MQTT311.value}", + f"{_MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_detect_socket_implementation().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}") + 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}") # J: tls_cipher_preference - security policy if tls_ctx is not None: - val = _tls_cipher_preference_metrics_value(tls_ctx.cipher_pref) + val = _tls_cipher_preference_metrics_value(tls_ctx._cipher_pref) if val: - features.append(f"{MetricsFeatureId.TLS_CIPHER_PREFERENCE.value}/{val}") + features.append(f"{_MetricsFeatureId.TLS_CIPHER_PREFERENCE.value}/{val}") # K: minimum_tls_version - the minimum TLS version set on TLSContextOptions if tls_ctx is not None: - val = _minimum_tls_version_metrics_value(tls_ctx.min_tls_ver) + val = _minimum_tls_version_metrics_value(tls_ctx._min_tls_ver) if val: - features.append(f"{MetricsFeatureId.MINIMUM_TLS_VERSION.value}/{val}") + features.append(f"{_MetricsFeatureId.MINIMUM_TLS_VERSION.value}/{val}") return ",".join(features) @@ -357,14 +357,14 @@ def _merge_feature_lists(crt_features, user_features): """Merge CRT-generated features with user-provided (IoT SDK) features. When both lists contain the same feature ID, the user-provided value - takes precedence. The result is sorted by feature ID. + takes precedence. Args: crt_features (str): CRT-generated feature list. user_features (str): User-provided feature list from the IoT SDK. May be empty string if no SDK features are provided. Returns: - str: The merged feature list string, sorted by feature ID. + str: The merged feature list string. """ merged = {} # Parse CRT Features @@ -377,12 +377,12 @@ def _merge_feature_lists(crt_features, user_features): if "/" in pair: fid, val = pair.split("/", 1) merged[fid] = val - return ",".join(f"{k}/{v}" for k, v in sorted(merged.items())) + return ",".join(f"{k}/{v}" for k, v in merged.items()) # Metrics creation -def create_metrics(user_metrics, crt_feature_list): +def _create_metrics(user_metrics, crt_feature_list): """Create the final AWSIoTMetrics object by merging CRT and user-provided data. Applies the following rules to produce the final metrics: @@ -433,7 +433,7 @@ def create_metrics(user_metrics, crt_feature_list): # Merge features: if version matches, merge CRT + SDK; otherwise CRT only if (user_metrics_version is not None and user_metrics_version.isdigit() and int( - user_metrics_version) == IOT_SDK_METRICS_FEATURE_VERSION and user_feature): + user_metrics_version) == IOT_SDK_METRICS_FEATURE_VERSION): metadata["IoTSDKFeature"] = _merge_feature_lists(crt_feature_list, user_feature) else: metadata["IoTSDKFeature"] = _merge_feature_lists(crt_feature_list, "") @@ -445,7 +445,7 @@ def create_metrics(user_metrics, crt_feature_list): return final_metrics -def create_metrics_mqtt5(client_options): +def _create_metrics_mqtt5(client_options): """Create the final AWSIoTMetrics object for an MQTT5 client. Generates the CRT feature list from the full set of MQTT5 ClientOptions @@ -457,10 +457,10 @@ def create_metrics_mqtt5(client_options): AWSIoTMetrics: The final metrics object with merged CRT and SDK features. """ crt_feature_list = _get_encoded_feature_list(client_options) - return create_metrics(client_options.metrics, crt_feature_list) + return _create_metrics(client_options.metrics, crt_feature_list) -def create_metrics_mqtt3(user_metrics=None, proxy_options=None, tls_ctx=None): +def _create_metrics_mqtt3(user_metrics=None, proxy_options=None, tls_ctx=None): """Creates the final AWSIoTMetrics object for an MQTT3 connection. Generates the CRT feature list from the MQTT3 connection parameters @@ -477,4 +477,4 @@ def create_metrics_mqtt3(user_metrics=None, proxy_options=None, tls_ctx=None): AWSIoTMetrics: The final metrics object with merged CRT and SDK features. """ crt_feature_list = _get_encoded_feature_list_mqtt3(proxy_options, tls_ctx) - return create_metrics(user_metrics, crt_feature_list) + return _create_metrics(user_metrics, crt_feature_list) diff --git a/awscrt/io.py b/awscrt/io.py index cfb340ecc..8bfd0dd79 100644 --- a/awscrt/io.py +++ b/awscrt/io.py @@ -606,15 +606,15 @@ class ClientTlsContext(NativeResource): Args: options (TlsContextOptions): Configuration options. """ - __slots__ = ('min_tls_ver', 'cipher_pref') + __slots__ = ('_min_tls_ver', '_cipher_pref') def __init__(self, options): assert isinstance(options, TlsContextOptions) super().__init__() - self.min_tls_ver = options.min_tls_ver - self.cipher_pref = options.cipher_pref + self._min_tls_ver = options.min_tls_ver + self._cipher_pref = options.cipher_pref self._binding = _awscrt.client_tls_ctx_new( options.min_tls_ver.value, diff --git a/awscrt/mqtt.py b/awscrt/mqtt.py index 6ea9df7f6..91e5ed6e4 100644 --- a/awscrt/mqtt.py +++ b/awscrt/mqtt.py @@ -16,7 +16,7 @@ from awscrt.io import ClientBootstrap, ClientTlsContext, SocketOptions from dataclasses import dataclass from awscrt.mqtt5 import Client as Mqtt5Client -from awscrt.aws_iot_metrics import AWSIoTMetrics, IoTMetricsMetadata, create_metrics_mqtt3 +from awscrt.aws_iot_metrics import AWSIoTMetrics, IoTMetricsMetadata, _create_metrics_mqtt3 class QoS(IntEnum): @@ -415,10 +415,10 @@ def __init__(self, self.password = password self.socket_options = socket_options if socket_options else SocketOptions() self.proxy_options = proxy_options if proxy_options else websocket_proxy_options - if not disable_metrics: - self._metrics = create_metrics_mqtt3(metrics, self.proxy_options, self.client.tls_ctx) - else: + if disable_metrics: self._metrics = None + else: + self._metrics = _create_metrics_mqtt3(metrics, self.proxy_options, self.client.tls_ctx) self._binding = _awscrt.mqtt_client_connection_new( self, diff --git a/awscrt/mqtt5.py b/awscrt/mqtt5.py index 8f0da1997..57ee84202 100644 --- a/awscrt/mqtt5.py +++ b/awscrt/mqtt5.py @@ -15,7 +15,7 @@ from dataclasses import dataclass from collections.abc import Sequence from inspect import signature -from awscrt.aws_iot_metrics import AWSIoTMetrics, IoTMetricsMetadata, create_metrics_mqtt5 +from awscrt.aws_iot_metrics import AWSIoTMetrics, IoTMetricsMetadata, _create_metrics_mqtt5 class QoS(IntEnum): @@ -1821,10 +1821,10 @@ def __init__(self, client_options: ClientOptions): socket_options = SocketOptions() # Handle metrics configuration - if not client_options.disable_metrics: - self._metrics = create_metrics_mqtt5(client_options) - else: + if client_options.disable_metrics: self._metrics = None + else: + self._metrics = _create_metrics_mqtt5(client_options) if not connect_options.will: is_will_none = True diff --git a/source/mqtt5_client.c b/source/mqtt5_client.c index f8faa0431..3ab57768a 100644 --- a/source/mqtt5_client.c +++ b/source/mqtt5_client.c @@ -831,6 +831,67 @@ static bool s_py_topic_aliasing_options_init( return success; } +/******************************************************************************* + * Metrics Parsing + ******************************************************************************/ + +bool aws_py_metrics_parse(PyObject *metrics_py, struct aws_mqtt_iot_metrics *out_metrics) { + AWS_ZERO_STRUCT(*out_metrics); + + 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"); + + if (metadata_entries_py && metadata_entries_py != Py_None && PyList_Check(metadata_entries_py)) { + Py_ssize_t count = PyList_Size(metadata_entries_py); + 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); + PyObject *key_py = PyObject_GetAttrString(entry_py, "key"); + PyObject *value_py = PyObject_GetAttrString(entry_py, "value"); + + entries[i].key = aws_byte_cursor_from_pyunicode(key_py); + entries[i].value = aws_byte_cursor_from_pyunicode(value_py); + + 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; + } + } + + Py_XDECREF(metadata_entries_py); + return true; +} + +void aws_py_metrics_clean_up(struct aws_mqtt_iot_metrics *metrics) { + if (metrics->metadata_entries) { + aws_mem_release(aws_py_get_allocator(), (void *)metrics->metadata_entries); + } +} + /******************************************************************************* * Client Init ******************************************************************************/ @@ -974,9 +1035,6 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { AWS_ZERO_STRUCT(tls_options); struct aws_mqtt5_user_property *user_properties_tmp = NULL; struct aws_mqtt5_user_property *will_user_properties_tmp = NULL; - struct aws_mqtt_metadata_entry *metadata_entries = NULL; - PyObject *library_name_py = NULL; - PyObject *metadata_entries_py = NULL; struct aws_mqtt5_client_options client_options; AWS_ZERO_STRUCT(client_options); @@ -1325,45 +1383,9 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { AWS_ZERO_STRUCT(metrics_tmp); if (PyObject_IsTrue(is_metrics_enabled_py) && metrics_py != Py_None) { - library_name_py = PyObject_GetAttrString(metrics_py, "library_name"); - metrics_tmp.library_name = aws_byte_cursor_from_pyunicode(library_name_py); - if (!metrics_tmp.library_name.ptr) { - PyErr_SetString(PyExc_TypeError, "metrics.library_name must be str type"); + if (!aws_py_metrics_parse(metrics_py, &metrics_tmp)) { goto done; } - - metadata_entries_py = PyObject_GetAttrString(metrics_py, "metadata_entries"); - - if (metadata_entries_py && metadata_entries_py != Py_None && PyList_Check(metadata_entries_py)) { - Py_ssize_t count = PyList_Size(metadata_entries_py); - if (count > 0) { - metadata_entries = - aws_mem_calloc(aws_py_get_allocator(), (size_t)count, sizeof(struct aws_mqtt_metadata_entry)); - if (!metadata_entries) { - PyErr_SetAwsLastError(); - goto done; - } - - for (Py_ssize_t i = 0; i < count; ++i) { - PyObject *entry_py = PyList_GetItem(metadata_entries_py, i); - PyObject *key_py = PyObject_GetAttrString(entry_py, "key"); - PyObject *value_py = PyObject_GetAttrString(entry_py, "value"); - - metadata_entries[i].key = aws_byte_cursor_from_pyunicode(key_py); - metadata_entries[i].value = aws_byte_cursor_from_pyunicode(value_py); - - Py_XDECREF(key_py); - Py_XDECREF(value_py); - - if (!metadata_entries[i].key.ptr || !metadata_entries[i].value.ptr) { - PyErr_SetString(PyExc_TypeError, "metadata_entries items must have str key and value"); - goto done; - } - } - metrics_tmp.metadata_count = (size_t)count; - metrics_tmp.metadata_entries = metadata_entries; - } - } client_options.metrics = &metrics_tmp; } @@ -1411,11 +1433,7 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { PyBuffer_Release(&will_correlation_data_stack); /* Cleanup metrics */ - Py_XDECREF(library_name_py); - Py_XDECREF(metadata_entries_py); - if (metadata_entries) { - aws_mem_release(allocator, metadata_entries); - } + aws_py_metrics_clean_up(&metrics_tmp); if (success) { return capsule; diff --git a/source/mqtt5_client.h b/source/mqtt5_client.h index d05930177..129b98530 100644 --- a/source/mqtt5_client.h +++ b/source/mqtt5_client.h @@ -7,6 +7,22 @@ #include "module.h" +#include + +/** + * 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. + * + * On failure, returns false and a Python error has been set. + */ +bool aws_py_metrics_parse(PyObject *metrics_py, struct aws_mqtt_iot_metrics *out_metrics); + +/** + * Clean up resources allocated by aws_py_metrics_parse(). + */ +void aws_py_metrics_clean_up(struct aws_mqtt_iot_metrics *metrics); + PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args); PyObject *aws_py_mqtt5_client_start(PyObject *self, PyObject *args); PyObject *aws_py_mqtt5_client_stop(PyObject *self, PyObject *args); diff --git a/source/mqtt_client_connection.c b/source/mqtt_client_connection.c index ff31dbf56..ab5f4e2ae 100644 --- a/source/mqtt_client_connection.c +++ b/source/mqtt_client_connection.c @@ -445,69 +445,18 @@ static bool s_set_metrics(struct aws_mqtt_client_connection *connection, PyObjec return false; } - bool success = false; - struct aws_mqtt_iot_metrics metrics_tmp; - AWS_ZERO_STRUCT(metrics_tmp); - PyObject *metadata_entries_py = NULL; - struct aws_mqtt_metadata_entry *metadata_entries = NULL; - size_t metadata_count = 0; - - PyObject *library_name_py = PyObject_GetAttrString(metrics, "library_name"); - metrics_tmp.library_name = aws_byte_cursor_from_pyunicode(library_name_py); - if (!metrics_tmp.library_name.ptr) { - PyErr_SetString(PyExc_TypeError, "metrics.library_name must be str type"); - goto done; - } - - metadata_entries_py = PyObject_GetAttrString(metrics, "metadata_entries"); - - if (metadata_entries_py && metadata_entries_py != Py_None && PyList_Check(metadata_entries_py)) { - Py_ssize_t count = PyList_Size(metadata_entries_py); - if (count > 0) { - metadata_entries = - aws_mem_calloc(aws_py_get_allocator(), (size_t)count, sizeof(struct aws_mqtt_metadata_entry)); - if (!metadata_entries) { - PyErr_SetAwsLastError(); - goto done; - } - - for (Py_ssize_t i = 0; i < count; ++i) { - PyObject *entry_py = PyList_GetItem(metadata_entries_py, i); - PyObject *key_py = PyObject_GetAttrString(entry_py, "key"); - PyObject *value_py = PyObject_GetAttrString(entry_py, "value"); - - metadata_entries[i].key = aws_byte_cursor_from_pyunicode(key_py); - metadata_entries[i].value = aws_byte_cursor_from_pyunicode(value_py); - - Py_XDECREF(key_py); - Py_XDECREF(value_py); - - if (!metadata_entries[i].key.ptr || !metadata_entries[i].value.ptr) { - PyErr_SetString(PyExc_TypeError, "metadata_entries items must have str key and value"); - goto done; - } - } - metadata_count = (size_t)count; - } + if (!aws_py_metrics_parse(metrics, &metrics_tmp)) { + return false; } - metrics_tmp.metadata_count = metadata_count; - metrics_tmp.metadata_entries = metadata_entries; - + bool success = true; if (aws_mqtt_client_connection_set_metrics(connection, &metrics_tmp)) { PyErr_SetAwsLastError(); - goto done; + success = false; } - success = true; - -done: - Py_XDECREF(library_name_py); - Py_XDECREF(metadata_entries_py); - if (metadata_entries) { - aws_mem_release(aws_py_get_allocator(), metadata_entries); - } + aws_py_metrics_clean_up(&metrics_tmp); return success; } diff --git a/test/test_aws_iot_metrics.py b/test/test_aws_iot_metrics.py index 98a68d8b4..f7ac9b548 100644 --- a/test/test_aws_iot_metrics.py +++ b/test/test_aws_iot_metrics.py @@ -7,15 +7,15 @@ from awscrt.aws_iot_metrics import ( AWSIoTMetrics, IoTMetricsMetadata, - MetricsFeatureId, - MetricsProtocolVersionValue, - MetricsSocketImplementationValue, - MetricsHttpProxyTypeValue, + _MetricsFeatureId, + _MetricsProtocolVersionValue, + _MetricsSocketImplementationValue, + _MetricsHttpProxyTypeValue, IOT_SDK_METRICS_FEATURE_VERSION, _get_encoded_feature_list, _get_encoded_feature_list_mqtt3, _merge_feature_lists, - create_metrics, + _create_metrics, ) from awscrt.mqtt5 import ( ClientOptions, @@ -32,8 +32,8 @@ def _expected_socket_value(): if sys.platform == "win32": - return MetricsSocketImplementationValue.WINSOCK - return MetricsSocketImplementationValue.POSIX + return _MetricsSocketImplementationValue.WINSOCK + return _MetricsSocketImplementationValue.POSIX class TestMinimalOptionsEncoding(NativeResourceTest): @@ -45,8 +45,8 @@ 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}/{_MetricsProtocolVersionValue.MQTT5.value}", result) + self.assertIn(f"{_MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_expected_socket_value().value}", result) parts = result.split(",") self.assertEqual(2, len(parts)) @@ -54,8 +54,8 @@ 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}/{_MetricsProtocolVersionValue.MQTT311.value}", result) + self.assertIn(f"{_MetricsFeatureId.SOCKET_IMPLEMENTATION.value}/{_expected_socket_value().value}", result) parts = result.split(",") self.assertEqual(2, len(parts)) @@ -71,9 +71,9 @@ def test_default_enum_values_omitted(self): result = _get_encoded_feature_list(options) - self.assertNotIn(f"{MetricsFeatureId.RETRY_JITTER_MODE.value}/", result) - self.assertNotIn(f"{MetricsFeatureId.SESSION_BEHAVIOR.value}/", result) - self.assertNotIn(f"{MetricsFeatureId.OFFLINE_QUEUE_BEHAVIOR.value}/", result) + self.assertNotIn(f"{_MetricsFeatureId.RETRY_JITTER_MODE.value}/", result) + self.assertNotIn(f"{_MetricsFeatureId.SESSION_BEHAVIOR.value}/", result) + self.assertNotIn(f"{_MetricsFeatureId.OFFLINE_QUEUE_BEHAVIOR.value}/", result) class TestOptionsWithMultipleNonDefaultFeaturesEncoding(NativeResourceTest): @@ -195,7 +195,7 @@ def test_user_overrides_crt(self): def test_user_overrides_multiple_crt_features(self): """User can override multiple CRT features at once.""" result = _merge_feature_lists("A/B,F/5,G/A,K/D", "A/C,F/3,K/E") - # User overrides A, F, K; G remains from CRT + # User overrides A, F, K; G remains from CRT; insertion order preserved self.assertEqual("A/C,F/3,G/A,K/E", result) def test_empty_user_features(self): @@ -206,9 +206,9 @@ def test_empty_crt_features(self): result = _merge_feature_lists("", "A/B") self.assertEqual("A/B", result) - def test_sorted_output(self): + def test_insertion_order_preserved(self): result = _merge_feature_lists("G/A,F/5", "A/B") - self.assertEqual("A/B,F/5,G/A", result) + self.assertEqual("G/A,F/5,A/B", result) def test_disjoint_features(self): result = _merge_feature_lists("F/5,G/A", "I/A,K/D") @@ -223,7 +223,7 @@ class TestCreateMetricsWithDefaultOptions(NativeResourceTest): """Test create_metrics with no user metrics or default user metrics.""" def test_none_user_metrics(self): - result = create_metrics(None, "F/5,G/A") + result = _create_metrics(None, "F/5,G/A") self.assertEqual("IoTDeviceSDK/Python", result.library_name) metadata_dict = {e.key: e.value for e in result.metadata_entries} @@ -233,14 +233,14 @@ def test_none_user_metrics(self): def test_empty_user_metrics(self): user = AWSIoTMetrics() - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") self.assertEqual("IoTDeviceSDK/Python", result.library_name) metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertEqual("F/5,G/A", metadata_dict["IoTSDKFeature"]) def test_version_always_set(self): - result = create_metrics(None, "F/5,G/A") + result = _create_metrics(None, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertEqual(str(IOT_SDK_METRICS_FEATURE_VERSION), metadata_dict["IoTSDKMetricsVersion"]) @@ -255,7 +255,7 @@ def test_user_feature_added(self): IoTMetricsMetadata(key="IoTSDKFeature", value="I/A"), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertIn("I/A", metadata_dict["IoTSDKFeature"]) @@ -270,7 +270,7 @@ def test_user_feature_overrides_crt(self): IoTMetricsMetadata(key="IoTSDKFeature", value="F/3,I/B"), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertIn("F/3", metadata_dict["IoTSDKFeature"]) @@ -284,7 +284,7 @@ def test_empty_user_feature_string(self): IoTMetricsMetadata(key="IoTSDKFeature", value=""), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertEqual("F/5,G/A", metadata_dict["IoTSDKFeature"]) @@ -300,7 +300,7 @@ def test_higher_version(self): IoTMetricsMetadata(key="IoTSDKFeature", value="I/A"), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertNotIn("I/A", metadata_dict["IoTSDKFeature"]) @@ -313,7 +313,7 @@ def test_lower_version(self): IoTMetricsMetadata(key="IoTSDKFeature", value="I/A"), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertNotIn("I/A", metadata_dict["IoTSDKFeature"]) @@ -325,7 +325,7 @@ def test_non_numeric_version(self): IoTMetricsMetadata(key="IoTSDKFeature", value="I/A"), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertNotIn("I/A", metadata_dict["IoTSDKFeature"]) @@ -336,7 +336,7 @@ def test_no_version_set(self): IoTMetricsMetadata(key="IoTSDKFeature", value="I/A"), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertNotIn("I/A", metadata_dict["IoTSDKFeature"]) @@ -353,7 +353,7 @@ def test_user_cannot_override_crt_version(self): IoTMetricsMetadata(key="CRTVersion", value="fake_version"), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertEqual(actual_version, metadata_dict["CRTVersion"]) @@ -367,7 +367,7 @@ def test_empty_crt_version_overridden(self): IoTMetricsMetadata(key="CRTVersion", value=""), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertEqual(actual_version, metadata_dict["CRTVersion"]) @@ -382,7 +382,7 @@ def test_sdk_version_preserved(self): IoTMetricsMetadata(key="IoTSDKVersion", value="2.0.0"), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertEqual("2.0.0", metadata_dict["IoTSDKVersion"]) @@ -393,7 +393,7 @@ def test_custom_key_preserved(self): IoTMetricsMetadata(key="CustomKey", value="custom_value"), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertEqual("custom_value", metadata_dict["CustomKey"]) @@ -406,7 +406,7 @@ def test_mixed_metadata(self): IoTMetricsMetadata(key="CustomKey", value="val"), ] - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") metadata_dict = {e.key: e.value for e in result.metadata_entries} self.assertNotEqual("should_be_ignored", metadata_dict["CRTVersion"]) @@ -416,7 +416,7 @@ def test_mixed_metadata(self): def test_custom_library_name(self): user = AWSIoTMetrics(library_name="MyCustomSDK/1.0") - result = create_metrics(user, "F/5,G/A") + result = _create_metrics(user, "F/5,G/A") self.assertEqual("MyCustomSDK/1.0", result.library_name) From 2d9d5b1fd53585a01b745bae7efc4a8ab2badf0b Mon Sep 17 00:00:00 2001 From: Rakshil Modi Date: Tue, 26 May 2026 12:37:12 -0700 Subject: [PATCH 4/8] removing double negative --- awscrt/mqtt5.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/awscrt/mqtt5.py b/awscrt/mqtt5.py index 57ee84202..7c4da4957 100644 --- a/awscrt/mqtt5.py +++ b/awscrt/mqtt5.py @@ -1821,7 +1821,9 @@ 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) @@ -1877,7 +1879,7 @@ def __init__(self, client_options: ClientOptions): client_options.ack_timeout_sec, client_options.topic_aliasing_options, websocket_is_none, - not client_options.disable_metrics, + enable_metrics, self._metrics, core) From a95e89fc598d5b40f2c0ea7f89465e905decc5eb Mon Sep 17 00:00:00 2001 From: Rakshil Modi Date: Tue, 26 May 2026 13:41:03 -0700 Subject: [PATCH 5/8] resolving overflow error --- source/mqtt5_client.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source/mqtt5_client.c b/source/mqtt5_client.c index 3ab57768a..dcc7e356a 100644 --- a/source/mqtt5_client.c +++ b/source/mqtt5_client.c @@ -1035,6 +1035,8 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { AWS_ZERO_STRUCT(tls_options); struct aws_mqtt5_user_property *user_properties_tmp = NULL; struct aws_mqtt5_user_property *will_user_properties_tmp = NULL; + struct aws_mqtt_iot_metrics metrics_tmp; + AWS_ZERO_STRUCT(metrics_tmp); struct aws_mqtt5_client_options client_options; AWS_ZERO_STRUCT(client_options); @@ -1379,9 +1381,6 @@ PyObject *aws_py_mqtt5_client_new(PyObject *self, PyObject *args) { } /* METRICS */ - struct aws_mqtt_iot_metrics metrics_tmp; - AWS_ZERO_STRUCT(metrics_tmp); - if (PyObject_IsTrue(is_metrics_enabled_py) && metrics_py != Py_None) { if (!aws_py_metrics_parse(metrics_py, &metrics_tmp)) { goto done; From 274af59b460ec92333abe9390637d827b28c86e1 Mon Sep 17 00:00:00 2001 From: Rakshil Modi Date: Mon, 1 Jun 2026 10:29:55 -0700 Subject: [PATCH 6/8] Updates on metrics --- awscrt/aws_iot_metrics.py | 66 +++++++++++++++--------------------- awscrt/mqtt5.py | 4 +-- source/mqtt5_client.c | 16 --------- source/mqtt5_client.h | 14 +++++--- test/test_aws_iot_metrics.py | 18 +++++----- 5 files changed, 47 insertions(+), 71 deletions(-) diff --git a/awscrt/aws_iot_metrics.py b/awscrt/aws_iot_metrics.py index 920a22f23..1e9867d5e 100644 --- a/awscrt/aws_iot_metrics.py +++ b/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: diff --git a/awscrt/mqtt5.py b/awscrt/mqtt5.py index 7c4da4957..57ee84202 100644 --- a/awscrt/mqtt5.py +++ b/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) diff --git a/source/mqtt5_client.c b/source/mqtt5_client.c index dcc7e356a..4c365325e 100644 --- a/source/mqtt5_client.c +++ b/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; diff --git a/source/mqtt5_client.h b/source/mqtt5_client.h index 129b98530..dc3ccb75f 100644 --- a/source/mqtt5_client.h +++ b/source/mqtt5_client.h @@ -10,11 +10,17 @@ #include /** - * 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); diff --git a/test/test_aws_iot_metrics.py b/test/test_aws_iot_metrics.py index f7ac9b548..7b59615c4 100644 --- a/test/test_aws_iot_metrics.py +++ b/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,8 +43,8 @@ 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)) @@ -54,8 +52,8 @@ 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)) From 9bbf5c9313727e080331ab70ac5c52253fd5defa Mon Sep 17 00:00:00 2001 From: Rakshil Modi Date: Mon, 1 Jun 2026 10:35:02 -0700 Subject: [PATCH 7/8] lint --- awscrt/aws_iot_metrics.py | 2 -- source/mqtt5_client.h | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/awscrt/aws_iot_metrics.py b/awscrt/aws_iot_metrics.py index 1e9867d5e..c17614c50 100644 --- a/awscrt/aws_iot_metrics.py +++ b/awscrt/aws_iot_metrics.py @@ -204,8 +204,6 @@ def _tls_cipher_preference_metrics_value(pref): return mapping.get(pref) - - # MQTT5 encoding list def _get_encoded_feature_list(client_options): """Generates the encoded feature list string for metrics from MQTT5 ClientOptions. diff --git a/source/mqtt5_client.h b/source/mqtt5_client.h index dc3ccb75f..2025169e5 100644 --- a/source/mqtt5_client.h +++ b/source/mqtt5_client.h @@ -20,7 +20,7 @@ * * 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); From eff4963db6009e7c55af600a3318ccc1d1744a30 Mon Sep 17 00:00:00 2001 From: Rakshil Modi Date: Mon, 1 Jun 2026 11:28:37 -0700 Subject: [PATCH 8/8] removing doc string --- awscrt/aws_iot_metrics.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/awscrt/aws_iot_metrics.py b/awscrt/aws_iot_metrics.py index c17614c50..c3db4f909 100644 --- a/awscrt/aws_iot_metrics.py +++ b/awscrt/aws_iot_metrics.py @@ -94,8 +94,6 @@ def _http_proxy_type_metrics_value(proxy_options): return "B" return "A" -# Mappings from existing enums to metrics values - def _retry_jitter_metrics_value(mode): """Map ExponentialBackoffJitterMode to its single-character metrics value.