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

Add taskStatus dimension to service/heartbeat metric #17488

Merged
merged 18 commits into from
Dec 6, 2024
Merged

Add taskStatus dimension to service/heartbeat metric #17488

merged 18 commits into from
Dec 6, 2024

Conversation

zachjsh
Copy link
Contributor

@zachjsh zachjsh commented Nov 18, 2024 •
edited
Loading

Description

Added a taskStatus dimension to the service/heartbeat metric. This can be used to detect cases when a particular task has been reporting its heartbeat in a particular state for an unusually long time; for example if a streaming task is stuck in paused state. Only tasks derived from SeekableStreamIndexTask report their taskStatus with this metric for now, other task types do not.

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@zachjsh zachjsh requested a review from georgew5656 November 18, 2024 21:43
kfaraz
kfaraz reviewed Nov 20, 2024
View reviewed changes
Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

Left some suggestions, similar to what had been suggested in an older PR #17268 (comment).

@zachjsh zachjsh requested a review from kfaraz December 4, 2024 20:26
kfaraz
kfaraz reviewed Dec 5, 2024
View reviewed changes
Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

Thanks for the update, @zachjsh . I have left some suggestions/queries.

@zachjsh zachjsh requested a review from kfaraz December 5, 2024 20:36
kfaraz
kfaraz approved these changes Dec 6, 2024
View reviewed changes
Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

Looks good, minor non-blocking suggestions.

@zachjsh zachjsh merged commit 3b6a3ae into apache:master Dec 6, 2024
80 of 82 checks passed
@zachjsh zachjsh deleted the heartbeat-metric-taskStatus-dim branch December 6, 2024 22:19
Akshat-Jain added a commit to Akshat-Jain/druid that referenced this pull request Dec 18, 2024
@Akshat-Jain
Revert "Add taskStatus dimension to service/heartbeat metric (apache#…
8990209 
…17488)"

This reverts commit 3b6a3ae.
@adarshsanjeev adarshsanjeev added this to the 32.0.0 milestone Jan 16, 2025
@adarshsanjeev adarshsanjeev mentioned this pull request Jan 28, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kfaraz kfaraz kfaraz approved these changes

@georgew5656 georgew5656 Awaiting requested review from georgew5656

Assignees
No one assigned
Labels
Area - Documentation Area - Ingestion Area - Metrics/Event Emitting Area - Streaming Ingestion
Projects
None yet
Milestone
32.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@zachjsh @kfaraz @adarshsanjeev