aya-log-common: support logging byte slices #585
Merged
Conversation
This file contains 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
✅ Deploy Preview for aya-rs-docs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
alessandrod
reviewed
Apr 19, 2023
tamird
changed the title
alessandrod
reviewed
Apr 27, 2023
There was a problem hiding this comment.
Looks great! Could you add an integration test for the new hex formatter? Somewhere here https://github.com/aya-rs/aya/blob/main/test/integration-ebpf/src/log.rs
|
Added! |
Remove mem::forget::<HashMap>() calls in tests which fail to compile when
HashMap is provided by hashbrown:
info: running `cargo check --all-targets --no-default-features` on aya-obj (11/23)
Checking aya-obj v0.1.0 (/home/tamird/src/aya/aya-obj)
error[E0505]: cannot move out of `map` because it is borrowed
--> aya-obj/src/relocation.rs:594:21
|
578 | let map = fake_legacy_map(1);
| --- binding `map` declared here
579 | let maps_by_symbol = HashMap::from([(1, ("test_map", Some(1), &map))]);
| ---- borrow of `map` occurs here
...
594 | mem::forget(map);
| ^^^ move out of `map` occurs here
595 | }
| - borrow might be used here, when `maps_by_symbol` is dropped and runs the destructor for type `hashbrown::HashMap<usize, (&str, Option<i32>, &maps::Map)>`
error[E0505]: cannot move out of `map_1` because it is borrowed
--> aya-obj/src/relocation.rs:655:21
|
632 | let map_1 = fake_legacy_map(1);
| ----- binding `map_1` declared here
...
635 | (1, ("test_map_1", Some(1), &map_1)),
| ------ borrow of `map_1` occurs here
...
655 | mem::forget(map_1);
| ^^^^^ move out of `map_1` occurs here
656 | mem::forget(map_2);
657 | }
| - borrow might be used here, when `maps_by_symbol` is dropped and runs the destructor for type `hashbrown::HashMap<usize, (&str, Option<i32>, &maps::Map)>`
error[E0505]: cannot move out of `map_2` because it is borrowed
--> aya-obj/src/relocation.rs:656:21
|
633 | let map_2 = fake_legacy_map(2);
| ----- binding `map_2` declared here
...
636 | (2, ("test_map_2", Some(2), &map_2)),
| ------ borrow of `map_2` occurs here
...
656 | mem::forget(map_2);
| ^^^^^ move out of `map_2` occurs here
657 | }
| - borrow might be used here, when `maps_by_symbol` is dropped and runs the destructor for type `hashbrown::HashMap<usize, (&str, Option<i32>, &maps::Map)>`
error[E0505]: cannot move out of `map` because it is borrowed
--> aya-obj/src/relocation.rs:694:21
|
678 | let map = fake_btf_map(1);
| --- binding `map` declared here
679 | let maps_by_symbol = HashMap::from([(1, ("test_map", Some(1), &map))]);
| ---- borrow of `map` occurs here
...
694 | mem::forget(map);
| ^^^ move out of `map` occurs here
695 | }
| - borrow might be used here, when `maps_by_symbol` is dropped and runs the destructor for type `hashbrown::HashMap<usize, (&str, Option<i32>, &maps::Map)>`
error[E0505]: cannot move out of `map_1` because it is borrowed
--> aya-obj/src/relocation.rs:755:21
|
732 | let map_1 = fake_btf_map(1);
| ----- binding `map_1` declared here
...
735 | (1, ("test_map_1", Some(1), &map_1)),
| ------ borrow of `map_1` occurs here
...
755 | mem::forget(map_1);
| ^^^^^ move out of `map_1` occurs here
756 | mem::forget(map_2);
757 | }
| - borrow might be used here, when `maps_by_symbol` is dropped and runs the destructor for type `hashbrown::HashMap<usize, (&str, Option<i32>, &maps::Map)>`
error[E0505]: cannot move out of `map_2` because it is borrowed
--> aya-obj/src/relocation.rs:756:21
|
733 | let map_2 = fake_btf_map(2);
| ----- binding `map_2` declared here
...
736 | (2, ("test_map_2", Some(2), &map_2)),
| ------ borrow of `map_2` occurs here
...
756 | mem::forget(map_2);
| ^^^^^ move out of `map_2` occurs here
757 | }
| - borrow might be used here, when `maps_by_symbol` is dropped and runs the destructor for type `hashbrown::HashMap<usize, (&str, Option<i32>, &maps::Map)>`
For more information about this error, try `rustc --explain E0505`.
error: could not compile `aya-obj` due to 6 previous errors
warning: build failed, waiting for other jobs to finish...
error: process didn't exit successfully: `/home/tamird/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo check --all-targets --manifest-path aya-obj/Cargo.toml --no-default-features` (exit status: 101)
Previously `struct TagLenValue` was defined in both aya-log and aya-log-common where the former implemented reading and the latter writing; the reading logic doesn't need the struct, so remove it.
Previously any old `write` method could be selected.
|
@alessandrod ping. |
alessandrod
reviewed
May 5, 2023
|
Not sure what to make of these build failures; they don't appear to be related to this PR. |
I'll kick CI once new cargo nightly gets uploaded rust-lang/cargo#12088 |
Could you give it an attempt? The fix should be available starting from toolchain nightly-2023-05-05. |
Seems to work, thanks for the quick fix :) |
- Replace all `#[repr(usize)]` with `#[repr(u8)]`; this saves 3*(sizeof(word)-1) bytes per log message. - Encode payload length as u16 rather than usize; this saves sizeof(word)-1 bytes per log message. This is safe because the maximum size of a log message is less than (1 << 16 - 1). This changes `level` to a require field in every log message. It was already always present, but was treated as optional when reading.
This allows logging values backed by generators.
These only support LowerHex and UpperHex hints for now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
These only support LowerHex and UpperHex hints for now.