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

feat: run metrics improvements #23510

Merged
merged 1 commit into from
Oct 8, 2024
Merged

feat: run metrics improvements #23510

merged 1 commit into from
Oct 8, 2024

Conversation

mlarose
Copy link
Contributor

@mlarose mlarose commented Aug 8, 2024 •
edited
Loading

Summary & Motivation

  • Fix edge conditions discovered while working on code server metrics
  • Calculate cpu usage rate instead of relying on datadog derivative calculation

How I Tested These Changes

  • BK / new unit tests
  • Manual testing (retested 2024/09/17)

@mlarose Graphite App
Copy link
Contributor Author

mlarose commented Aug 8, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @mlarose and the rest of your teammates on Graphite Graphite

@mlarose mlarose mentioned this pull request Aug 8, 2024
Closed
4 tasks
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from d4b99de to cf5a567 Compare August 8, 2024 16:56
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from 270f2ef to d269f2e Compare August 8, 2024 16:56
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from cf5a567 to 4591f89 Compare August 26, 2024 17:37
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from d269f2e to dddc68b Compare August 26, 2024 17:37
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from 4591f89 to 1e6b92b Compare September 10, 2024 20:35
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from dddc68b to 4d5fd83 Compare September 10, 2024 20:35
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from 1e6b92b to 1f04d03 Compare September 11, 2024 20:21
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from 4d5fd83 to fc4b868 Compare September 11, 2024 20:21
@mlarose mlarose marked this pull request as ready for review September 12, 2024 19:03
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from 1f04d03 to 4287757 Compare September 13, 2024 11:20
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch 2 times, most recently from d1b32ad to 3cf095a Compare September 13, 2024 14:38
@mlarose mlarose changed the base branch from mlarose/run_metrics_ga to master September 13, 2024 14:38
@mlarose mlarose requested a review from gibsondan September 13, 2024 14:38
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch 2 times, most recently from 0940854 to 8b1ca66 Compare September 16, 2024 17:31
@mlarose mlarose changed the base branch from master to mlarose/run_metrics_ga September 16, 2024 17:31
@mlarose mlarose mentioned this pull request Sep 16, 2024
Merged
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from 558c7c6 to 2263605 Compare September 16, 2024 18:46
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from 8b1ca66 to 257ef52 Compare September 16, 2024 18:46
gibsondan
gibsondan reviewed Sep 17, 2024
View reviewed changes
Copy link
Member

@gibsondan gibsondan left a comment

Choose a reason for hiding this comment

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

just one question inline

python_modules/dagster/dagster/_core/execution/run_metrics_thread.py Outdated Show resolved Hide resolved
@gibsondan gibsondan requested a review from dpeng817 September 17, 2024 14:56
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from 2263605 to 62b8c1d Compare September 17, 2024 14:56
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from 257ef52 to d26c06f Compare September 17, 2024 14:57
gibsondan
gibsondan approved these changes Sep 17, 2024
View reviewed changes
Copy link
Member

@gibsondan gibsondan left a comment

Choose a reason for hiding this comment

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

got it, thanks - cc @dpeng817 in case you have any context from imlpementing code server metrics, but seems reasonable to me

@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from 62b8c1d to 7d09782 Compare September 17, 2024 18:24
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch 2 times, most recently from 60a8343 to 2a6ce7f Compare September 17, 2024 18:30
@mlarose mlarose mentioned this pull request Sep 17, 2024
Merged
2 tasks
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from 7d09782 to ff5bd36 Compare September 23, 2024 11:22
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from 2a6ce7f to a6376e2 Compare September 23, 2024 11:22
This was referenced Sep 23, 2024
Merged
Merged
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from ff5bd36 to cf39dcc Compare October 7, 2024 14:43
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from a6376e2 to 3eff5c2 Compare October 7, 2024 14:43
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from cf39dcc to ab94436 Compare October 7, 2024 14:59
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from 3eff5c2 to e51900f Compare October 7, 2024 14:59
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from ab94436 to 5366df3 Compare October 7, 2024 18:02
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from e51900f to d338339 Compare October 7, 2024 18:03
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from 5366df3 to 0e33604 Compare October 8, 2024 11:45
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from d338339 to 6c246bc Compare October 8, 2024 11:45
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from 0e33604 to c40d3bc Compare October 8, 2024 15:15
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from 6c246bc to 29e1dc3 Compare October 8, 2024 15:15
@mlarose Graphite App
Copy link
Contributor Author

mlarose commented Oct 8, 2024 •
edited
Loading

Merge activity

  • Oct 8, 2:05 PM EDT: @mlarose started a stack merge that includes this pull request via Graphite.
  • Oct 8, 2:10 PM EDT: Graphite rebased this pull request as part of a merge.
  • Oct 8, 2:11 PM EDT: @mlarose merged this pull request with Graphite.

@mlarose mlarose changed the base branch from mlarose/run_metrics_ga to graphite-base/23510 October 8, 2024 18:05
@mlarose mlarose changed the base branch from graphite-base/23510 to master October 8, 2024 18:07
@mlarose
feat: run metrics improvements
a88a239 
- better handling of cgroup limits edge cases
- don't push invalid memory_limit
- remove low value cfs metrics
- push calculated cpu usage rate for future use
@mlarose mlarose force-pushed the mlarose/run-metrics-improvement branch from 29e1dc3 to a88a239 Compare October 8, 2024 18:09
@mlarose mlarose merged commit b4037a3 into master Oct 8, 2024
1 check was pending
@mlarose mlarose deleted the mlarose/run-metrics-improvement branch October 8, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gibsondan gibsondan gibsondan approved these changes

@dpeng817 dpeng817 Awaiting requested review from dpeng817

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mlarose @gibsondan