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

refactor: run metric threads startup and shutdown #24512

Merged
merged 1 commit into from
Oct 8, 2024
Merged

refactor: run metric threads startup and shutdown #24512

merged 1 commit into from
Oct 8, 2024

Conversation

mlarose
Copy link
Contributor

@mlarose mlarose commented Sep 16, 2024 •
edited
Loading

Summary & Motivation

  • ensure that the run telemetry function is supported by the instance before starting the thread
  • refactors the run metrics thread functionality to reduce code duplication.
  • adjusts the default polling intervals.

Replaces: #23503 - revert behavior to starting the metric collection thread only if the dagster/run_metrics tag is set.

How I Tested These Changes

  • BK
  • Manual testing (retested 2024/09/17)

Changelog

NOCHANGELOG

@mlarose Graphite App
Copy link
Contributor Author

mlarose commented Sep 16, 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 changed the title feat: automatically enable container metric collection in a containerized environment refactor: run metric threads startup and shutdown Sep 16, 2024
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from 0eb7602 to 558c7c6 Compare 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 requested a review from gibsondan September 16, 2024 19:24
@mlarose mlarose marked this pull request as ready for review September 16, 2024 19:24
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch 2 times, most recently from 62b8c1d to 7d09782 Compare September 17, 2024 18:24
@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
This was referenced Sep 23, 2024
Merged
Merged
@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch 5 times, most recently from 0e33604 to c40d3bc 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:06 PM EDT: Graphite rebased this pull request as part of a merge.
  • Oct 8, 2:07 PM EDT: @mlarose merged this pull request with Graphite.

@mlarose mlarose force-pushed the mlarose/run_metrics_ga branch from c40d3bc to f0938fd Compare October 8, 2024 18:05
@mlarose mlarose merged commit b7a88a2 into master Oct 8, 2024
1 check was pending
@mlarose mlarose deleted the mlarose/run_metrics_ga branch October 8, 2024 18:07
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

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