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

Group servers in upstream #90

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Goodluckhf
Copy link

@Goodluckhf Goodluckhf commented Sep 5, 2020

This PR fix issue: If you put the same server multiple times to upstream you will get an error "...was collected before with the same name and label" But I have to use this for retries to the same server

groupedByServer[index].Responses.FourXx += upstream.Responses.FourXx
groupedByServer[index].Responses.FiveXx += upstream.Responses.FiveXx
groupedByServer[index].RequestMsec = (groupedByServer[index].RequestMsec + upstream.RequestMsec) / 2
groupedByServer[index].ResponseMsec = (groupedByServer[index].ResponseMsec + upstream.ResponseMsec) / 2

Choose a reason for hiding this comment

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

upstream.ResponseMsec seems to be computed as an average, but computing an average on top of averages cannot technically be considered an average. For example, I think for a single server in upstream with ResponseMsec=42ms the aggregated ResponseMsec would be 21ms ((0 + 42) / 2 = 21), which is different from the expected 42ms.

I'd convert the averages back to the sums of times (assuming that RequestCounter is the denominator of the averages), and then compute the aggregated average using the sums, something like this:

if (groupedByServer[index].RequestCounter + upstream.RequestCounter) > 0 {
	groupedByServer[index].RequestMsec = (groupedByServer[index].RequestMsec * groupedByServer[index].RequestCounter + upstream.RequestMsec * upstream.RequestCounter) / (groupedByServer[index].RequestCounter + upstream.RequestCounter)
	groupedByServer[index].ResponseMsec = (groupedByServer[index].ResponseMsec * groupedByServer[index].RequestCounter + upstream.ResponseMsec * upstream.RequestCounter) / (groupedByServer[index].RequestCounter + upstream.RequestCounter)
}
groupedByServer[index].RequestCounter += upstream.RequestCounter  // note that this increment has to happen *after* the averages

Despite this little issue -- thank you for the great patch!

(Note, I'm not a maintainer of this project, I just faced the same issue and have reused your helpful solution).

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