Skip to content
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

Some tests fail with 0.33.0 (sources::host_metrics::cgroups::tests::generates_cgroups_metrics) #18697

Closed
hashworks opened this issue Sep 27, 2023 · 10 comments · Fixed by #18698 or #20294
Closed
Labels
meta: confirmed A bug that has been reproduced or confirmed. source: host_metrics Anything `host_metrics` source related type: bug A code related bug.

Comments

@hashworks
Copy link
Contributor

hashworks commented Sep 27, 2023

A note for the community

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Problem

The test convert_config::tests::convert_all_from_dir fails with 0.33.0. Worked fine in 0.32.2.

Version

0.33.0

Debug Output

running 1 test
Converting "/build/vector/src/vector/tests/data/cmd/config/config_2.toml" config to Yaml.
Converting "/build/vector/src/vector/tests/data/cmd/config/config_3.json" config to Yaml.
test convert_config::tests::convert_all_from_dir ... FAILED

failures:

failures:
    convert_config::tests::convert_all_from_dir

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 241 filtered out; finished in 0.00s


--- TRY 4 STDERR:        vector convert_config::tests::convert_all_from_dir ---
thread 'convert_config::tests::convert_all_from_dir' panicked at 'called `Result::unwrap()` on an `Err` value: ["TOML parse error at line 8, column 1\n  |\n8 | [sources.source0]\n  | ^^^^^^^^^^^^^^^^^\nunknown variant `demo_logs`, expected one of `test_backpressure`, `test_basic`, `test_error`, `test_panic`, `test_tripwire`, `unit_test`, `unit_test_stream`\n", "unknown variant `demo_logs`, expected one of `test_backpressure`, `test_basic`, `test_error`, `test_panic`, `test_tripwire`, `unit_test`, `unit_test_stream` at line 15 column 5"]', src/convert_config.rs:271:70

Context

The Arch Linux vector PKGBUILD was used.

@hashworks hashworks added the type: bug A code related bug. label Sep 27, 2023
jszwedko added a commit that referenced this issue Sep 27, 2023
These tests require `demo_logs`, `remap`, and `console` components.

Fixes: #18697

Signed-off-by: Jesse Szwedko <jesse.szwedko@datadoghq.com>
@jszwedko
Copy link
Member

jszwedko commented Sep 27, 2023

Hi @hashworks !

Apologies for this, I was able to reproduce it. The issue is that the test depends on additional features being enabled. I opened #18698 to gate those additional tests.

If you are able to modify that PKGBUILD, I'd recommend changing:

    # Unit-Tests only, integration tests require services
    cargo nextest run \
        --workspace \
        --fail-fast \
        --test-threads num-cpus \
        --frozen \
        --release \
        --locked \
        --offline \
        --no-default-features \
        --target "${_target}"

to

    # Unit-Tests only, integration tests require services
    cargo nextest run \
        --workspace \
        --fail-fast \
        --test-threads num-cpus \
        --frozen \
        --release \
        --locked \
        --offline \
        --no-default-features \
        --features "target-${_target}"
        --target "${_target}"

This will run all of the unit tests for the components included in the given target platform, but notably still not the integration tests so there are no external dependencies. Incidentally it should avoid this failure.

@hashworks
Copy link
Contributor Author

If you are able to modify that PKGBUILD, I'd recommend changing

Done, thanks. Next error:

running 1 test
test sources::host_metrics::cgroups::tests::generates_cgroups_metrics ... FAILED

failures:

failures:
    sources::host_metrics::cgroups::tests::generates_cgroups_metrics

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 1466 filtered out; finished in 0.00s


--- TRY 4 STDERR:        vector sources::host_metrics::cgroups::tests::generates_cgroups_metrics ---
thread 'sources::host_metrics::cgroups::tests::generates_cgroups_metrics' panicked at 'assertion failed: `(left != right)`

github-merge-queue bot pushed a commit that referenced this issue Sep 27, 2023
These tests require `demo_logs`, `remap`, and `console` components.

Fixes: #18697

Signed-off-by: Jesse Szwedko <jesse.szwedko@datadoghq.com>
@jszwedko
Copy link
Member

If you are able to modify that PKGBUILD, I'd recommend changing

Done, thanks. Next error:

running 1 test
test sources::host_metrics::cgroups::tests::generates_cgroups_metrics ... FAILED

failures:

failures:
    sources::host_metrics::cgroups::tests::generates_cgroups_metrics

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 1466 filtered out; finished in 0.00s


--- TRY 4 STDERR:        vector sources::host_metrics::cgroups::tests::generates_cgroups_metrics ---
thread 'sources::host_metrics::cgroups::tests::generates_cgroups_metrics' panicked at 'assertion failed: `(left != right)`

Interesting, I see the same. We'll take a look.

@jszwedko
Copy link
Member

I'll reopen this until we resolve that test failure too.

@jszwedko jszwedko reopened this Sep 28, 2023
@hashworks hashworks changed the title The test convert_config::tests::convert_all_from_dir fails with 0.33.0 Some tests fail with 0.33.0 Sep 28, 2023
@neuronull
Copy link
Contributor

Just dropping a note that I briefly looked at this but didn't have enough time to root cause it. The issue seems to be related to the gauge cgroup_memory_anon_bytes.

@neuronull
Copy link
Contributor

It also seems to have not been working in v0.24.0

@neuronull neuronull added source: host_metrics Anything `host_metrics` source related meta: confirmed A bug that has been reproduced or confirmed. labels Oct 18, 2023
@dsmith3197 dsmith3197 changed the title Some tests fail with 0.33.0 Some tests fail with 0.33.0 (sources::host_metrics::cgroups::tests::generates_cgroups_metrics) Dec 19, 2023
@dsmith3197
Copy link
Contributor

Below is the failing line.

assert_ne!(count_name(&metrics, "cgroup_memory_anon_bytes"), 0);

@hashworks
Copy link
Contributor Author

FYI, I've removed the test from the Arch Linux build for now, since I wasn't able to provide an updated version for three months due to failing checks.

@jszwedko
Copy link
Member

FYI, I've removed the test from the Arch Linux build for now, since I wasn't able to provide an updated version for three months due to failing checks.

👍 thanks for following up. I think we can leave this issue open until we are able to resolve the test failure.

@bwerthmann
Copy link
Contributor

bwerthmann commented Apr 12, 2024

This patch made the tests pass on my dev box.

Not sure why the is_root check is there, on my machine (cgroups v2) the memory is only in the root cgroup dir, there are no child dirs in the cgroup root on my machine.

diff --git a/src/sources/host_metrics/cgroups.rs b/src/sources/host_metrics/cgroups.rs
index 2f88a4f16..5f999003f 100644
--- a/src/sources/host_metrics/cgroups.rs
+++ b/src/sources/host_metrics/cgroups.rs
@@ -116,7 +116,7 @@ impl<'a> CGroupRecurser<'a> {
             if self.load_cpu {
                 self.load_cpu(&cgroup, &tags).await;
             }
-            if self.load_memory && !cgroup.is_root() {
+            if self.load_memory {
                 self.load_memory(&cgroup, &tags).await;
             }

@@ -271,10 +271,6 @@ struct CGroup {
 }

 impl CGroup {
-    fn is_root(&self) -> bool {
-        self.name == Path::new("/")
-    }
-
     fn tags(&self) -> MetricTags {
         metric_tags! {
             "cgroup" => self.name.to_string_lossy(),
@@ -599,11 +595,11 @@ mod tests {
             );
             assert_eq!(
                 count_name(&metrics, "cgroup_memory_anon_bytes"),
-                SUBDIRS.len() - 1
+                SUBDIRS.len()
             );
             assert_eq!(
                 count_name(&metrics, "cgroup_memory_file_bytes"),
-                SUBDIRS.len() - 1
+                SUBDIRS.len()
             );
         }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta: confirmed A bug that has been reproduced or confirmed. source: host_metrics Anything `host_metrics` source related type: bug A code related bug.
Projects
None yet
5 participants