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

How to set slowThreshold #4641

Closed
Nanosk07 opened this issue Feb 11, 2025 · 2 comments · Fixed by #4654
Closed

How to set slowThreshold #4641

Nanosk07 opened this issue Feb 11, 2025 · 2 comments · Fixed by #4654

Comments

@Nanosk07
Copy link
Contributor

In the function below, slowThreshold is used to determine if a call is slow. However, when I try to modify slowThreshold using SetSlowThreshold function, the comment suggests using StatConf instead. StatConf.SlowThreshold is passed into the logDuration function as the durationThreshold parameter. It is only used in the if isSlow(duration, durationThreshold) check, but it does not affect slowThreshold. Does this indicate a bug?

func logDuration(ctx context.Context, method string, req any, duration time.Duration,ignoreMethods *collection.Set, durationThreshold time.Duration) {

      ...

	if !shouldLogContent(method, ignoreMethods) {
		if isSlow(duration, durationThreshold) {
			logger.Slowf("[RPC] slowcall - %s - %s", addr, method)
		}
	} else {
		content, err := json.Marshal(req)
		if err != nil {
			logx.WithContext(ctx).Errorf("%s - %s", addr, err.Error())
		} else if duration > slowThreshold.Load() {
			logger.Slowf("[RPC] slowcall - %s - %s - %s", addr, method, string(content))
		} else {
			logger.Infof("%s - %s - %s", addr, method, string(content))
		}
	}
}



// SetSlowThreshold sets the slow threshold.
// Deprecated: use StatConf instead.
func SetSlowThreshold(threshold time.Duration) {
	slowThreshold.Set(threshold)
}
@kevwan
Copy link
Contributor

kevwan commented Feb 13, 2025

Yes, would you please submit a PR? Thanks!

@Nanosk07
Copy link
Contributor Author

Nanosk07 commented Feb 14, 2025

Yes, would you please submit a PR? Thanks!

Hi,
I have submitted a PR #4654 that completely removes all usages of slowThreshold.

I also removed the default value for StatConf.SlowThreshold, allowing users to decide whether to enable slow logs. However, I’m not sure if this aligns with the original design. Please help me confirm. If this change is not appropriate, I will restore the default value.

Thanks!

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 a pull request may close this issue.

2 participants