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

Add support for NGINX Plus versions 4-7 #10750

Merged
merged 22 commits into from
Dec 7, 2021
Merged

Add support for NGINX Plus versions 4-7 #10750

merged 22 commits into from
Dec 7, 2021

Conversation

sarah-witt
Copy link
Contributor

@sarah-witt sarah-witt commented Nov 29, 2021 •
edited
Loading

What does this PR do?

  • Adds support for new endpoints and data added in nginx plus versions 4-7
  • These metrics represent counts, so they are submitted as counts only, which is unlike the other nginx plus metrics, which are sent as both gauges and counts, with _count appended to their names
  • Additionally sends metrics (nginx.stream.zone_sync.zone.records_total, nginx.upstream.peers.downtime, and 'nginx.stream.upstream.peers.downtime) from nginx plus as a count since they are currently only sent as gauges
  • fixes typo in metric nginx.cache.revalidated.responses so it is properly sent as a count as well
  • Adds new test fixture data from the nginx demo api

Motivation

Support more versions of nginx plus

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@github-actions
Copy link

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@codecov
Copy link

codecov bot commented Nov 29, 2021 •
edited
Loading

Codecov Report

Merging #10750 (95b8455) into master (8b8284e) will increase coverage by 0.02%.
The diff coverage is 99.43%.

Flag Coverage Δ
nginx 96.27% <99.43%> (+1.86%) ⬆️

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

@sarah-witt sarah-witt marked this pull request as ready for review November 29, 2021 23:22
@sarah-witt sarah-witt requested review from a team as code owners November 29, 2021 23:22
apigirl
apigirl previously approved these changes Nov 30, 2021
View reviewed changes
Copy link
Contributor

@apigirl apigirl left a comment

Choose a reason for hiding this comment

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

looks good for docs!

coignetp
coignetp requested changes Nov 30, 2021
View reviewed changes
nginx/datadog_checks/nginx/nginx.py Outdated Show resolved Hide resolved
nginx/tests/fixtures/plus_api_http_location_zones.json Outdated Show resolved Hide resolved
nginx/tests/test_unit.py Show resolved Hide resolved
nginx/tests/test_unit.py Show resolved Hide resolved
coignetp
coignetp requested changes Dec 2, 2021
View reviewed changes
Copy link
Contributor

@coignetp coignetp left a comment

Choose a reason for hiding this comment

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

Thanks! Just a nit

nginx/tests/test_nginx.py Show resolved Hide resolved
coignetp
coignetp previously approved these changes Dec 2, 2021
View reviewed changes
Copy link
Contributor

@coignetp coignetp left a comment

Choose a reason for hiding this comment

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

🚀

@hithwen hithwen mentioned this pull request Dec 3, 2021
Merged
@github-actions
Copy link

github-actions bot commented Dec 6, 2021

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

ofek
ofek approved these changes Dec 7, 2021
View reviewed changes
Copy link
Contributor

@ofek ofek left a comment

Choose a reason for hiding this comment

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

🔥

@sarah-witt sarah-witt merged commit 503c0a2 into master Dec 7, 2021
@sarah-witt sarah-witt deleted the sarah/nginx-v6 branch December 7, 2021 13:49
@sarah-witt sarah-witt mentioned this pull request Dec 7, 2021
Merged
4 tasks
cswatt pushed a commit that referenced this pull request Jan 5, 2022
@sarah-witt @cswatt
Add support for NGINX Plus versions 4-7 (#10750)
88cbf44 
* Add test data and limit_reqs endpoint

* Add support for new endpoints in check

* Update metadata with new metrics

* add more tests for tags

* Add more tests and http limit conns endpoint

* Add tests for all plus api versions

* Reformat code and fix typos

* Remove unneeded mappings

* Remove unneeded mapping and test fixture

* Properly handle metrics that should be counts

* Change more metrics to count

* Fix short names of metrics

* Update style and add assertion for all count metrics

* Update conf.example with new description

* Update test folder structure and add mapping for plus api endpoints

* Assert metadata in all tests

* Add missing count metrics, assert metrics covered, and refactor tests

* Remove slashes in directories

* Update comment typo and fix units in metadata.csv

* Remove peer unit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ofek ofek ofek approved these changes

@coignetp coignetp coignetp approved these changes

@apigirl apigirl apigirl left review comments

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

Successfully merging this pull request may close these issues.
4 participants
@sarah-witt @ofek @coignetp @apigirl