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

Issue #111: Fix LinuxIpNetwork #157

Merged
merged 8 commits into from
Jan 17, 2025
Merged

Conversation

MedMaalej
Copy link
Contributor

  • Refactor network interface detection logic in networkDiscovery.awk
  • Refactor network interface detection logic in networkCollect.awk
  • Update LinuxNetwork-header

* Refactor network interface detection logic in networkDiscovery.awk
* Refactor network interface detection logic in networkCollect.awk
* Update LinuxNetwork-header
@MedMaalej MedMaalej added the bug Something isn't working label Jan 9, 2025
@MedMaalej MedMaalej self-assigned this Jan 9, 2025
@bertysentry
Copy link
Member

While we're there, we should merge all Linux network connectors in the same directory!

* Refactor network interface detection logic in networkDiscovery.awk
* Refactor network interface detection logic in networkCollect.awk
* Update LinuxNetwork-header
@NassimBtk NassimBtk marked this pull request as draft January 14, 2025 08:37
* Refactor network interface detection logic in networkDiscovery.awk for LinuxIfconfigNetwork
* Refactor network interface detection logic in networkCollect.awk for LinuxIfConfigNetwork
* Update Hardware folder path reference in LinuxIpNetwork and LinuxIfConfigNetwork
* Merge LinuxIpNetwork, LinuxIfConfigNetwork and LinuxNetwork-header under LinuxNetwork folder
* Update interface name detection regex in LinuxIfConfigNetwork
@@ -19,7 +15,7 @@ $1 ~ /^eth[0-9][0-9]*:?|^vmnic[0-9][0-9]*:?|^em[0-9]*:?|^[Pp][0-9][0-9]*[Pp][0-9
}
}

/ +inet addr:[0-9]+/ {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should still support addr:

@@ -8,7 +8,7 @@ BEGIN {
}

# ifconfig
$1 ~ /^eth[0-9][0-9]*:?|^vmnic[0-9][0-9]*:?|^em[0-9]*:?|^[Pp][0-9][0-9]*[Pp][0-9][0-9]*:?|^en[os][0-9]*:?|^enp[0-9]*s[0-9]*:?/ {
$1 ~ /:$/ && $2 ~ /flags/ {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure flags is always available.

@MedMaalej
Copy link
Contributor Author

I tested this PR using the hosts babbage and mm-docker-01 (mm-docker-01 uses Linux Debian and babbage uses CentOs)

mm-docker-01 (Debian)

Linux commands check

ip a result

1

Please note that here only en192 is a physical network interface ℹ️

ls -l /sys/class/net result

Capture d'écran 2025-01-09 160749

This command shows both physical and virtual network interfaces ℹ️

### ls -l /sys/class/net | awk $0 !~ /virtual/ && $0 !~ /total/ {print $9} result

ens192

This command shows only physical network interfaces names ℹ️

Prometheus metrics check

The following are the expected metrics of the LinuxIpNetwork connector

        hw.network.packets{direction="receive"}
        hw.network.packets{direction="transmit"}
        hw.errors{hw.type="network"}
        hw.network.io{direction="receive"}
        hw.network.io{direction="transmit"}
        hw.network.up: legacyLinkStatus
        hw.network.bandwidth.limit
        hw.network.full_duplex

Capture d'écran 2025-01-09 152657
Capture d'écran 2025-01-09 152642
Capture d'écran 2025-01-09 152629
Capture d'écran 2025-01-09 152615
Capture d'écran 2025-01-09 152555
Capture d'écran 2025-01-09 152542
Capture d'écran 2025-01-09 152525

All the metrics were reported ✅

Metricshub logs check

The expected attributes of the LinuxIpNetwork connector are:

        id
        physical_address
        physical_address_type
        logical_address
        logical_address_type
        hw.parent.type
        name

2

The expected number of monitors is 3 since we have only one physical network interface ens192 plus the connector and the host monitors. ✅

Capture d'écran 2025-01-09 164231

babbage (CentOs)

Linux commands check

ip a result

3

Please note that here only eno1 and eno2 are physical network interfaces ℹ️

ls -l /sys/class/net result

Capture d'écran 2025-01-09 161043

This command shows both physical and virtual network interfaces ℹ️

### ls -l /sys/class/net | awk $0 !~ /virtual/ && $0 !~ /total/ {print $9} result

eno1
eno2

This command shows only physical network interfaces names ℹ️

Metricshub logs check

The attributes were correctly retrieved ✅
4
5

The number of monitors is correct (4): eno1, eno2, the connector monitor and the host monitor ✅
Capture d'écran 2025-01-09 164844

rt-spectrum-ubuntu (Ubuntu)

This test covers both ifConfig and ip a commands.

Linux commands check

6
7

Metricshub logs check

5 monitors were collected (Only UP physical network interfaces were reported)

LinuxIPNetwork_network_ens160(Physical interface detected with Ip)
LinuxIfConfigNetwork_network_ens160 (Physical interface detected with IfConfig)
connector_LinuxIPNetwork (Ip connector)
connector_LinuxIfConfigNetwork (Ifconfig connector)
10.0.220.96 (host)

@MedMaalej MedMaalej marked this pull request as ready for review January 16, 2025 13:32
#ifconfig
$1 ~ /^eth[0-9][0-9]*:?|^vmnic[0-9][0-9]*:?|^em[0-9]*:?|^[Pp][0-9][0-9]*[Pp][0-9][0-9]*:?|^en[os][0-9]*:?|^enp[0-9]*s[0-9]*:?/ {
/^./ && ($2 ~ /flags/ || $2 ~ /Link/ && $3 ~ /encap/) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first regex is probably unnecessary:

Suggested change
/^./ && ($2 ~ /flags/ || $2 ~ /Link/ && $3 ~ /encap/) {
($2 ~ /flags/ || $2 ~ /Link/ && $3 ~ /encap/) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first regex is probably unnecessary:

@bertysentry Yes, you are right. It's an unnecessary. I removed it and checked the result again. It works.

@@ -1,6 +1,6 @@
---
extends:
- ../Hardware/Hardware
- ../../Hardware/Hardware
Copy link
Member

@bertysentry bertysentry Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lowercase or uppercase h for the directory name?

Sorry, don't bother! 😅

@NassimBtk NassimBtk linked an issue Jan 16, 2025 that may be closed by this pull request
@NassimBtk NassimBtk added this to the 1.0.08 milestone Jan 16, 2025
* Update interface name detection regex in LinuxIfConfigNetwork
@MedMaalej MedMaalej requested a review from bertysentry January 17, 2025 08:30
@NassimBtk NassimBtk merged commit ce367f0 into main Jan 17, 2025
1 check passed
@NassimBtk NassimBtk deleted the feature/issue-111-Fix-LinuxIpNetwork branch January 17, 2025 08:40
@MedMaalej MedMaalej added the developer_tested The changes are tested by the developer label Jan 17, 2025
@NassimBtk NassimBtk removed this from the 1.0.08 milestone Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working developer_tested The changes are tested by the developer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LinuxIPNetwork: Fails to monitor some Ethernet interfaces
3 participants