Skip to content

Comments

Add NetClassByIface#376

Merged
SuperQ merged 1 commit intoprometheus:masterfrom
digineo:netclass-by-iface
Jun 23, 2021
Merged

Add NetClassByIface#376
SuperQ merged 1 commit intoprometheus:masterfrom
digineo:netclass-by-iface

Conversation

@corny
Copy link
Contributor

@corny corny commented Apr 11, 2021

See also: prometheus/node_exporter#1915

Do we need to keep the NetClass() method?

See also: prometheus/node_exporter#1915

Signed-off-by: Julian Kornberger <jk+github@digineo.de>
@corny corny force-pushed the netclass-by-iface branch from 89bc4e6 to 8e91b32 Compare April 11, 2021 08:57
@jan--f
Copy link

jan--f commented Apr 19, 2021

Sorry I only saw this after opening #378. In any case it's a different approach, would appreciate if you would take a look. It should solve #1915 too.

@discordianfish
Copy link
Member

Hi,

can you both outline how the changes to node-exporter will look like? That would help figuring out the best way to go

@corny
Copy link
Contributor Author

corny commented Apr 20, 2021

I prefer to keep out the matching logic from the procfs package and to have only two methods:

  • one method for fetching the list of all devices - just the device names/paths
  • another method to fetch the info of a single device

@discordianfish
Copy link
Member

@corny But there is no function for just listing the devices, right?
But I tend to agree that regex based filtering is something that should be done in the consumer of this lib (e.g node-exporter).
@SuperQ wdyt?

@jan--f
Copy link

jan--f commented Apr 29, 2021

@corny But there is no function for just listing the devices, right?

There is NetClassDevices().

But I tend to agree that regex based filtering is something that should be done in the consumer of this lib (e.g node-exporter).
@SuperQ wdyt?

Fwiw this sound good to me. I'd have the node_exporter change ready to go here: https://github.com/prometheus/node_exporter/compare/master...jan--f:netclass-filter-before-parsing?expand=1

Copy link
Member

@discordianfish discordianfish left a comment

Choose a reason for hiding this comment

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

AH gotcha. Okay if we all agree on that, let's go with this one. LGTM

@jan--f
Copy link

jan--f commented May 25, 2021

@discordianfish is anything blocking a merge here? I would love to finish up prometheus/node_exporter#2033 and ship that soon.

@discordianfish
Copy link
Member

Nothing on my side but still needs a second LGTM (from @SuperQ or @pgier)

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM

@SuperQ SuperQ merged commit 6de0743 into prometheus:master Jun 23, 2021
jan--f added a commit to jan--f/node_exporter that referenced this pull request Jun 28, 2021
We should filter excluded interfaces before parsing the interface
details.
This change is based on prometheus/procfs#376

Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
jan--f added a commit to jan--f/node_exporter that referenced this pull request Jul 16, 2021
We should filter excluded interfaces before parsing the interface
details.
This change is based on prometheus/procfs#376

Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
(cherry picked from commit e656b79)
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/node_exporter that referenced this pull request Jul 20, 2021
We should filter excluded interfaces before parsing the interface
details.
This change is based on prometheus/procfs#376

Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
(cherry picked from commit e656b79)
remijouannet pushed a commit to remijouannet/procfs that referenced this pull request Oct 20, 2022
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this pull request Apr 9, 2024
We should filter excluded interfaces before parsing the interface
details.
This change is based on prometheus/procfs#376

Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this pull request Apr 9, 2024
We should filter excluded interfaces before parsing the interface
details.
This change is based on prometheus/procfs#376

Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants