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

upstream metrics query improvements from grafana #6051

Closed
3 tasks done
sanderr opened this issue May 26, 2023 · 0 comments
Closed
3 tasks done

upstream metrics query improvements from grafana #6051

sanderr opened this issue May 26, 2023 · 0 comments
Assignees
@arnaudsjs
Labels
enhancement New feature or request usability This ticket is related to improved usability

Comments

@sanderr
Copy link
Contributor

sanderr commented May 26, 2023 •
edited by arnaudsjs

  • Use nicely rounded timestamps instead of counting back exactly from now. See [grafana dashboard]: open up the "lsm.service_count" panel with "edit". Note how if you hover the graph you only see nicely rounded interval boundaries. Click "query inspector", note how the $__timeGroup macro is expanded to a bucket value similar to our own but rounding down to the nearest timestamp that is divisible by the interval width, ensuring more readable boundaries.
  • use AVG instead of sum(count)::float)/(count(*)::float
  • Consider using a sliding window query like those on grafana, aggregating not only over the interval itself but over a wider history. This one requires more discussion, may or may not be desired. If desired, open up the "compile rate sliding window" panel with "edit", then click "query inspector". Note especially the generate_series subquery and the subsequent join. We may want to delegate this checkbox to some time in the future when we have better insights in how useful the sliding window is.
@sanderr sanderr added enhancement New feature or request usability This ticket is related to improved usability labels May 26, 2023
@arnaudsjs arnaudsjs self-assigned this Aug 7, 2023
This was referenced Aug 16, 2023
Closed
Closed
inmantaci pushed a commit that referenced this issue Aug 25, 2023
@arnaudsjs @inmantaci
Added support to the GET /metrics endpoint to round the returned ti…
0523e05 
…mestamps to a full hour. (Issue #6051, PR #6396)

# Description

This PR adds support to the `GET /metrics` endpoint to round the returned timestamps to the previous hour

closes #6051

# Self Check:

- [x] Attached issue to pull request
- [x] Changelog entry
- [x] Type annotations are present
- [x] Code is clear and sufficiently documented
- [x] No (preventable) type errors (check using make mypy or make mypy-diff)
- [x] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [ ] ~~End user documentation is included or an issue is created for end-user documentation~~
- [ ] ~~If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)~~
inmantaci pushed a commit that referenced this issue Aug 25, 2023
@arnaudsjs @inmantaci
Added support to the GET /metrics endpoint to round the returned ti…
bc85cf0 
…mestamps to a full hour. (Issue #6051, PR #6396)

Pull request opened by the merge tool on behalf of #6396
@arnaudsjs arnaudsjs mentioned this issue Aug 29, 2023
Closed
9 tasks
inmantaci pushed a commit that referenced this issue Aug 29, 2023
@arnaudsjs @inmantaci
Fix test case that breaks when run on a machine with a different defa…
3d260f4 
…ult timestamp. (Issue #6051, PR #6439)

# Description

The timestamp inputs provided to the `test_get_environment_metrics_api_endpoint_round_timestamp` test case were using the default timezone of the machine the test case was executed on. As such, the test case used different inputs depending on the timezone it was executed on. This caused the test case when executing in the UTC timezone. This PR fixes that issue by using fixed inputs.

Belongs to #6051

# Self Check:

- [x] Attached issue to pull request
- [x] Changelog entry
- [x] Type annotations are present
- [x] Code is clear and sufficiently documented
- [x] No (preventable) type errors (check using make mypy or make mypy-diff)
- [x] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [ ] ~~End user documentation is included or an issue is created for end-user documentation~~
- [ ] ~~If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)~~
inmantaci pushed a commit that referenced this issue Aug 29, 2023
@arnaudsjs @inmantaci
Fix test case that breaks when run on a machine with a different defa…
a41bc7c 
…ult timestamp. (Issue #6051, PR #6439)

# Description

The timestamp inputs provided to the `test_get_environment_metrics_api_endpoint_round_timestamp` test case were using the default timezone of the machine the test case was executed on. As such, the test case used different inputs depending on the timezone it was executed on. This caused the test case when executing in the UTC timezone. This PR fixes that issue by using fixed inputs.

Belongs to #6051

# Self Check:

- [x] Attached issue to pull request
- [x] Changelog entry
- [x] Type annotations are present
- [x] Code is clear and sufficiently documented
- [x] No (preventable) type errors (check using make mypy or make mypy-diff)
- [x] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [ ] ~~End user documentation is included or an issue is created for end-user documentation~~
- [ ] ~~If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)~~
inmantaci pushed a commit that referenced this issue Aug 29, 2023
@arnaudsjs @inmantaci
Fix test case that breaks when run on a machine with a different defa…
c8dc8c0 
…ult timestamp. (Issue #6051, PR #6439)

Pull request opened by the merge tool on behalf of #6439
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@arnaudsjs arnaudsjs

Labels
enhancement New feature or request usability This ticket is related to improved usability
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for rounded timestamps on environment metrics inmanta/inmanta-core
2 participants
@arnaudsjs @sanderr