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

fix: More correctly report starting phase during kafka-reader startup #14632

Merged
merged 1 commit into from
Oct 29, 2024
Merged

fix: More correctly report starting phase during kafka-reader startup #14632

merged 1 commit into from
Oct 29, 2024

Conversation

benclive
Copy link
Contributor

What this PR does / why we need it:
Separates starting/running phases of kafka reader based on service state

  • We only emit a single datapoint for the startup phase in Kafka. Now we've implemented logic to delay startup until kafka is ready, I'm switching this to use the service state to better indicate how this happens.

@benclive benclive requested a review from a team as a code owner October 28, 2024 16:01
@benclive benclive changed the title Report starting phase during startup fix: More correctly report starting phase during kafka-reader startup Oct 28, 2024
cyriltovena
cyriltovena approved these changes Oct 29, 2024
View reviewed changes
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@cyriltovena cyriltovena merged commit ea798e0 into main Oct 29, 2024
62 checks passed
@cyriltovena cyriltovena deleted the report-starting-based-onstate branch October 29, 2024 07:52
@loki-gh-app loki-gh-app bot mentioned this pull request Nov 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyriltovena cyriltovena cyriltovena approved these changes

Assignees
No one assigned
Labels
size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@benclive @cyriltovena