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

feat(compute): Add some basic compute_ctl metrics #10504

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ololobus
Copy link
Member

@ololobus ololobus commented Jan 24, 2025

Problem

There are several parts of compute_ctl with a very low visibility of errors:

  1. DB migrations that run async in the background after compute start.
  2. Requests made to control plane (currently only GetSpec).
  3. Requests made to the remote extensions server.

Summary of changes

Add new counters to quickly evaluate the amount of errors among the fleet.

Copy link

github-actions bot commented Jan 24, 2025

7403 tests run: 7052 passed, 0 failed, 351 skipped (full report)


Flaky tests (11)

Postgres 17

Postgres 15

Code coverage* (full report)

  • functions: 33.5% (8498 of 25361 functions)
  • lines: 49.2% (71498 of 145197 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
67d5061 at 2025-01-27T22:55:27.600Z :recycle:

compute_tools/src/metrics.rs Outdated Show resolved Hide resolved
compute_tools/src/metrics.rs Show resolved Hide resolved
compute_tools/src/metrics.rs Outdated Show resolved Hide resolved
compute_tools/src/metrics.rs Outdated Show resolved Hide resolved
Copy link
Member

@tristan957 tristan957 left a comment

Choose a reason for hiding this comment

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

I think it could be a nice touch if the compute migration regression tests were extended to check that that the migration metric works as expected

@ololobus ololobus force-pushed the alexk/compute-ctl-metrics branch from c3df6c5 to de78bcd Compare January 27, 2025 21:49
@ololobus ololobus force-pushed the alexk/compute-ctl-metrics branch from de78bcd to 67d5061 Compare January 27, 2025 21:52
@ololobus ololobus marked this pull request as ready for review January 27, 2025 21:53
@ololobus ololobus requested a review from a team as a code owner January 27, 2025 21:53
@ololobus
Copy link
Member Author

I think it could be a nice touch if the compute migration regression tests were extended to check that that the migration metric works as expected

Yeah, also had tests on the list. Added to migrations and remote extensions testing

Copy link
Member

@tristan957 tristan957 left a comment

Choose a reason for hiding this comment

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

This is a good first iteration!

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.

2 participants