Skip to content

fix(collector): preserve jmb39x-q device type and accept usable SMART JSON #663

Closed

Description

@Starosdev

Summary

Scrutiny cannot reliably monitor QNAP TR-004 disks in Individual mode when smartmontools uses the jmb39x-q,N backend.

Source

Upstream discussion: AnalogJ#1028

Problem

Two collector behaviors prevent usable SMART data:

  1. A configured device type such as jmb39x-q,0 is split into separate arguments instead of being passed as one --device jmb39x-q,0 value.
  2. The collector rejects otherwise complete SMART JSON when smartctl returns exit_status = 4 for the non-fatal warning Read Device Statistics page 0x00 failed.

Expected behavior

  • Preserve jmb39x-q,N as one smartctl device-type argument for information and SMART collection commands.
  • Parse and store valid SMART data when the JSON contains usable identity and metrics despite a non-zero warning status.
  • Continue rejecting genuinely unusable or fatal smartctl output.

Acceptance criteria

  • Add regression coverage for comma-containing device types such as jmb39x-q,0.
  • Add regression coverage for valid SMART JSON with non-zero warning exit status.
  • Verify QNAP TR-004 collection produces device identity and SMART metrics instead of No Data.
  • Document any required QNAP collector configuration.

Activity

  1. moved this to Bug Backlog in Release Scheduleon Jul 17, 2026
  2. Starosdev commented on Jul 28, 2026

    @Starosdev
    OwnerAuthor

    Fixed in #677.

    Both reported behaviors were real, and the second one turned out to be the more damaging of the two.

    Device type splitting. The argument construction was never at fault. All three --device call sites already appended the type as a single element, and the shell layer uses exec.CommandContext with no re-tokenization. The split happened earlier, at config parse time: viper installs a decode hook that splits a scalar string on commas when the destination is a slice, so type: 'jmb39x-q,0' decoded to the two bogus types jmb39x-q and 0. A comma is part of the smartctl device-type grammar, not a separator, so that hook is now overridden for the devices key. Both the scalar form and the list form now preserve the type intact.

    Non-zero warning exit status. SmartCtlInfo masked the exit code with 0xBF, but the tolerant branch logged "Continuing." and then fell straight into a pre-existing return err. It was dead code, so no non-zero smartctl exit had ever survived detection. Detection now uses the same rule the collection path and the server-side upload validation already applied: only exit bits 0x01 and 0x02 mean the payload cannot be trusted, because every other bit describes a condition of the disk rather than of the output. Empty or identity-less output is still rejected.

    Worth noting for anyone who worked around the first bug with the list form type: ['jmb39x-q,0']: you were still affected. Without the second fix the drive registered with no serial and therefore no WWN, and since every metrics query filters on the device_wwn tag, that empty WWN is what actually produced No Data.

    Acceptance criteria:

    • Regression coverage for comma-containing device types such as jmb39x-q,0 - config parsing in scalar and list form, plus --device argv assertions on all three smartctl call sites.
    • Regression coverage for valid SMART JSON with non-zero warning exit status - accepted on exit 4 with identity populated, rejected on exit 2 and on empty output.
    • Verify QNAP TR-004 collection produces device identity and SMART metrics - not verified on real hardware, see below.
    • Document required QNAP collector configuration - new JMicron JMB39x section in docs/TROUBLESHOOTING_DEVICE_COLLECTOR.md, plus an example in example.collector.yaml.

    Two things that would benefit from a hand on the actual hardware:

    1. The end-to-end criterion is unverified. If you have a TR-004, running the collector with COLLECTOR_DEBUG=true and confirming the logged command line shows --device jmb39x-q,0 as one argument, and that the disks report model/serial/capacity, would close it out.
    2. The detect test fixture is modeled on a standard SAT --info --json response rather than captured from a real jmb39x-q device. Sanitized real output from smartctl --info --json -d jmb39x-q,0 /dev/sdX would make the regression test stronger. Happy to swap it in.

    One upgrade note: a drive affected by either bug was registered through the metadata-less identity fallback. Once its model, serial and WWN can be read it hashes to a new device id and shows up as a new device, leaving the old No Data entry behind. Delete the stale entry; its history is empty and cannot be merged usefully.

  3. added a commit that references this issue on Jul 28, 2026
    967bbaa
  4. added a commit that references this issue on Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcollectorCollector component

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions