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

Add logs to debug timer tasks #5581

Merged
merged 2 commits into from
Jan 5, 2024
Merged

Add logs to debug timer tasks #5581

merged 2 commits into from
Jan 5, 2024

Conversation

Shaddoll
Copy link
Contributor

@Shaddoll Shaddoll commented Jan 5, 2024

What changed?
Add logs to debug timer tasks

Why?
To improve observability for debugging duplicate timer issues

How did you test it?

Potential risks

Release notes

Documentation Changes

@@ -1919,6 +1919,9 @@ const (
// Value type: Bool
// Default value: true
EnableShardIDMetrics

Copy link
Contributor

Choose a reason for hiding this comment

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

can you add the comments as the other constants have? it's kind of redundant with below definition but better to be consistent until we come up with a better structure to define these dynamicconfigs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I'm planning to remove those comments.

tag.TaskID(task.TaskID),
tag.WorkflowTimerID(timerInfo.TimerID),
tag.WorkflowScheduleID(timerInfo.StartedID),
tag.WorkflowNextEventID(mutableState.GetNextEventID()),
Copy link
Contributor

Choose a reason for hiding this comment

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

do you think it would help to include details from timerSequenceID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

timerSequenceID.EventID is the same as timerInfo.StartedID

Comment on lines +220 to +227
tag.WorkflowDomainID(task.DomainID),
tag.WorkflowID(task.WorkflowID),
tag.WorkflowRunID(task.RunID),
tag.TaskType(task.TaskType),
tag.TaskID(task.TaskID),
tag.WorkflowTimerID(timerInfo.TimerID),
tag.WorkflowScheduleID(timerInfo.StartedID),
tag.WorkflowNextEventID(mutableState.GetNextEventID()),
Copy link
Contributor

Choose a reason for hiding this comment

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

you can define the tags at line 206 above if/else so they don't go out of sync between info and debug calls in the future

@Shaddoll Shaddoll merged commit a184c39 into uber:master Jan 5, 2024
16 checks passed
@Shaddoll Shaddoll deleted the timer-debug branch January 5, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants