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

refactor: move metrics out of /rules(/:id) to /rules/:id/metrics #9461

Conversation

sstrigler
Copy link
Contributor

@sstrigler sstrigler commented Dec 1, 2022

Fixes EMQX-7988

Summary:

  • you have to query GET /rules/:id/metrics explicitly in order to get metrics to a given rule. To reset metrics of a given you resource you PUT /rules/:id/metrics/reset.
  • /rules/:id/metrics/reset now returns 204 instead of 200 to comply with rest of API
  • for all queries that include :id a 404 is returned in case rule does not exist

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/emqx/emqx/blob/master/CONTRIBUTING.md.

PR Checklist

Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:

  • Added tests for the changes
  • Changed lines covered in coverage report
  • Change log has been added to changes/ dir
  • For EMQX 4.x: appup files updated (execute scripts/update-appup.sh emqx)
  • For internal contributor: there is a jira ticket to track this change
  • If there should be document changes, a PR to emqx-docs.git is sent, or a jira ticket is created to follow up
  • In case of non-backward compatible changes, reviewer should check this item as a write-off, and add details in Backward Compatibility section
  • Sync with dasboard team

Backward Compatibility

More information

@sstrigler sstrigler force-pushed the EMQX-7988-move-metrics-out-of-get-rules-response-to-separate-api branch from cc5dfb1 to cbead81 Compare December 1, 2022 14:57
@sstrigler sstrigler changed the base branch from master to release-50 December 5, 2022 10:00
@sstrigler sstrigler force-pushed the EMQX-7988-move-metrics-out-of-get-rules-response-to-separate-api branch 5 times, most recently from 5ddf926 to 333de23 Compare December 5, 2022 10:54
@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 3619333174

  • 18 of 19 (94.74%) changed or added relevant lines in 2 files are covered.
  • 40 unchanged lines in 7 files lost coverage.
  • Overall coverage increased (+0.02%) to 78.323%

Changes Missing Coverage Covered Lines Changed/Added Lines %
apps/emqx_rule_engine/src/emqx_rule_engine_api.erl 12 13 92.31%
Files with Coverage Reduction New Missed Lines %
apps/emqx/src/emqx_broker.erl 1 84.62%
apps/emqx/src/emqx_logger.erl 1 80.49%
apps/emqx_conf/src/emqx_conf_schema.erl 1 92.31%
apps/emqx/src/emqx_connection.erl 1 88.18%
apps/emqx/src/emqx_banned.erl 2 90.91%
apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl 15 74.11%
apps/emqx/src/emqx_reason_codes.erl 19 85.29%
Totals Coverage Status
Change from base Build 3619032721: 0.02%
Covered Lines: 21021
Relevant Lines: 26839

💛 - Coveralls

@sstrigler sstrigler force-pushed the EMQX-7988-move-metrics-out-of-get-rules-response-to-separate-api branch 2 times, most recently from 0d25636 to 26f97c7 Compare December 6, 2022 09:11
@sstrigler sstrigler force-pushed the EMQX-7988-move-metrics-out-of-get-rules-response-to-separate-api branch from 26f97c7 to b43312a Compare December 6, 2022 11:12
@sstrigler sstrigler force-pushed the EMQX-7988-move-metrics-out-of-get-rules-response-to-separate-api branch from b43312a to 0b324da Compare December 6, 2022 14:24
@sstrigler sstrigler merged commit fa80c4d into emqx:release-50 Dec 6, 2022
@sstrigler sstrigler deleted the EMQX-7988-move-metrics-out-of-get-rules-response-to-separate-api branch February 17, 2023 09:21
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.

None yet

3 participants