feat: filter log types using include/exclude patterns #3838

albinsuresh · 2025-10-29T14:50:07Z

Allow users to filter log types reported by log plugins using include and exclude patterns in the plugin configuration.

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

codecov · 2025-10-29T15:07:05Z

Codecov Report

❌ Patch coverage is 95.56452% with 11 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/extensions/tedge_log_manager/src/config.rs	95.92%	8 Missing and 1 partial ⚠️
crates/extensions/tedge_log_manager/src/actor.rs	80.00%	2 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.

github-actions · 2025-10-29T16:41:21Z

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %	⏱️ Duration
742	0	3	742	100	2h23m57.611217999s

didier-wenzek · 2025-10-31T09:26:52Z

docs/src/extend/log-management.md

+[[plugins.journald]]
+include = "tedge-*"
+
+[[plugins.journald]]
+exclude = "systemd*"


According to the filtering rules, this include clause is useless. I would be good to come with an example that better demonstrates the usefulness of include / exclude clauses.

The point here is only to show something sensible, the details being explained in the following sections.

It can be "exclude all systemd services except systemd-logind".

Or better, something simpler with two plugins:

Suggested change

[[plugins.journald]]

include = "tedge-*"

[[plugins.journald]]

exclude = "systemd*"

[[plugins.docker]]

include = "tedge-*"

[[plugins.journald]]

exclude = "systemd*"

Or better, something simpler with two plugins:

Yup, I'll do that since this is just the introduction section

It can be "exclude all systemd services except systemd-logind".

Have included that example in the examples section below.

Resolved by 2d9f927

didier-wenzek · 2025-10-31T09:43:09Z

crates/extensions/tedge_log_manager/src/config.rs

+                if let Ok(compiled) = glob::Pattern::new(pattern) {
+                    include_patterns.push(compiled);
+                } else {


I would extract this compilation stage out of the filter_log_types method. Can this be done when the file is read?

Can this be done when the file is read?

Yes. We can keep these patterns compiled when the config file is read/reloaded.

Resolved by 2d9f927

didier-wenzek · 2025-10-31T09:48:44Z

crates/extensions/tedge_log_manager/src/config.rs

+pub struct PluginFilterEntry {
+    #[serde(default)]
+    pub include: Option<String>,
+    #[serde(default)]
+    pub exclude: Option<String>,
+}


Looking how this is used, an enum seems more appropriate.

Suggested change

pub struct PluginFilterEntry {

#[serde(default)]

pub include: Option<String>,

#[serde(default)]

pub exclude: Option<String>,

}

pub enum PluginFilterEntry {

Include(String),

Exclude(String),

}

I would even go a step further with:

Suggested change

pub struct PluginFilterEntry {

#[serde(default)]

pub include: Option<String>,

#[serde(default)]

pub exclude: Option<String>,

}

pub enum PluginFilterEntry {

Include(glob::Pattern),

Exclude(glob::Pattern),

}

After this comment from @reubenmiller I'm having second thoughts on converting that struct definition into an enum as it prevents users from defining both include and exclude filters under the same plugin entry as follows:

[[plugins.journald]] exclude = "systemd-*" include = "systemd-logind"

The struct might be more future-proof as well if we want to support more fields beyond include and exclude in future.

My suggestion is not to prevent a combination of include and exclude rules for a given plugin, but to simplify the representation and the implementation logic. Note that currently a Vec<PluginFilterEntry>> is attached to each log type, making the enum enough and clearer. Two different structs might be required, though. A first one for the TOML file, aka PluginConfig, a second one to control the extent of log types to manage.

Have done the compilation to regex pattern in 2d9f927. The enum couldn't be done due to the possibility of both include and exclude filters under the [[plugins.journald]] entry, unless we use an enum like:

enum Filter { Include(Regex) Exclude(Regex) Both(Regex) }

which would make little difference in the filtering logic.

Okay let's keep the struct.

However, I disagree with the rationale. The in-memory representation of the filters doesn't have to match the external schema of the configuration file. We could have a vector of enum values with two cases or two vectors (one for the included regex the other for the excluded ones)

We could have a vector of enum values with two cases or two vectors (one for the included regex the other for the excluded ones)

Aah yes. For the internal representation, we can absolutely do that. I was only thinking of the external struct used by serde.

Resolved that as well in 9d4bbb6 (squashed commit) with a TomlPluginConfig struct used for parsing and a compiled PluginConfig struct that maintains include_patterns and exclude_patterns separately.

didier-wenzek · 2025-10-31T09:57:41Z

crates/extensions/tedge_log_manager/src/config.rs

+                }
+            },
+            Err(_) => {
+                // File doesn't exist or not readable - this is OK, just use defaults


Okay, but it would be good to warn the user.

This is a good idea. I would be careful with the wording if this is potentially not actually an issue, just highlighting that we didn't read the configuration from the specified file. If the default behaviour of thin-edge is that this file doesn't exist, this warning will appear often, and we should make it clear that this is not an issue. I may also be misassuming how things work in this case, and it may be the case that we can cope but this is a weird edge case that the user should fix, in which case a normal warning would be entirely justified.

Resolved by 2d9f927.

reubenmiller · 2025-10-30T17:22:42Z

docs/src/extend/log-management.md

+### Configuration Format
+
+Use the `[[plugins.<plugin-name>]]` sections to define filters.
+Both `include` and `exclude` support [glob patterns](https://en.wikipedia.org/wiki/Glob_(programming)).


It might be worth using regex instead of glob patterns to be consistent with other parts of tedge (e.g. the sm plugin filtering)

Note that we are using glob patterns for file paths in config/log management. However, I also agree with using regex patterns for filtering.

Switched to regex patterns in 2d9f927.

reubenmiller · 2025-11-03T10:33:06Z

docs/src/extend/log-management.md

+[[plugins.journald]]
+include = "systemd-logind"
+
+[[plugins.journald]]
+exclude = "systemd-*"


I'm not sure about the chosen data format here, as it involves arrays of objects, which makes it a bit difficult to manipulate. Could we just have a flat object structure and change the "include" and "exclude" keys to be an array?

e.g.

[plugin.journald] include = ["systemd-logind"] exclude = ["systemd-*"]

Yeah, I've had much deliberation on this format. And I agree that the very first format that came to my mind was exactly what you've proposed, which is fairly intuitive as well. But I went with the array of objects for easier manipulation and extensibility.

I agree that all include patterns in a single array would be easier for manual manipulation, but if a user wants to programatically add a new include pattern, it'd be a lot more difficult to append to an existing array, right?

With the proposed array of objects patterns, it can be a simple append of the following block, without having to know the existing values in the pattern:

[[plugin.journald]] include = "new-pattern-*"

Programmatically removing an existing pattern looks equally difficult with both formats, but I'm expecting more dynamic appends than removals.

This format would also come in handy in future, if/when we add support for drop-in extension files for tedge-log-plugin.toml. In that case also, a new include/exclude pattern for a plugin can be defined in an extension file the exact same way, even if include/exclude patterns for the same plugin are defined in the main tedge-log-plugin.toml` or any other extension file. But if we go with the simple format that you've proposed, a new pattern cannot be added to an extension file if the include patterns for that same plugin is defined elsewhere.

These were the reasons why I went with the array of objects format, although I agree that it is not the most intuitive format for someone new to TOML.

I'm just wondering if the data model is essentially an array of objects, rather than an array of strings, that this makes it harder to manipulate the data in the future.

Though is there anything stopping people also providing both the include and exclude properties within one array item, e.g.

[[plugins.journald]] include = "systemd-logind" exclude = "systemd-*"

Though is there anything stopping people also providing both the include and exclude properties within one array item, e.g.

[[plugins.journald]] include = "systemd-logind" exclude = "systemd-*"

That's perfectly valid as the keys are different (include and exclude).

Or did you mean to ask if multiple include filters are allowed under the same object array as follows:

[[plugins.journald]] include = "abc" include = "xyz"

OR

[[plugins.journald]] include = ["abc", "def"] include = ["xyz"]

and expect all the include patterns to be combined into a single include array?

That's an invalid toml due to key duplication under the same table. The second include pattern must be placed in a separate [[plugins.journald]] block.

No I just meant the example that I posted. I just wanted to make sure that it also worked, as adding two array items seemed strange when you want to just define an include and an exclude.

After the demo, I'm ok with the overall data structure (as long as the user can also provided include and exclude on the same array item).

jarhodes314 · 2025-11-03T12:03:15Z

crates/extensions/tedge_log_manager/src/config.rs

+    pub fn from_file(path: &std::path::Path) -> Self {
+        match std::fs::read_to_string(path) {


Suggested change

pub fn from_file(path: &std::path::Path) -> Self {

match std::fs::read_to_string(path) {

pub async fn from_file(path: &std::path::Path) -> Self {

match tokio::fs::read_to_string(path).await {

Resolved by 2d9f927.

jarhodes314 · 2025-11-03T15:40:03Z

crates/extensions/tedge_log_manager/src/config.rs

+                let matches_include = include_patterns.iter().any(|p| p.matches(log_type));
+                let matches_exclude = exclude_patterns.iter().any(|p| p.matches(log_type));
+
+                // When both include and exclude patterns exist, use OR logic


I'm slightly confused by this logic. Essentially this means if I have an include filter and I add an exclude filter, that will potentially cause more items to match than before we added the second filter rule. I think my natural expectation is for the two to be ANDed together.

TBH, I agree with you that the natural expectation would be an AND. But this OR combination was implemented because the sm-plugin filtering also does the same and we wanted to keep it consistent. Actually there are use-cases for both.

If it was AND I could define rules like:

[[plugin.journald]] include = "tedge-*" exclude = "tedge-watcher"

where we want to include everything matching tedge-* but selectively exclude tedge-watcher from it.

Now, the ORcan also be used to define the reverse rule:

[[plugin.journald]] exclude = "systemd-*" include = "systemd-logind"

where we exclude everything systemd-* but would like to selectively override that blanket exclusion by including systemd-logind.

Both are useful in my opinion and for some reason we chose the latter for sm-plugins and we're still sticking to it.

What confuses me is the current implementation is the default values.

When no exclude rules are given it means exclude everything, i.e. take only what is explicitly included.

When no include rules are given it means include everything, unless there is an exclude rule in which case it means include nothing.

What confuses me is the current implementation is the default values.

When no exclude rules are given it means exclude everything, i.e. take only what is explicitly included.

When no include rules are given it means include everything, unless there is an exclude rule in which case it means include nothing.

I totally agree that it is difficult to easily put the meaning of that combined case into words as can be done for the other cases as follows:

Neither include or exclude: Include everything

include only: Include just what is specified

exclude only: Include everything that is not excluded

But the combined case is really difficult to put in a sentence:

include and exclude: Include everything that is not excluded + what an be selectively included from the excluded list

If we had done the AND case, it was a lot easier: Include everything matching include, but further filter that with those specified in exclude.

For my brain, the AND is easier to comprehend than the OR.

But, I believe the reasoning for that choice was that users are more likely to provide exclusion filters for what they don't want, rather than providing inclusion filters for everything that they want. Once an exclude filter is provided, people would mostly want to selectively include a few out of those.

But, I believe the reasoning for that choice was that users are more likely to provide exclusion filters for what they don't want, rather than providing inclusion filters for everything that they want. Once an exclude filter is provided, people would mostly want to selectively include a few out of those.

If so, the doc must highlight it and sounds natural.

One way to do so might be to a layman AND ALSO as a substitute for the formal OR.
As done with a + in the following sentence:

Include everything that is not excluded + what an be selectively included from the excluded list

Are retained all the log types that are not excluded and also those explicitly included.

docs/src/extend/log-management.md

rina23q · 2025-11-03T16:23:13Z

docs/src/extend/log-management.md

+### Configuration Format
+
+Use the `[[plugins.<plugin-name>]]` sections to define filters.
+Both `include` and `exclude` support [glob patterns](https://en.wikipedia.org/wiki/Glob_(programming)).


Note that we are using glob patterns for file paths in config/log management. However, I also agree with using regex patterns for filtering.

docs/src/extend/log-management.md

crates/extensions/tedge_log_manager/src/config.rs

didier-wenzek

Approved

This PR has to be rebased on top of #3846, to see the system tests fixed.

rina23q

Looks good. I just have minor system test improvement suggestions.

rina23q · 2025-11-06T22:40:16Z

tests/RobotFramework/tests/cumulocity/log/log_operation_plugins.robot

 Library             ThinEdgeIO

-Suite Setup         Custom Setup
+Test Setup          Custom Setup


Since you changed Suite Setup to Test Setup, I think Set Suite Variable should also be changed to Set Test Variable in the Custom Setup.

Custom Setup ${DEVICE_SN}= Setup Set Suite Variable $DEVICE_SN # this line

Resolved in the squashed commit bd7c2a9

rina23q · 2025-11-06T22:44:08Z

tests/RobotFramework/tests/cumulocity/log/log_operation_plugins.robot

+    # Clean up any existing plugins for test isolation
+    Execute Command    rm /usr/share/tedge/log-plugins/file
+    Execute Command    rm /usr/share/tedge/log-plugins/journald
+


Why dmsg is not removed in this test?

Execute Command rm /usr/share/tedge/log-plugins/dmesg

and if it's forgotten, probably it's better to create a key, like Cleanup existing plugins since these removals repeat in several tests.

Resolved in the squashed commit bd7c2a9

Allow users to filter log types reported by log plugins using include and exclude patterns in the plugin configuration.

albinsuresh temporarily deployed to Test Pull Request October 29, 2025 14:50 — with GitHub Actions Inactive

albinsuresh had a problem deploying to Test Auto October 29, 2025 16:18 — with GitHub Actions Failure

albinsuresh force-pushed the feat/filter-config-for-log-types branch from 2db56ee to 0bba586 Compare October 31, 2025 07:08

albinsuresh temporarily deployed to Test Pull Request October 31, 2025 07:08 — with GitHub Actions Inactive

albinsuresh had a problem deploying to Test Auto October 31, 2025 07:18 — with GitHub Actions Failure

albinsuresh force-pushed the feat/filter-config-for-log-types branch from 0bba586 to 53925a2 Compare October 31, 2025 08:31

albinsuresh temporarily deployed to Test Pull Request October 31, 2025 08:31 — with GitHub Actions Inactive

albinsuresh had a problem deploying to Test Auto October 31, 2025 08:42 — with GitHub Actions Failure

albinsuresh marked this pull request as ready for review October 31, 2025 09:20

albinsuresh requested review from a team, didier-wenzek, jarhodes314, reubenmiller and rina23q as code owners October 31, 2025 09:20

didier-wenzek reviewed Oct 31, 2025

View reviewed changes

reubenmiller reviewed Nov 3, 2025

View reviewed changes

jarhodes314 reviewed Nov 3, 2025

View reviewed changes

rina23q reviewed Nov 3, 2025

View reviewed changes

reubenmiller added theme:troubleshooting Theme: Troubleshooting and remote control skip-release-notes Don't include the ticket in the auto generated release notes labels Nov 5, 2025

albinsuresh temporarily deployed to Test Pull Request November 5, 2025 14:00 — with GitHub Actions Inactive

albinsuresh had a problem deploying to Test Auto November 5, 2025 14:10 — with GitHub Actions Failure

albinsuresh temporarily deployed to Test Pull Request November 6, 2025 10:21 — with GitHub Actions Inactive

albinsuresh had a problem deploying to Test Auto November 6, 2025 10:31 — with GitHub Actions Failure

albinsuresh requested a review from didier-wenzek November 6, 2025 11:01

albinsuresh requested review from jarhodes314, reubenmiller and rina23q November 6, 2025 11:01

didier-wenzek approved these changes Nov 6, 2025

View reviewed changes

albinsuresh temporarily deployed to Test Pull Request November 6, 2025 16:00 — with GitHub Actions Inactive

albinsuresh had a problem deploying to Test Auto November 6, 2025 16:12 — with GitHub Actions Error

rina23q approved these changes Nov 6, 2025

View reviewed changes

albinsuresh force-pushed the feat/filter-config-for-log-types branch from 34480cf to 9d4bbb6 Compare November 7, 2025 07:03

albinsuresh had a problem deploying to Test Pull Request November 7, 2025 07:03 — with GitHub Actions Failure

feat: filter log types using include/exclude patterns

bd7c2a9

Allow users to filter log types reported by log plugins using include and exclude patterns in the plugin configuration.

albinsuresh force-pushed the feat/filter-config-for-log-types branch from 9d4bbb6 to bd7c2a9 Compare November 7, 2025 07:19

albinsuresh temporarily deployed to Test Pull Request November 7, 2025 07:19 — with GitHub Actions Inactive

albinsuresh had a problem deploying to Test Auto November 7, 2025 07:32 — with GitHub Actions Failure

albinsuresh temporarily deployed to Test Auto November 7, 2025 09:19 — with GitHub Actions Inactive

albinsuresh added this pull request to the merge queue Nov 7, 2025

Merged via the queue into thin-edge:main with commit dab6cca Nov 7, 2025
51 of 52 checks passed

albinsuresh deleted the feat/filter-config-for-log-types branch November 10, 2025 06:23

		pub fn from_file(path: &std::path::Path) -> Self {
		match std::fs::read_to_string(path) {

feat: filter log types using include/exclude patterns #3838

feat: filter log types using include/exclude patterns #3838

Uh oh!

Conversation

albinsuresh commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Types of changes

Paste Link to the issue

Checklist

Further comments

Uh oh!

codecov bot commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Robot Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

albinsuresh Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

albinsuresh Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reubenmiller Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

albinsuresh Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

albinsuresh Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reubenmiller Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jarhodes314 Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

albinsuresh commented Oct 29, 2025 •

edited

Loading

codecov bot commented Oct 29, 2025 •

edited

Loading

github-actions bot commented Oct 29, 2025 •

edited

Loading

albinsuresh Nov 6, 2025 •

edited

Loading

albinsuresh Nov 7, 2025 •

edited

Loading

reubenmiller Nov 3, 2025 •

edited

Loading

albinsuresh Nov 4, 2025 •

edited

Loading

albinsuresh Nov 5, 2025 •

edited

Loading

reubenmiller Nov 5, 2025 •

edited

Loading

jarhodes314 Nov 3, 2025 •

edited

Loading

albinsuresh Nov 4, 2025 •

edited

Loading

didier-wenzek Nov 5, 2025 •

edited

Loading

albinsuresh Nov 6, 2025 •

edited

Loading

didier-wenzek left a comment •

edited

Loading