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 more metrics to aggregated endpoint #13684

Merged
merged 4 commits into from
Jan 19, 2023
Merged

add more metrics to aggregated endpoint #13684

merged 4 commits into from
Jan 19, 2023

Conversation

iliakur
Copy link
Contributor

@iliakur iliakur commented Jan 12, 2023

What does this PR do?

Motivation

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
  • If the PR doesn't need to be tested during QA, please add a qa/skip-qa label.

@ghost ghost added the integration/rabbitmq label Jan 12, 2023
This was referenced Jan 12, 2023
Merged
Merged
@iliakur Graphite App
Copy link
Contributor Author

iliakur commented Jan 12, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@iliakur iliakur force-pushed the 01-12-add_more_metrics_to_aggregated_endpoint branch 2 times, most recently from 53fe752 to 55cee8f Compare January 13, 2023 17:03
This was referenced Jan 13, 2023
Merged
Merged
@codecov
Copy link

codecov bot commented Jan 13, 2023 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (ik/AI-2736/rabbitmq-openmetrics@365ccfb). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 85c38dc differs from pull request most recent head fd220ef. Consider uploading reports for the commit fd220ef to get more accurate results

Flag Coverage Δ
rabbitmq 94.67% <0.00%> (?)

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

@iliakur iliakur marked this pull request as ready for review January 13, 2023 17:32
@iliakur iliakur requested a review from a team as a code owner January 13, 2023 17:32
@iliakur iliakur force-pushed the 01-12-add_more_metrics_to_aggregated_endpoint branch from d96f2f4 to 8b2b863 Compare January 18, 2023 19:18
This was referenced Jan 18, 2023
Merged
Merged
@iliakur iliakur force-pushed the 01-11-only_aggregated_endpoint branch from 845687a to 7c97e4b Compare January 18, 2023 19:22
@iliakur iliakur requested a review from a team as a code owner January 18, 2023 19:22
@iliakur iliakur force-pushed the 01-12-add_more_metrics_to_aggregated_endpoint branch from 8b2b863 to b12a5e1 Compare January 18, 2023 19:22
@iliakur iliakur mentioned this pull request Jan 18, 2023
Merged
5 tasks
@maycmlee
Copy link
Contributor

Just checking if this is still work in progress because the metric descriptions are missing in the metadata.csv.

@iliakur iliakur mentioned this pull request Jan 19, 2023
Merged
5 tasks
Base automatically changed from 01-11-only_aggregated_endpoint to ik/AI-2736/rabbitmq-openmetrics January 19, 2023 17:20
@iliakur iliakur force-pushed the 01-12-add_more_metrics_to_aggregated_endpoint branch from 85c38dc to fd220ef Compare January 19, 2023 17:21
@iliakur iliakur merged commit 43f5503 into ik/AI-2736/rabbitmq-openmetrics Jan 19, 2023
@iliakur iliakur deleted the 01-12-add_more_metrics_to_aggregated_endpoint branch January 19, 2023 17:22
iliakur added a commit that referenced this pull request Jan 19, 2023
@iliakur @fanny-jiang
add more metrics to aggregated endpoint (#13684)
129c216 
* add more metrics to aggregated endpoint

* separate out summary metrics

* Refactor aggregated endpoint test (#13730)

* refactor aggregated endpoints test

* add missing metrics

* fix dd metric names

Co-authored-by: Fanny Jiang <[email protected]>
iliakur added a commit that referenced this pull request Jan 20, 2023
@iliakur @fanny-jiang
add more metrics to aggregated endpoint (#13684)
09a2293 
* add more metrics to aggregated endpoint

* separate out summary metrics

* Refactor aggregated endpoint test (#13730)

* refactor aggregated endpoints test

* add missing metrics

* fix dd metric names

Co-authored-by: Fanny Jiang <[email protected]>
iliakur added a commit that referenced this pull request Jan 20, 2023
@iliakur @fanny-jiang
add more metrics to aggregated endpoint (#13684)
aed6503 
* add more metrics to aggregated endpoint

* separate out summary metrics

* Refactor aggregated endpoint test (#13730)

* refactor aggregated endpoints test

* add missing metrics

* fix dd metric names

Co-authored-by: Fanny Jiang <[email protected]>
fanny-jiang added a commit that referenced this pull request Jan 20, 2023
@iliakur @alai97
@fanny-jiang @buraizu @estherk15
Support RabbitMQ Prometheus Metrics (#13662)
7f9a405 
* Setting the stage (#13437)

### What does this PR do?
<!-- A brief description of the change being made with this pull request. -->

The Openmetrics-related changes are minimal: just a basic class and a test that mostly makes sure that this class can be imported and run.

The rest is scaffolding: I needed new versions of rmq to test out my changes, the old tests needed adjusting to pass -> one config field had to be made optional to make sure my tests run -> etc...


### Motivation
<!-- What inspired you to submit this pull request? -->

### Additional Notes
<!-- Anything else we should know when reviewing? -->

### 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)](https://github.com/DataDog/integrations-core/blob/master/CONTRIBUTING.md#pull-request-title)
- [ ] 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
- [ ] If the PR doesn't need to be tested during QA, please add a `qa/skip-qa` label.

* Only select aggregated endpoint (#13673)

* only aggregated endpoint

* fix erlang.vm.allocators assertion

* copy suggestions

Co-authored-by: Austin Lai <[email protected]>

* Update rabbitmq/assets/configuration/spec.yaml

Co-authored-by: Austin Lai <[email protected]>

* Update models and example config to reflect current spec

* Don't test rabbitmq 3.8 (openmetrics) with python2

* fix PY2 failure

* consistent python env names; better setup to skip openmetrics tests

* Add openmetrics to E2E environment (#13695)

* add openmetrics e2e

* re-add newline to compose file

* revert hatch.toml changes

Co-authored-by: Austin Lai <[email protected]>
Co-authored-by: Fanny Jiang <[email protected]>

* add more metrics to aggregated endpoint (#13684)

* add more metrics to aggregated endpoint

* separate out summary metrics

* Refactor aggregated endpoint test (#13730)

* refactor aggregated endpoints test

* add missing metrics

* fix dd metric names

Co-authored-by: Fanny Jiang <[email protected]>

* support for specifying just detailed endpoint (#13691)

* support for specifying just detailed endpoint

* Update rabbitmq/datadog_checks/rabbitmq/data/conf.yaml.example

Co-authored-by: Bryce Eadie <[email protected]>

* Update rabbitmq/datadog_checks/rabbitmq/data/conf.yaml.example

Co-authored-by: Bryce Eadie <[email protected]>

* sync config

* apply and sync all the suggestions

Co-authored-by: Bryce Eadie <[email protected]>

* support for specifying just per-object endpoint (#13692)

* support for specifying just per-object endpoint

* Style fix: remove f-string without interpolation

* specify both detailed and aggregated endpoints in one instance (#13727)

* validate check configuration values (#13728)

* validate check configuration values

* several fixes:

- remove unnecessary validation logic
- fix url in one of the tests,
- fix style
- aggregated endpoint is enabled by default

* extend and refactor tests (#13737)

* extend and refactor tests

* add comment explaining assertion

* document implementation decisions (#13729)

* fix license headers

* Add openmetrics E2E test (#13744)

* enable openmetrics e2e test

* assert metrics

* small docs fixes (#13746)

* add license header to test file

* Define multiple instances in config spec (#13742)

* wip

* resync config

* document example detailed query endpoint

* fix e2e test

* remove example code from validators

* resync config spec

* add missing fields to metadata.csv, fix metric name (#13745)

* add missing fields to metadata.csv, fix metric name

* add special detailed metrics, fix validation errors

* Apply suggestions from code review

Co-authored-by: Fanny Jiang <[email protected]>

* remove last "total" suffixes

* add missing unit names

Co-authored-by: Fanny Jiang <[email protected]>

* readme draft 1

* readme, migration guide

* split integration and e2e tests

* Apply suggestions from code review

Co-authored-by: Fanny Jiang <[email protected]>

* fix license header

* move section

* remove "we" pronoun

* Update rabbitmq/README.md

Co-authored-by: Esther Kim <[email protected]>

* add migration guide section

* fix e2e test

Co-authored-by: Austin Lai <[email protected]>
Co-authored-by: Fanny Jiang <[email protected]>
Co-authored-by: Bryce Eadie <[email protected]>
Co-authored-by: Esther Kim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fanny-jiang fanny-jiang fanny-jiang approved these changes

Assignees
No one assigned
Labels
changelog/no-changelog integration/rabbitmq
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iliakur @maycmlee @fanny-jiang