-
Notifications
You must be signed in to change notification settings - Fork 467
metrics-collector - split D2C messages greater than limit #6904
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…sages are greater than limit
|
@microsoft-github-policy-service agree |
|
Any chance this could be reviewed and merged? It's a very handy feature and we are currently having to run patched versions of this in order to work around the issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd need to see the results of performance testing for this change, since this PR changes the performance characteristics of the module significantly, even for metrics that don't exceed the size threshold.
This PR would also need to include tests for the updated logic to demonstrate that the new code behaves as expected, and that the integrity of metrics data is always preserved, even when it is split into smaller messages.
| { | ||
| foreach (IEnumerable<ExportMetric> metrics in splitMetrics) | ||
| { | ||
| metricsData = serializeMetrics(metrics); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A list of metrics that is close to but not exceeding the size limit will always be serialized twice, including possibly transforming the data for IoT Central and compressing the data. If the list of metrics exceeds the size limit, then the entire list will be serialized, followed by half the list, etc., until all sub-lists are within the size, then each sublist is serialized again. So we're going from O(n) to O(n log n) performance. This will likely be a noticeable (and possibly unacceptable) performance dip for many users.
We currently use metrics-collector configured with environment variable
IoTMessageto send metrics as D2C messages upstream to IoT Hub. The current version of metrics-collector does not split D2C messages, even if they are greater than the D2C message limit (256KB). This results in us currently getting a MessageTooLargeException when trying to scrape our custom modules in addition to edgeHub and edgeAgent. This PR adds functionality to split D2C messages before sending upstream if messages are greater than limit.Azure IoT Edge PR checklist:
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines and Best Practices
Testing Guidelines