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

chore: add _total suffix to counter metrics #8263

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

estensen
Copy link
Contributor

Closes #8150

@@ -405,10 +407,11 @@ impl TaskExecutor {
.in_current_span();

// Clone only the specific counter that we need.
let finished_critical_tasks_metrics = self.metrics.finished_critical_tasks.clone();
let finished_critical_tasks_total_metrics =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should the var names here map to the metrics 1-1? they do tend to become long

@estensen
Copy link
Contributor Author

I couldn't find any Grafana dashboards to update

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

ty

lgtm, pending @shekhirin or @emhane

Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

please update the grafana dashboard file

@emhane emhane added D-good-first-issue Nice and easy! A great choice to get started C-debt A section of code is hard to understand or change A-observability Related to tracing, metrics, logs and other observability tools labels May 16, 2024
@mattsse mattsse requested a review from gakonst as a code owner May 21, 2024 10:05
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

updated all used grafana variables I could find @onbjerg

@mattsse mattsse enabled auto-merge May 21, 2024 10:06
@mattsse mattsse requested a review from onbjerg May 21, 2024 10:09
@DaniPopes DaniPopes requested review from onbjerg and removed request for onbjerg May 23, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-observability Related to tracing, metrics, logs and other observability tools C-debt A section of code is hard to understand or change D-good-first-issue Nice and easy! A great choice to get started
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Counter metric name should adhere to *_total pattern
5 participants