refactor: replace the collectd mapper with local flows

[didier-wenzek]

Proposed changes

The goal of this PR is to deprecate the collectd mapper providing the tools to implement the same using flows

• Flow script translating collectd messages into te measurements
• Flow transformer batching messages
• Merge Collectd Flow example: feat: translate collect metrics into tedge measurements tedge-flows-examples#121
• Merge Collectd setup: feat!: consume collectd data using a flow tedge-collectd-setup#6 (review)
• Rewrite system tests to use a flow-based collectd mapper
• Update sections of the docs that use collectd to illustrate device monitoring
• Deprecate the batcher actor
• Deprecate the collectd mapper

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
• Documentation Update (if none of the other choices apply)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
• I ran just format as mentioned in CODING_GUIDELINES
• I used just check as mentioned in CODING_GUIDELINES
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Further comments

[github-actions]

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %	⏱️ Duration
919	0	7	919	100	2h18m42.062401s

[codecov]

Codecov Report

❌ Patch coverage is 91.06700% with 36 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...tedge_flows/src/transformers/group_measurements.rs	91.56%	24 Missing and 3 partials ⚠️
crates/extensions/tedge_flows/src/flow.rs	43.75%	9 Missing ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[didier-wenzek]

These checks have been moved to the suite setup. They are not system tests but prerequisite checks.

[rina23q]

I guess still code is in progress, so I reviewed the documentation changes.

[rina23q]

(unrelated) Out of curiosity, The watchdog feature doesn't work also for aws and local mappers?
I found this document is actually unlisted, so maybe it doesn't matter how precise information we have here.

[rina23q]

Probably we can also remove the section "Collectd logs" below (inside "Third-party component logs"). Instead, we can mention how to see the log of collectd in the new repository for the collectd flow.

[rina23q]

I guess this collectd.conf file is also a candidate to move out of the thin-edge.io repository later.

[didier-wenzek]

Updating the docs

The following sections use collectd as an example of an app that can be managed with thin-edge => okay, unchanged

• https://thin-edge.github.io/thin-edge.io/references/agent/tedge-configuration-management/
• https://thin-edge.github.io/thin-edge.io/references/agent/device-management-api/
• https://thin-edge.github.io/thin-edge.io/references/agent/software-management/
• https://thin-edge.github.io/thin-edge.io/references/agent/device-profiles/
• https://thin-edge.github.io/thin-edge.io/references/mappers/c8y-mapper/
• https://thin-edge.github.io/thin-edge.io/references/mappers/flows/#flow-configuration
• https://thin-edge.github.io/thin-edge.io/references/mqtt-api/
• https://thin-edge.github.io/thin-edge.io/references/software-management-plugin-api/
• https://thin-edge.github.io/thin-edge.io/understand/faq/

Partially updated

• https://thin-edge.github.io/thin-edge.io/next/operate/troubleshooting/log-files/

Fully removed because only focused on collectd

• https://thin-edge.github.io/thin-edge.io/operate/troubleshooting/device-monitoring/

What about those ones?

• https://thin-edge.github.io/thin-edge.io/start/getting-started/#step-4-monitor-the-device
• https://thin-edge.github.io/thin-edge.io/start/device-monitoring/

I decided to fully remove the first one i.e. from "A tour of thin-edge.io". Indeed, the point of this tour is to highlight key features, not extensions.
Not perfect, because collectd is used as an example for configuration management :-(

The section "device monitoring" can be kept.
But, I propose to move it under https://thin-edge.github.io/thin-edge.io/operate/monitoring/.

[reubenmiller]

Great that we can split the collectd stuff and decouple it from the thin-edge.io project, though moving this functionality outside of the core package will required a multi-step transition to ensure we don't break the existing package and can cleaning transition the core collectd logic to the tedge-collectd-setup package (and rename the package in the future).

So I would propose the following actions to facilitate the transition:

1. Split out the Rust related commits from this PR so that we can merge the new "group-measurements" Rust based step for future use by the collectd flow
2. Create a naive "group-measurement" JavaScript implementation of the "group-measurements" so that it can be used in the tedge-collectd-setup in this transition period until the next thin-edge.io release
3. After the next official thin-edge.io release, merge the collectd packaging changes (and docs) to remove the tedge-mapper-collectd service handling. This will require a change in the maintainer script generator to also cleanly stop any existing tedge-mapper-collectd service before removing it so that we don't leave a stale service afterwards (this is something I can help)
4. In the tedge-collectd-setup, transition to use the Rust based "group-measurements" step instead of the naive Javascript implementation (and add the specific thin-edge.io package version as a dependency to the package)

[didier-wenzek]

1. Split out the Rust related commits from this PR so that we can merge the new "group-measurements" Rust based step for future use by the collectd flow

Done: #4201

2. Create a naive "group-measurement" JavaScript implementation of the "group-measurements" so that it can be used in the tedge-collectd-setup in this transition period until the next thin-edge.io release

Done: thin-edge/tedge-collectd-setup@c7e0b73

3. After the next official thin-edge.io release, merge the collectd packaging changes (and docs) to remove the tedge-mapper-collectd service handling. This will require a change in the maintainer script generator to also cleanly stop any existing tedge-mapper-collectd service before removing it so that we don't leave a stale service afterwards (this is something I can help)

I will clean up that PR, once merged #4201

4. In the tedge-collectd-setup, transition to use the Rust based "group-measurements" step instead of the naive Javascript implementation (and add the specific thin-edge.io package version as a dependency to the package)

This will be a simple change reverting thin-edge/tedge-collectd-setup@c7e0b73

-   { script = "group-measurements.js", interval = "1s" },
+   { builtin = "group-measurements", interval = "1s", config = { time_window = "500ms" } },

[albinsuresh]

The refactoring has significantly improved the testability of the batching logic and that itself is a huge win.

The only thing that I found weird is batches getting split when conflicting measurements arrived within a batch window. I was wondering if replacing the old values with newer would have been better, so that batches are always emitted at the configured intervals. But, I understand our current rationale to push those into separate batches so that no mesurements are lost and the batch timestamps don't need dynamic re-compute either, which would have further complicated the batching logic.

[didier-wenzek]

The only thing that I found weird is batches getting split when conflicting measurements arrived within a batch window. I was wondering if replacing the old values with newer would have been better, so that batches are always emitted at the configured intervals.

Yeah, the main difference with the deprecated batcher is that there is no attempt to concurrently build more than one batch of messages per topic. So when a message is received for a topic but cannot be added to the associated batch we are forced to take a decision:

• either to keep the batch unchanged and to forward the new message: this is what is done for late messages
• or to close the current batch and to create a new one with the new messages: this is what is done for conflicting messages and those more recent than the batch time-window

But, I understand our current rationale to push those into separate batches so that no mesurements are lost

Indeed, avoiding message lost has been the driver.

the batch timestamps don't need dynamic re-compute either, which would have further complicated the batching logic.

There is a bit of window re-computation logic though. When a message is received late (i.e. its event time is older than the oldest batched so far) but not too late (i.e. the difference between that message and the newest batched so far is within the configured time window), then that message is accepted and the batch time window adjusted accordingly.