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.

Sign up for GitHub

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

Jump to bottom

Group servers in upstream #90

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

Group servers in upstream #90

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion nginx_vts_exporter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,31 @@ func newCacheMetric(metricName string, docString string, labels []string) *prome
)
}

func groupUpstreamsByServer(upstreamList []Upstream) []Upstream {
groupedByServer := make([]Upstream, 0)
upstreamsMap := make(map[string]int)

for _, upstream := range upstreamList {
if index, exists := upstreamsMap[upstream.Server]; exists {
groupedByServer[index].RequestCounter += upstream.RequestCounter
groupedByServer[index].InBytes += upstream.InBytes
groupedByServer[index].OutBytes += upstream.OutBytes
groupedByServer[index].Responses.OneXx += upstream.Responses.OneXx
groupedByServer[index].Responses.TwoXx += upstream.Responses.TwoXx
groupedByServer[index].Responses.ThreeXx += upstream.Responses.ThreeXx
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
Copy link

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).

} else {
groupedByServer = append(groupedByServer, upstream)
upstreamsMap[upstream.Server] = len(groupedByServer) - 1
}
}

return groupedByServer
}

func NewExporter(uri string) *Exporter {
return &Exporter{
URI: uri,
Expand Down Expand Up @@ -281,7 +306,9 @@ func (e *Exporter) Collect(ch chan<- prometheus.Metric) {

// UpstreamZones
for name, upstreamList := range nginxVtx.UpstreamZones {
for _, s := range upstreamList {
groupedUpstreamsByServer := groupUpstreamsByServer(upstreamList)

for _, s := range groupedUpstreamsByServer {
ch <- prometheus.MustNewConstMetric(e.upstreamMetrics["responseMsec"], prometheus.GaugeValue, float64(s.ResponseMsec), name, s.Server)
ch <- prometheus.MustNewConstMetric(e.upstreamMetrics["requestMsec"], prometheus.GaugeValue, float64(s.RequestMsec), name, s.Server)

Expand Down