tests: tag features collected from debug logs #15091
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thu Mar 27 14:32:18 UTC 2025 Failures:Preparing:
Executing:
Restoring:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15091 +/- ##
=========================================
Coverage ? 0
=========================================
Files ? 0
Lines ? 0
Branches ? 0
=========================================
Hits ? 0
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
maykathm
force-pushed
the
SNAPDENG-34442-collect-logs-from-tasks
branch
from
2b75f48 to
218f9ec
Compare
bboozzoo
reviewed
Mar 13, 2025
tests/lib/nested.sh
Outdated
| if [ -n "$TAG_FEATURES" ]; then | ||
| # If feature tagging is enabled, then we need to enable debug logging | ||
| remote.exec "sudo mkdir -p /etc/systemd/system/snapd.service.d" | ||
| remote.exec "echo -e '[Service]\nEnvironment=SNAPD_DEBUG_HTTP=7 SNAPD_DEBUG=1 SNAPPY_TESTING=1' | sudo tee /etc/systemd/system/snapd.service.d/local.conf" |
Contributor
There was a problem hiding this comment.
Suggested change
| remote.exec "echo -e '[Service]\nEnvironment=SNAPD_DEBUG_HTTP=7 SNAPD_DEBUG=1 SNAPPY_TESTING=1' | sudo tee /etc/systemd/system/snapd.service.d/local.conf" | |
| remote.exec "printf '[Service]\nEnvironment=SNAPD_DEBUG_HTTP=7 SNAPD_DEBUG=1 SNAPPY_TESTING=1\n' | sudo tee /etc/systemd/system/snapd.service.d/99-feature-tags.conf" |
aren't nested tests already doing this?
Contributor
Author
There was a problem hiding this comment.
I updated with your suggestion.
As to your question, at least some nested tests seem to not have debug logging enabled and without this addition, did not produce debug logs. If that is an error and nested tests should have that enabled, I can always shift the debug logging addition to outside the feature tagging check.
maykathm
force-pushed
the
SNAPDENG-34442-collect-logs-from-tasks
branch
from
5bf824f to
975119e
Compare
Member
andrewphelpsj
left a comment
andrewphelpsj
left a comment
There was a problem hiding this comment.
Thank you! Talked with Katie about some potential simplifications to journal-analyzer.py that leverages our expectation of structured logs from snapd, rather than using regular expressions to extract them.
maykathm
force-pushed
the
SNAPDENG-34442-collect-logs-from-tasks
branch
from
d394931 to
7f6d860
Compare
spread.yaml
Outdated
| "$TESTSLIB"/prepare-restore.sh --prepare-suite | ||
| prepare-each: | | ||
| "$TESTSLIB"/prepare-restore.sh --prepare-suite-each | ||
| "$TESTSLIB"/analyze-features.sh --before-non-nested-task |
Collaborator
There was a problem hiding this comment.
I think this could be the last step of the prepare-suite-each
Collaborator
There was a problem hiding this comment.
Still not sure if we should tag/find the whole test execution, including prepare and restore
Contributor
Author
There was a problem hiding this comment.
Initially we said that it's ok to tag all three steps. Probably eventually we will want to only tag execution.
maykathm
force-pushed
the
SNAPDENG-34442-collect-logs-from-tasks
branch
from
b5d6380 to
835c4bb
Compare
maykathm
force-pushed
the
SNAPDENG-34442-collect-logs-from-tasks
branch
from
835c4bb to
b87ce8e
Compare
bboozzoo
reviewed
Mar 26, 2025
Contributor
bboozzoo
left a comment
bboozzoo
left a comment
There was a problem hiding this comment.
Some minor things, but overall looks ok
| try: | ||
| line_json = json.loads(line) | ||
| for feature_class in feature_classes: | ||
| feature_class.maybe_add_feature(feature_dict, line_json, state_json) |
Contributor
There was a problem hiding this comment.
I guess later on new classes with add feature depending on contents of the log entry?
Contributor
Author
There was a problem hiding this comment.
Yes exactly. They would check keys and values and see if the log entry is relevant to them. If it's relevant, then they would grab the information they need and, if necessary, query the state.json for any extra info.
|
|
||
| restore_suite_each() { | ||
| if not tests.nested is-nested; then | ||
| "$TESTSLIB"/collect-features.sh --after-non-nested-task |
Collaborator
There was a problem hiding this comment.
we should do this just when SPREAD_TAG_FEATURES is set
Contributor
Author
There was a problem hiding this comment.
The collect-features.sh script checks if TAG_FEATURES is set before actually calling the function. I thought that was easier than checking in all the places the script is called. If you think the check should be outside, I can move it outside
bboozzoo
approved these changes
Mar 27, 2025
tests/lib/tools/feature_extractor.py
Outdated
|
|
||
| @staticmethod | ||
| def maybe_add_feature(feature_dict, json_entry, state_json): | ||
| def maybe_add_feature(feature_dict: dict, json_entry: dict, state_json: dict): |
Contributor
There was a problem hiding this comment.
you'll need to bring in typing.Any
Suggested change
| def maybe_add_feature(feature_dict: dict, json_entry: dict, state_json: dict): | |
| def maybe_add_feature(feature_dict: dict[str, list[Any]], json_entry: dict[str, Any], state_json: dict[str, Any]): |
tests/lib/tools/feature_extractor.py
Outdated
| parser.add_argument('-o', '--output', help='Output file', required=True) | ||
| parser.add_argument( | ||
| '-f', '--features', help='Features to extract from journal in a comma-separated list {all}', required=True) | ||
| '-f', '--features', help='Features to extract from journal {all}', nargs='+') |
Contributor
There was a problem hiding this comment.
since we're collecting them now, maybe --feature ?
Suggested change
| '-f', '--features', help='Features to extract from journal {all}', nargs='+') | |
| '-f', '--features', help='Features to extract from journal {all}, can be repeated multiple times', nargs='+') |
maykathm
added a commit
to maykathm/snapd
that referenced
this pull request
Mar 28, 2025
* tests: add feature analyzer using spread * tests: removed unnecessary VM removal * tests: put back xz-utils removal given the switch to gzip sergiocazzolato/spread#6 * tests: spelling correction and small optimizations * tests: changed forward slash replacement to backslash * tests: address review comments and add optional cursor input to journal-analyzer * tests: use json instead of regex for feature extraction * tests: minor corrections and use cursor instead of timestamp * tests: only copy journal and state.json during restore * tests: grep for keyword in logs and minor fixes to python * tests: expand dict type hints and make argument name and help more precise
maykathm
added a commit
to maykathm/snapd
that referenced
this pull request
Apr 8, 2025
* tests: add feature analyzer using spread * tests: removed unnecessary VM removal * tests: put back xz-utils removal given the switch to gzip sergiocazzolato/spread#6 * tests: spelling correction and small optimizations * tests: changed forward slash replacement to backslash * tests: address review comments and add optional cursor input to journal-analyzer * tests: use json instead of regex for feature extraction * tests: minor corrections and use cursor instead of timestamp * tests: only copy journal and state.json during restore * tests: grep for keyword in logs and minor fixes to python * tests: expand dict type hints and make argument name and help more precise
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This introduces the ability to collect and process logs during spread runs and extract features of interest.
It requires a version of spread that supports both project-level artifacts and a switch to use gzip instead of xz to be able to download the data from core.
The following tests are incompatible with the feature tagging and explained in the jira ticket:
Currently, the only "feature" available is a fake features called "all" that will match all content of all lines. It will get removed in favor of real features. Note that currently it will obtain nothing since the log entries are not in json format.
As an example, here's the command to get the fake feature for the
tests/main/acktest:After running the command, one would then find in the specified feature-artifacts folder: