Skip to content

Add task category tag to history tasks metrics #6769

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

Merged
merged 1 commit into from
Apr 1, 2025

Conversation

Shaddoll
Copy link
Member

@Shaddoll Shaddoll commented Mar 31, 2025

What changed?
Add task category tag to history tasks metrics

Why?
Improve metrics

How did you test it?
verified in dev2
Screenshot 2025-04-01 at 11 29 21 AM

Potential risks

Release notes

Documentation Changes

@@ -143,6 +143,23 @@ func (p *base) call(scope int, op func() error, tags ...metrics.Tag) error {
return err
}

func (p *base) callWithoutDomainTag(scope int, op func() error, tags ...metrics.Tag) error {
Copy link
Member

Choose a reason for hiding this comment

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

what about the functions that uses callWithDomainAndShardScope? category will be missing for those

Copy link
Member Author

Choose a reason for hiding this comment

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

These history tasks methods don't fall into that branch, because they are not domainTaggedRequest.

Copy link
Member

Choose a reason for hiding this comment

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

Got it. So some of the generated method never call that branch which uses callWithDomainAndShardScope and all history task related methods are in that category. Would be nice to change code gen template to avoid spitting unused code into each function.

@Shaddoll Shaddoll merged commit b233ee0 into cadence-workflow:master Apr 1, 2025
22 checks passed
@Shaddoll Shaddoll deleted the p10 branch April 1, 2025 21:01
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.

2 participants