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

[BUG] Streak length metric reporting is broken #5170

Closed
2 tasks done
Tom-Newton opened this issue Apr 3, 2024 · 1 comment · Fixed by #5172
Closed
2 tasks done

[BUG] Streak length metric reporting is broken #5170

Tom-Newton opened this issue Apr 3, 2024 · 1 comment · Fixed by #5172
Labels
bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers

Comments

@Tom-Newton
Copy link
Contributor

Describe the bug

The streak length is always reported to be 0, but I can see from the logs that its actually non-zero.

Expected behavior

The metric should accurately report streak length.

Additional context to reproduce

Deploy flytepropeller and look at the flyte:propeller:all:round:streak_length_unlabeled metric. It will always report 0.

Root cause

Code looks like this:

	streak := 0
	defer p.metrics.StreakLength.Add(ctx, float64(streak))

	maxLength := p.cfg.MaxStreakLength
	if maxLength <= 0 {
		maxLength = 1
	}

	for streak = 0; streak < maxLength; streak++ {
		w, err = p.streak(ctx, w, wfClosureCrdFields)
		if err != nil {
			return err
		} else if w == nil {
			break
		}

		logger.Infof(ctx, "FastFollow Enabled. Detected State change, we will try another round. StreakLength [%d]", streak)
	}
	logger.Infof(ctx, "Streak ended at [%d]/Max: [%d]", streak, maxLength)
	return nil
}

The problem is that defer evaluates float64(streak) = 0 immediately and just waits until the end to report a zero. I can make a PR to fix this.

Screenshots

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@Tom-Newton Tom-Newton added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Apr 3, 2024
Copy link

dosubot bot commented Apr 3, 2024

It seems like you've got this under control, if you want help or have specific questions, let me know what I can do for you!

About Dosu

This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant