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

MapR integration #4380

Merged
merged 19 commits into from
Oct 11, 2019
Merged

MapR integration #4380

merged 19 commits into from
Oct 11, 2019

Conversation

FlorianVeaux
Copy link
Member

@FlorianVeaux FlorianVeaux commented Aug 15, 2019

MapR integration

@FlorianVeaux FlorianVeaux requested review from a team as code owners August 15, 2019 18:43
@hithwen hithwen self-assigned this Aug 22, 2019
@hithwen hithwen changed the title Initial POC MapR integration Aug 22, 2019
@codecov
Copy link

codecov bot commented Aug 23, 2019

Codecov Report

Merging #4380 into master will increase coverage by 0.59%.
The diff coverage is 86.31%.

Impacted Files Coverage Δ
mapr/datadog_checks/mapr/common.py 100% <100%> (ø)
mapr/datadog_checks/mapr/__about__.py 100% <100%> (ø)
mapr/tests/common.py 100% <100%> (ø)
mapr/datadog_checks/mapr/__init__.py 100% <100%> (ø)
mapr/datadog_checks/mapr/utils.py 100% <100%> (ø)
mapr/datadog_checks/mapr/mapr.py 73.03% <73.03%> (ø)
mapr/tests/conftest.py 95.45% <95.45%> (ø)
mapr/tests/test_unit.py 98.11% <98.11%> (ø)
vertica/tests/metrics.py
...checks_dev/tests/plugin/test_environment_runner.py
... and 854 more

Copy link
Contributor

@ruthnaebeck ruthnaebeck left a comment

Choose a reason for hiding this comment

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

Docs review

mapr/README.md Outdated Show resolved Hide resolved
mapr/README.md Outdated Show resolved Hide resolved
mapr/README.md Outdated Show resolved Hide resolved
mapr/README.md Outdated Show resolved Hide resolved
mapr/README.md Outdated Show resolved Hide resolved
mapr/metadata.csv Outdated Show resolved Hide resolved
mapr/metadata.csv Outdated Show resolved Hide resolved
mapr/metadata.csv Outdated Show resolved Hide resolved
mapr.rpc.bytes_sent,gauge,,byte,,The number of bytes sent by the MapR Filesystem over RPC.,0,mapr,
mapr.rpc.calls_recd,gauge,,message,,The number of RPC calls received by the MapR Filesystem.,0,mapr,
mapr.topology.utilization,gauge,,percent,, The aggregate percentage of CPU utilization.,0,mapr,
mapr.topology.disks_used_capacity,gauge,,gibibyte,,The amount disk space used in gigabytes.,0,mapr,
Copy link
Contributor

Choose a reason for hiding this comment

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

gibibytes or gigabytes?

mapr.rpc.calls_recd,gauge,,message,,The number of RPC calls received by the MapR Filesystem.,0,mapr,
mapr.topology.utilization,gauge,,percent,, The aggregate percentage of CPU utilization.,0,mapr,
mapr.topology.disks_used_capacity,gauge,,gibibyte,,The amount disk space used in gigabytes.,0,mapr,
mapr.topology.disks_total_capacity,gauge,,gibibyte,,The disk capacity in gigabytes. ,0,mapr,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
mapr.topology.disks_total_capacity,gauge,,gibibyte,,The disk capacity in gigabytes. ,0,mapr,
mapr.topology.disks_total_capacity,gauge,,gibibyte,,The disk capacity in gigabytes.,0,mapr,

gibibytes or gigabytes?

Copy link
Contributor

Choose a reason for hiding this comment

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

MapR documentation says gigabytes, metadata validation only allows for gibibyte 🤷‍♀

mapr/manifest.json Outdated Show resolved Hide resolved
@FlorianVeaux FlorianVeaux force-pushed the flo/mapr2 branch 2 times, most recently from 93e170d to 8b09ff7 Compare October 3, 2019 10:03
mapr/README.md Outdated Show resolved Hide resolved
mapr/README.md Outdated Show resolved Hide resolved
mapr/README.md Outdated Show resolved Hide resolved
FlorianVeaux and others added 2 commits October 7, 2019 11:23
Co-Authored-By: Pierre Guceski <[email protected]>
Co-Authored-By: Pierre Guceski <[email protected]>
mapr/README.md Outdated Show resolved Hide resolved
Copy link
Member

@AlexandreYang AlexandreYang left a comment

Choose a reason for hiding this comment

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

Awesome new integration !

👋, left a lot of comments, but some of them are clarification question and nits.

Feel free to ping me if something is not clear.

mapr/requirements-dev.txt Show resolved Hide resolved
mapr/README.md Outdated Show resolved Hide resolved
mapr/README.md Outdated Show resolved Hide resolved
mapr/README.md Show resolved Hide resolved
mapr/assets/service_checks.json Outdated Show resolved Hide resolved
mapr/datadog_checks/mapr/mapr.py Show resolved Hide resolved
mapr/datadog_checks/mapr/mapr.py Show resolved Hide resolved
mapr/datadog_checks/mapr/mapr.py Outdated Show resolved Hide resolved
mapr/manifest.json Show resolved Hide resolved
mapr/README.md Show resolved Hide resolved
mapr/metadata.csv Show resolved Hide resolved
mapr/metadata.csv Show resolved Hide resolved
mapr/metadata.csv Show resolved Hide resolved
mapr/metadata.csv Show resolved Hide resolved
mapr/metadata.csv Show resolved Hide resolved
@AlexandreYang
Copy link
Member

I added some comments about the metrics. But the type is quite difficult to check without that info in docs and without source code ...

Copy link
Member

@AlexandreYang AlexandreYang left a comment

Choose a reason for hiding this comment

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

LGTM

@FlorianVeaux FlorianVeaux merged commit 605f9fc into master Oct 11, 2019
@FlorianVeaux FlorianVeaux deleted the flo/mapr2 branch October 11, 2019 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants