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.

Sign up for GitHub

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

Jump to bottom

[NDM] Add cyberpower-pdu profile #15158

Merged
merged 3 commits into from
Aug 3, 2023
Merged

[NDM] Add cyberpower-pdu profile #15158

merged 3 commits into from
Aug 3, 2023

Conversation

FlorianVeaux
Copy link
Member

@FlorianVeaux FlorianVeaux commented Jul 4, 2023 •
edited
Loading

Add profile for cyberpower-pdu

@FlorianVeaux FlorianVeaux requested review from a team as code owners July 4, 2023 15:56
@ghost ghost added the integration/snmp label Jul 4, 2023
@github-actions
Copy link

github-actions bot commented Jul 4, 2023 •
edited
Loading

Test Results

       6 files      6 suites   29m 56s ⏱️
   375 tests 375 ✔️        0 💤 0 ❌
1 713 runs  555 ✔️ 1 158 💤 0 ❌

Results for commit aecf6d5.

♻️ This comment has been updated with latest results.

NouhaManai96
NouhaManai96 previously requested changes Jul 5, 2023
View reviewed changes
Copy link
Contributor

@NouhaManai96 NouhaManai96 left a comment

Choose a reason for hiding this comment

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

LGTM once conflicts are resolved and checks pass + a small change

@FlorianVeaux
[NDM] Add cyberpower-pdu profile
79831fc 
fix style

resort metadata.csv

metadata

metadata
@codecov
Copy link

codecov bot commented Aug 2, 2023 •
edited
Loading

Codecov Report

Merging #15158 (aecf6d5) into master (ee31b06) will decrease coverage by 0.25%.
Report is 4 commits behind head on master.
The diff coverage is 10.76%.

Flag Coverage Δ
snmp 52.92% <10.76%> (-1.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@FlorianVeaux FlorianVeaux merged commit 6c29cab into master Aug 3, 2023
@FlorianVeaux FlorianVeaux deleted the flo/cyberpower-pdu branch August 3, 2023 07:20
NouhaManai96 added a commit that referenced this pull request Aug 23, 2023
@NouhaManai96
fix mapping #15158
c4143c5
dplepage-dd
dplepage-dd reviewed Aug 23, 2023
View reviewed changes
snmp/datadog_checks/snmp/data/default_profiles/cyberpower-pdu.yaml
# 1: all_available_power_supplies_ok
# 2: power_supply_one_failed
# 3: power_supply_two_failed
# 4: power_supply_oneand_two_failed
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm unclear on what makes the commented-out block above different from the block below - why do we have to exclude the outlet bank table but not the status bank table?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh right, this comment seems wrong.
I don't remember the details that might be a mistake indeed

snmp/metadata.csv
@@ -510,6 +510,34 @@ snmp.cwoTfoStatsReservedConn,gauge,,,,"[CISCO-WAN-OPTIMIZATION-MIB] This object
snmp.cwoTfoStatsResetConn,count,,,,"[CISCO-WAN-OPTIMIZATION-MIB] This object contains total number of optimized TCP connections, which are reset since TFO was started or its statistics were last reset.",0,snmp,,
snmp.cwoTfoStatsTotalNormalClosedConn,count,,,,"[CISCO-WAN-OPTIMIZATION-MIB] This object contains total number of optimized TCP connections which were closed normally since TFO was started or its statistics were last reset.",0,snmp,,
snmp.cwoTfoStatsTotalOptConn,count,,,,"[CISCO-WAN-OPTIMIZATION-MIB] This object contains total number of TCP connections optimized since TFO was started or its statistics were last reset.",0,snmp,,
snmp.cyberpower.ePDU2BankStatusLoad,gauge,,,,"[CPS-MIB] Indicates the current draw, in tenths of Amps, of the load on the Rack PDU bank being queried.",0,snmp,,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it matter that this description is formatted differently from the others? The others are just e.g. "The peak current..." or "The power consumption..." instead of "Indicates the peak current..." or "Indicates the power consumption..."

I guess it's written that way in the MIB as well, so maybe we don't care.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah IMO we can stick to the MIB description here

snmp/metadata.csv
snmp.cyberpower.ePDUStatusOutlet,gauge,,,,"[CPS-MIB] Allows for getting of status of each Outlet of an Outlet Monitored Rack PDU. (Make 'sum by {X}' queries to count elements with the tag X.)",0,snmp,,
snmp.cyberpower.ePDUStatusPhase,gauge,,,,"[CPS-MIB] Allows for getting of status of each Rack PDU Phase (Make 'sum by {X}' queries to count elements with the tag X.)",0,snmp,,
snmp.cyberpower.envirHumidity,gauge,,,,"",0,snmp,,
snmp.cyberpower.envir_temperature,gauge,,,,"",0,snmp,,
Copy link
Contributor

Choose a reason for hiding this comment

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

These two entries are missing descriptions - the MIB descriptions are "The environment relative humidity expressed in percentage." and "The environment temperature expressed in 1/10 Fahrenheit."

snmp/datadog_checks/snmp/data/default_profiles/cyberpower-pdu.yaml
- MIB: CPS-MIB
symbol:
OID: 1.3.6.1.4.1.3808.1.1.4.2.1.0
name: cyberpower.envir_temperature
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, is there a reason we expose the fahrenheit but not the celsius? Does our UI handle unit switching?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call out, our UI can handle the unit switching but only if the metadata csv is configured with the right unit or if the user configures it. Otherwise the value is shown without transformation.

It could make sense to submit only the celsius one, or both the celsius + the fahrenheit one.

NouhaManai96 added a commit that referenced this pull request Aug 31, 2023
@NouhaManai96 @AlexandreYang
Fix SNMP profiles metric tags mappings using MIBs (#15668)
a50ecaa 
* fix mapping for apc-netbotz #15136

* for mapping for aruba-wireless-controller #15128

* fix mapping #15482

* fix mapping #15158

* fix mapping #15414

* fix mapping #15312

* fix mapping #15343

* fix mapping #15374

* fix mapping #15354

* fix mapping #15370

* fix mapping #15464

* fix mapping #15326

* fix mapping #15368

* fix mapping #15368

* fix mapping #15396

* fix mapping #15386

* fix mapping #15469

* fix mappings #15265

* fix mapping #15349

* fix mapping #15356

* fix mapping #15371

* fix mapping #15322

* fix mapping #15397

* fix mappings #15466

* fix mapping #15153

* fix mapping cisco

* fix mapping

* fix mapping hp

* fix mapping poweredge

* fix mapping f5

* update servertech: metricnames + metadata +tests

* update apc netbotz tests

* update aruba wlc tests

* update citrix netscaler sdx test

* update dell os10 tests

* update test hpe nimble

* update tests huawei

* update infinera coriant groove tests

* update tests infoblox

* update tests kyocera

* update tests raritan

* update tests sinetica

* update tests sophos

* update test zyxel

* update tests avaya

* update tests cisco ucs

* update tests huawei routers

* update tests ibm datapower

* update tests tripplite ups

* update test avaya cajun

* update test barracuda cloudgen

* update test _cisco_generic

* update test cisco nexus

* update test assert_extend_cisco_generic

* update test dell

* update test hp compaq health

* update test f5-big-ip

* update metrics names

* address reviews

* add changelog

* lint

* update changelog

* fix mib name

* address reviews

* revert to readynasos mib mapping

* address review

* remove tag

* add spacing

* Fix changelog

* update changelog

* fix changelog

* remove unnecessary ip_address formatting

---------

Co-authored-by: Alexandre Yang <alexandre.yang@datadoghq.com>
github-actions bot pushed a commit that referenced this pull request Aug 31, 2023
@NouhaManai96
Fix SNMP profiles metric tags mappings using MIBs (#15668)
f130e03 
* fix mapping for apc-netbotz #15136

* for mapping for aruba-wireless-controller #15128

* fix mapping #15482

* fix mapping #15158

* fix mapping #15414

* fix mapping #15312

* fix mapping #15343

* fix mapping #15374

* fix mapping #15354

* fix mapping #15370

* fix mapping #15464

* fix mapping #15326

* fix mapping #15368

* fix mapping #15368

* fix mapping #15396

* fix mapping #15386

* fix mapping #15469

* fix mappings #15265

* fix mapping #15349

* fix mapping #15356

* fix mapping #15371

* fix mapping #15322

* fix mapping #15397

* fix mappings #15466

* fix mapping #15153

* fix mapping cisco

* fix mapping

* fix mapping hp

* fix mapping poweredge

* fix mapping f5

* update servertech: metricnames + metadata +tests

* update apc netbotz tests

* update aruba wlc tests

* update citrix netscaler sdx test

* update dell os10 tests

* update test hpe nimble

* update tests huawei

* update infinera coriant groove tests

* update tests infoblox

* update tests kyocera

* update tests raritan

* update tests sinetica

* update tests sophos

* update test zyxel

* update tests avaya

* update tests cisco ucs

* update tests huawei routers

* update tests ibm datapower

* update tests tripplite ups

* update test avaya cajun

* update test barracuda cloudgen

* update test _cisco_generic

* update test cisco nexus

* update test assert_extend_cisco_generic

* update test dell

* update test hp compaq health

* update test f5-big-ip

* update metrics names

* address reviews

* add changelog

* lint

* update changelog

* fix mib name

* address reviews

* revert to readynasos mib mapping

* address review

* remove tag

* add spacing

* Fix changelog

* update changelog

* fix changelog

* remove unnecessary ip_address formatting

---------

Co-authored-by: Alexandre Yang <alexandre.yang@datadoghq.com> a50ecaa
iliakur pushed a commit that referenced this pull request Sep 5, 2023
@NouhaManai96 @AlexandreYang @iliakur
Fix SNMP profiles metric tags mappings using MIBs (#15668)
0858837 
* fix mapping for apc-netbotz #15136

* for mapping for aruba-wireless-controller #15128

* fix mapping #15482

* fix mapping #15158

* fix mapping #15414

* fix mapping #15312

* fix mapping #15343

* fix mapping #15374

* fix mapping #15354

* fix mapping #15370

* fix mapping #15464

* fix mapping #15326

* fix mapping #15368

* fix mapping #15368

* fix mapping #15396

* fix mapping #15386

* fix mapping #15469

* fix mappings #15265

* fix mapping #15349

* fix mapping #15356

* fix mapping #15371

* fix mapping #15322

* fix mapping #15397

* fix mappings #15466

* fix mapping #15153

* fix mapping cisco

* fix mapping

* fix mapping hp

* fix mapping poweredge

* fix mapping f5

* update servertech: metricnames + metadata +tests

* update apc netbotz tests

* update aruba wlc tests

* update citrix netscaler sdx test

* update dell os10 tests

* update test hpe nimble

* update tests huawei

* update infinera coriant groove tests

* update tests infoblox

* update tests kyocera

* update tests raritan

* update tests sinetica

* update tests sophos

* update test zyxel

* update tests avaya

* update tests cisco ucs

* update tests huawei routers

* update tests ibm datapower

* update tests tripplite ups

* update test avaya cajun

* update test barracuda cloudgen

* update test _cisco_generic

* update test cisco nexus

* update test assert_extend_cisco_generic

* update test dell

* update test hp compaq health

* update test f5-big-ip

* update metrics names

* address reviews

* add changelog

* lint

* update changelog

* fix mib name

* address reviews

* revert to readynasos mib mapping

* address review

* remove tag

* add spacing

* Fix changelog

* update changelog

* fix changelog

* remove unnecessary ip_address formatting

---------

Co-authored-by: Alexandre Yang <alexandre.yang@datadoghq.com>
lizaerem added a commit that referenced this pull request Sep 20, 2023
@AlexandreYang @NouhaManai96
@lizaerem @jhgilbert
Add assert_all_profile_metrics_and_tags_covered & fix mappings in pro…
3632e04 
…files (#15720)

* fix mapping for apc-netbotz #15136

* for mapping for aruba-wireless-controller #15128

* fix mapping #15482

* fix mapping #15158

* fix mapping #15414

* fix mapping #15312

* fix mapping #15343

* fix mapping #15374

* fix mapping #15354

* fix mapping #15370

* fix mapping #15464

* fix mapping #15326

* fix mapping #15368

* fix mapping #15368

* fix mapping #15396

* fix mapping #15386

* fix mapping #15469

* fix mappings #15265

* fix mapping #15349

* fix mapping #15356

* fix mapping #15371

* fix mapping #15322

* fix mapping #15397

* fix mappings #15466

* fix mapping #15153

* fix mapping cisco

* fix mapping

* fix mapping hp

* fix mapping poweredge

* fix mapping f5

* update servertech: metricnames + metadata +tests

* update apc netbotz tests

* update aruba wlc tests

* update citrix netscaler sdx test

* update dell os10 tests

* update test hpe nimble

* update tests huawei

* update infinera coriant groove tests

* update tests infoblox

* update tests kyocera

* update tests raritan

* update tests sinetica

* update tests sophos

* update test zyxel

* update tests avaya

* update tests cisco ucs

* update tests huawei routers

* update tests ibm datapower

* update tests tripplite ups

* update test avaya cajun

* update test barracuda cloudgen

* update test _cisco_generic

* update test cisco nexus

* update test assert_extend_cisco_generic

* update test dell

* update test hp compaq health

* update test f5-big-ip

* update metrics names

* address reviews

* add changelog

* lint

* update changelog

* fix mib name

* address reviews

* revert to readynasos mib mapping

* address review

* remove tag

* add spacing

* Fix changelog

* update changelog

* fix changelog

* remove unnecessary ip_address formatting

* Add assert_all_profile_metrics_and_tags_covered

* fix empty space

* Add assert_all_profile_metrics_and_tags_covered

* Add to test_profile__checkpoint_firewall_cpu_memory.py

* Add assertions

* better msg

* format

* not use f string

* not use f string

* Add ASSERT_ALL_PROFILE_EXCLUDED_TAGS

* fix cisco profiles tags & snmprec & test

* fix dell-rac snmprec (tag) & test

* fix _generic-entity-sensor snmprec (tags + mapping) & test

* fix alcatel-lucent-ent snmprec (tags + mapping) & test

* fix alcatel-lucent-ind (tags + mapping) & test

* rename anue-packet-broker snmprec to anue

* better test output

* fix aruba-clearpass tags & snmprec & test

* fix aruba-wc snmprec & test

* fix bluecat-server snmprec & test

* fix checkpoint snmprec & test

* fix eaton-epdu profile & test

* fix hp-h3c-switch snmprec & test

* fix huawei-routers snmprec & test

* fix infinera-coriant-groove snmprec & test

* fix juniper-variation test

* fix juniper-pulse-secure tags & snmprec & test

* fix nasuni-filer tags & snmprec & test

* fix meraki-cloud-controller device tag name

* fix netgear-readynas tag & snmprec & test

* fix riverbed-interceptor (comment scalar metric)

* fix servertech-pdu4 snmprec & test

* fix sinetica-eagle-i snmprec & test

* fix western-digital snmprec & test

* revert netgear-readynas & resolve conflict

* Update western-digital-mycloud-ex2-ultra.snmprec

fix type in snmprec for western-digital

* Update infinera-coriant-groove.snmprec

fix type in snmprec for infinera-coriant

* Update netgear-readynas.snmprec

fix type in snmprec for netgear-readynas

* update test_profiles.py

* update core tests

* let arista-switch test pass & fix it separately

* minor changes to profiles

* update _cisco-generic with the tooling

* update alcatel-lucent-ent with the tooling

* update alcatel-lucent-ind with the tooling

* update alcatel-lucent-omni-access-wlc with the tooling

* update aruba-wireless-controller with the tooling

* fix tests for cisco profiles

* update test for arista after fixing extends in profiles

* add changelog

* (turns out we cannot fix previous changelogs)

* fix mapping #15322

* rebase on master

* update test barracuda cloudgen

* address reviews

* address reviews

* revert to readynasos mib mapping

* remove tag

* Add assert_all_profile_metrics_and_tags_covered

* fix empty space

* Add assert_all_profile_metrics_and_tags_covered

* Add to test_profile__checkpoint_firewall_cpu_memory.py

* Add assertions

* better msg

* format

* not use f string

* not use f string

* Add ASSERT_ALL_PROFILE_EXCLUDED_TAGS

* fix cisco profiles tags & snmprec & test

* fix dell-rac snmprec (tag) & test

* fix _generic-entity-sensor snmprec (tags + mapping) & test

* fix alcatel-lucent-ent snmprec (tags + mapping) & test

* fix alcatel-lucent-ind (tags + mapping) & test

* rename anue-packet-broker snmprec to anue

* better test output

* fix aruba-clearpass tags & snmprec & test

* fix aruba-wc snmprec & test

* fix bluecat-server snmprec & test

* fix checkpoint snmprec & test

* fix eaton-epdu profile & test

* fix hp-h3c-switch snmprec & test

* fix huawei-routers snmprec & test

* fix infinera-coriant-groove snmprec & test

* fix juniper-variation test

* fix juniper-pulse-secure tags & snmprec & test

* fix nasuni-filer tags & snmprec & test

* fix meraki-cloud-controller device tag name

* rebase

* fix servertech-pdu4 snmprec & test

* fix sinetica-eagle-i snmprec & test

* fix western-digital snmprec & test

* rebase

* Update western-digital-mycloud-ex2-ultra.snmprec

fix type in snmprec for western-digital

* Update infinera-coriant-groove.snmprec

fix type in snmprec for infinera-coriant

* rebase

* update test_profiles.py

* update core tests

* let arista-switch test pass & fix it separately

* minor changes to profiles

* update _cisco-generic with the tooling

* update alcatel-lucent-ent with the tooling

* update alcatel-lucent-ind with the tooling

* update alcatel-lucent-omni-access-wlc with the tooling

* fix tests for cisco profiles

* update test for arista after fixing extends in profiles

* add changelog

* (turns out we cannot fix previous changelogs)

* fix mapping #15322

* address reviews

* address reviews

* fix nasuni-filer tags & snmprec & test

* rebase

* rebase

* let arista-switch test pass & fix it separately

* minor changes to profiles

* update test for arista after fixing extends in profiles

* fix cisco_nexus mapping & test for netgear

* delete jupiter-variation.yaml

* Update changelog

Co-authored-by: jhgilbert <jen.gilbert@datadoghq.com>

* fix netgear-readynas

* unexpected failure due to diagnoses metadata

* diagnoses metadata

* diagnoses metadata

---------

Co-authored-by: NouhaManai <nouha.manai@yahoo.fr>
Co-authored-by: lizaerem <elizaveta.eremina@datadoghq.com>
Co-authored-by: Elizaveta Eremina <70374721+lizaerem@users.noreply.github.com>
Co-authored-by: jhgilbert <jen.gilbert@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dplepage-dd dplepage-dd dplepage-dd left review comments

@AlexandreYang AlexandreYang AlexandreYang approved these changes

@NouhaManai96 NouhaManai96 Awaiting requested review from NouhaManai96

Assignees
No one assigned
Labels
integration/snmp
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@FlorianVeaux @AlexandreYang @NouhaManai96 @dplepage-dd