Skip to content

Commit aaaf568

Browse files
committed
include gorouter_time metrics report
1 parent ffd637c commit aaaf568

17 files changed

+154
-25
lines changed

accesslog/schema/access_log_record.go

+2-16
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ type AccessLogRecord struct {
124124
TlsHandshakeFinishedAt time.Time
125125
AppRequestFinishedAt time.Time
126126
FinishedAt time.Time
127+
GorouterTime float64
127128

128129
LocalAddress string
129130
}
@@ -136,17 +137,6 @@ func (r *AccessLogRecord) roundtripTime() float64 {
136137
return r.FinishedAt.Sub(r.ReceivedAt).Seconds()
137138
}
138139

139-
func (r *AccessLogRecord) gorouterTime() float64 {
140-
rt := r.roundtripTime()
141-
at := r.appTime()
142-
143-
if rt >= 0 && at >= 0 {
144-
return rt - at
145-
} else {
146-
return -1
147-
}
148-
}
149-
150140
func (r *AccessLogRecord) dialTime() float64 {
151141
if r.DialStartedAt.IsZero() || r.DialFinishedAt.IsZero() {
152142
return -1
@@ -168,10 +158,6 @@ func (r *AccessLogRecord) tlsTime() float64 {
168158
return r.TlsHandshakeFinishedAt.Sub(r.TlsHandshakeStartedAt).Seconds()
169159
}
170160

171-
func (r *AccessLogRecord) appTime() float64 {
172-
return r.AppRequestFinishedAt.Sub(r.AppRequestStartedAt).Seconds()
173-
}
174-
175161
// failedAttemptsTime will be negative if there was no failed attempt.
176162
func (r *AccessLogRecord) failedAttemptsTime() float64 {
177163
if r.LastFailedAttemptFinishedAt.IsZero() {
@@ -281,7 +267,7 @@ func (r *AccessLogRecord) makeRecord(performTruncate bool) []byte {
281267

282268
// #nosec G104 - ignore errors from writing the access log as it will only cause more errors to log this error
283269
b.WriteString(`gorouter_time:`)
284-
b.WriteDashOrFloatValue(r.gorouterTime())
270+
b.WriteDashOrFloatValue(r.GorouterTime)
285271

286272
// #nosec G104 - ignore errors from writing the access log as it will only cause more errors to log this error
287273
b.WriteString(`app_id:`)

accesslog/schema/access_log_record_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ var _ = Describe("AccessLogRecord", func() {
6060
AppRequestFinishedAt: time.Date(2000, time.January, 1, 0, 0, 55, 0, time.UTC),
6161
RequestBytesReceived: 30,
6262
RouterError: "some-router-error",
63+
GorouterTime: 10,
6364
}
6465
})
6566

@@ -227,6 +228,7 @@ var _ = Describe("AccessLogRecord", func() {
227228
record.AppRequestFinishedAt = time.Time{}
228229
record.RequestBytesReceived = 0
229230
record.RouterError = ""
231+
record.GorouterTime = -1
230232
})
231233

232234
It("makes a record", func() {
@@ -301,6 +303,7 @@ var _ = Describe("AccessLogRecord", func() {
301303
AppRequestFinishedAt: time.Date(2000, time.January, 1, 0, 0, 55, 0, time.UTC),
302304
RequestBytesReceived: 30,
303305
ExtraHeadersToLog: []string{},
306+
GorouterTime: 10,
304307
}
305308

306309
r := BufferReader(bytes.NewBufferString(record.LogMessage()))

config/config.go

+2
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,15 @@ var defaultPrometheusConfig = PrometheusConfig{
126126

127127
type MetersConfig struct {
128128
RouteLookupTimeHistogramBuckets []float64 `yaml:"route_lookup_time_histogram_buckets,omitempty"`
129+
GorouterTimeHistogramBuckets []float64 `yaml:"gorouter_time_histogram_buckets,omitempty"`
129130
RouteRegistrationLatencyHistogramBuckets []float64 `yaml:"route_registration_latency_histogram_buckets,omitempty"`
130131
RoutingResponseLatencyHistogramBuckets []float64 `yaml:"routing_response_latency_histogram_buckets,omitempty"`
131132
HTTPLatencyHistogramBuckets []float64 `yaml:"http_latency_histogram_buckets,omitempty"`
132133
}
133134

134135
var defaultMetersConfig = MetersConfig{
135136
RouteLookupTimeHistogramBuckets: []float64{10_000, 20_000, 30_000, 40_000, 50_000, 60_000, 70_000, 80_000, 90_000, 100_000},
137+
GorouterTimeHistogramBuckets: []float64{1, 2, 4, 6, 8, 10, 20, 40, 50, 100, 500, 1000},
136138
RouteRegistrationLatencyHistogramBuckets: []float64{0.1, 0.5, 1, 1.5, 2, 2.5, 3, 3.5, 4},
137139
RoutingResponseLatencyHistogramBuckets: []float64{1, 2, 4, 6, 8, 10, 20, 40, 50, 100, 500, 1000},
138140
HTTPLatencyHistogramBuckets: []float64{0.1, 0.2, 0.4, 0.8, 1.6, 3.2, 6.4, 12.8, 25.6},

config/config_test.go

+2
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ prometheus:
252252
meters:
253253
route_lookup_time_histogram_buckets: [0, 100, 10000]
254254
route_registration_latency_histogram_buckets: [-10, 0, 10]
255+
gorouter_time_histogram_buckets: [1,2,4]
255256
routing_response_latency_histogram_buckets: [0.1, 0.5, 1]
256257
`)
257258

@@ -260,6 +261,7 @@ prometheus:
260261

261262
Expect(config.Prometheus.Meters.RouteLookupTimeHistogramBuckets).To(Equal([]float64{0, 100, 10000}))
262263
Expect(config.Prometheus.Meters.RouteRegistrationLatencyHistogramBuckets).To(Equal([]float64{-10, 0, 10}))
264+
Expect(config.Prometheus.Meters.GorouterTimeHistogramBuckets).To(Equal([]float64{1, 2, 4}))
263265
Expect(config.Prometheus.Meters.RoutingResponseLatencyHistogramBuckets).To(Equal([]float64{0.1, 0.5, 1}))
264266
})
265267

handlers/access_log.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"net/http"
77
"slices"
88
"sync/atomic"
9-
"time"
109

1110
"github.com/urfave/negroni/v3"
1211

@@ -60,8 +59,6 @@ func (a *accessLog) ServeHTTP(rw http.ResponseWriter, r *http.Request, next http
6059
return
6160
}
6261

63-
reqInfo.FinishedAt = time.Now()
64-
6562
alr.HeadersOverride = reqInfo.BackendReqHeaders
6663
alr.RouteEndpoint = reqInfo.RouteEndpoint
6764
alr.RequestBytesReceived = requestBodyCounter.GetCount()
@@ -82,6 +79,7 @@ func (a *accessLog) ServeHTTP(rw http.ResponseWriter, r *http.Request, next http
8279
alr.TlsHandshakeFinishedAt = reqInfo.TlsHandshakeFinishedAt
8380
alr.AppRequestFinishedAt = reqInfo.AppRequestFinishedAt
8481
alr.FinishedAt = reqInfo.FinishedAt
82+
alr.GorouterTime = reqInfo.GorouterTime
8583

8684
alr.LocalAddress = reqInfo.LocalAddress
8785

handlers/access_log_test.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@ import (
55
"io"
66
"net/http"
77
"net/http/httptest"
8+
"time"
89

910
. "github.com/onsi/ginkgo/v2"
1011
. "github.com/onsi/gomega"
1112
"github.com/urfave/negroni/v3"
1213

1314
"code.cloudfoundry.org/gorouter/accesslog/fakes"
1415
"code.cloudfoundry.org/gorouter/handlers"
16+
metrics_fakes "code.cloudfoundry.org/gorouter/metrics/fakes"
1517
"code.cloudfoundry.org/gorouter/proxy/utils"
1618
"code.cloudfoundry.org/gorouter/route"
1719
"code.cloudfoundry.org/gorouter/test_util"
@@ -26,6 +28,7 @@ var _ = Describe("AccessLog", func() {
2628

2729
logger *test_util.TestLogger
2830
accessLogger *fakes.FakeAccessLogger
31+
fakeReporter *metrics_fakes.FakeMetricReporter
2932
extraHeadersToLog []string
3033

3134
nextCalled bool
@@ -52,6 +55,10 @@ var _ = Describe("AccessLog", func() {
5255
reqInfo, err := handlers.ContextRequestInfo(req)
5356
if err == nil {
5457
reqInfo.RouteEndpoint = testEndpoint
58+
timeNow := time.Now()
59+
reqInfo.ReceivedAt = timeNow.Add(-1 * time.Millisecond)
60+
reqInfo.AppRequestStartedAt = timeNow.Add(1 * time.Millisecond)
61+
reqInfo.AppRequestFinishedAt = timeNow.Add(2 * time.Millisecond)
5562
}
5663

5764
if next != nil {
@@ -75,13 +82,15 @@ var _ = Describe("AccessLog", func() {
7582
extraHeadersToLog = []string{}
7683

7784
accessLogger = &fakes.FakeAccessLogger{}
85+
fakeReporter = new(metrics_fakes.FakeMetricReporter)
7886

7987
logger = test_util.NewTestLogger("test")
8088

8189
handler = negroni.New()
8290
handler.Use(handlers.NewRequestInfo())
8391
handler.Use(handlers.NewProxyWriter(logger.Logger))
8492
handler.Use(handlers.NewAccessLog(accessLogger, extraHeadersToLog, nil, logger.Logger))
93+
handler.Use(handlers.NewReporter(fakeReporter, logger.Logger))
8594
handler.Use(nextHandler)
8695

8796
reqChan = make(chan *http.Request, 1)
@@ -107,8 +116,8 @@ var _ = Describe("AccessLog", func() {
107116
Expect(alr.Request.URL).To(Equal(req.URL))
108117
Expect(alr.Request.RemoteAddr).To(Equal(req.RemoteAddr))
109118
Expect(alr.ExtraHeadersToLog).To(Equal(extraHeadersToLog))
110-
Expect(alr.FinishedAt).ToNot(BeZero())
111119
Expect(alr.RequestBytesReceived).To(Equal(13))
120+
Expect(alr.GorouterTime).ToNot(BeZero())
112121
Expect(alr.BodyBytesSent).To(Equal(37))
113122
Expect(alr.StatusCode).To(Equal(http.StatusTeapot))
114123
Expect(alr.RouteEndpoint).To(Equal(testEndpoint))

handlers/reporter.go

+16
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,22 @@ func (rh *reporterHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request, ne
5757
requestInfo.RouteEndpoint, proxyWriter.Status(),
5858
requestInfo.ReceivedAt, requestInfo.AppRequestFinishedAt.Sub(requestInfo.ReceivedAt),
5959
)
60+
rh.calculateGorouterTime(requestInfo)
61+
rh.reporter.CaptureGorouterTime(requestInfo.GorouterTime)
62+
}
63+
64+
// calculateGorouterTime
65+
// calculate the gorouter time by subtracting app response time from the total roundtrip time.
66+
// Parameters:
67+
// - requestInfo *RequestInfo
68+
func (rh *reporterHandler) calculateGorouterTime(requestInfo *RequestInfo) {
69+
requestInfo.FinishedAt = time.Now()
70+
requestInfo.GorouterTime = -1
71+
appTime := requestInfo.AppRequestFinishedAt.Sub(requestInfo.AppRequestStartedAt).Seconds()
72+
rtTime := requestInfo.FinishedAt.Sub(requestInfo.ReceivedAt).Seconds()
73+
if rtTime >= 0 && appTime >= 0 {
74+
requestInfo.GorouterTime = rtTime - appTime
75+
}
6076
}
6177

6278
// validContentLength ensures that if the `Content-Length` header is set, it is not empty.

handlers/reporter_test.go

+11-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package handlers_test
22

33
import (
44
"bytes"
5+
"fmt"
56
"io"
67
"net/http"
78
"net/http/httptest"
@@ -51,7 +52,10 @@ var _ = Describe("Reporter Handler", func() {
5152
reqInfo, err := handlers.ContextRequestInfo(req)
5253
Expect(err).NotTo(HaveOccurred())
5354
reqInfo.RouteEndpoint = route.NewEndpoint(&route.EndpointOpts{AppId: "appID", PrivateInstanceIndex: "1", PrivateInstanceId: "id"})
54-
reqInfo.AppRequestFinishedAt = time.Now()
55+
timeNow := time.Now()
56+
reqInfo.ReceivedAt = timeNow.Add(-1 * time.Millisecond)
57+
reqInfo.AppRequestStartedAt = timeNow.Add(1 * time.Millisecond)
58+
reqInfo.AppRequestFinishedAt = timeNow.Add(2 * time.Millisecond)
5559

5660
nextCalled = true
5761
})
@@ -103,6 +107,12 @@ var _ = Describe("Reporter Handler", func() {
103107
Expect(latency).To(BeNumerically(">", 0))
104108
Expect(latency).To(BeNumerically("<", 10*time.Millisecond))
105109

110+
Expect(fakeReporter.CaptureGorouterTimeCallCount()).To(Equal(1))
111+
gorouterTime := fakeReporter.CaptureGorouterTimeArgsForCall(0)
112+
fmt.Println(gorouterTime)
113+
Expect(gorouterTime).To(BeNumerically(">", 0))
114+
Expect(gorouterTime).To(BeNumerically("<", 1*time.Millisecond))
115+
106116
Expect(nextCalled).To(BeTrue(), "Expected the next handler to be called.")
107117
})
108118

handlers/requestinfo.go

+2
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ type RequestInfo struct {
6363
// recorded before the access log, but we need the value to be able to
6464
// produce the log.
6565
FinishedAt time.Time
66+
// GorouterTime is calculated in the reporter
67+
GorouterTime float64
6668

6769
RoutePool *route.EndpointPool
6870
RouteEndpoint *route.Endpoint

metrics/compositereporter.go

+7
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type MetricReporter interface {
3030
CaptureRoutingRequest(b *route.Endpoint)
3131
CaptureRoutingResponse(statusCode int)
3232
CaptureRoutingResponseLatency(b *route.Endpoint, statusCode int, t time.Time, d time.Duration)
33+
CaptureGorouterTime(gorouterTime float64)
3334
CaptureRouteServiceResponse(res *http.Response)
3435
CaptureWebSocketUpdate()
3536
CaptureWebSocketFailure()
@@ -131,6 +132,12 @@ func (m MultiMetricReporter) CaptureRoutingResponseLatency(b *route.Endpoint, st
131132
}
132133
}
133134

135+
func (m MultiMetricReporter) CaptureGorouterTime(gorouterTime float64) {
136+
for _, r := range m {
137+
r.CaptureGorouterTime(gorouterTime)
138+
}
139+
}
140+
134141
func (m MultiMetricReporter) CaptureWebSocketUpdate() {
135142
for _, r := range m {
136143
r.CaptureWebSocketUpdate()

metrics/compositereporter_test.go

+8
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,14 @@ var _ = Describe("CompositeReporter", func() {
9191
Expect(callDuration).To(Equal(responseDuration))
9292
})
9393

94+
It("forwards CaptureGorouterTime to Multireporter", func() {
95+
composite.CaptureGorouterTime(3000)
96+
97+
Expect(fakeMultiReporter.CaptureGorouterTimeCallCount()).To(Equal(1))
98+
value := fakeMultiReporter.CaptureGorouterTimeArgsForCall(0)
99+
Expect(value).To(BeEquivalentTo(3000))
100+
})
101+
94102
It("forwards CaptureRoutingServiceResponse to proxy reporter", func() {
95103
composite.CaptureRouteServiceResponse(response)
96104

metrics/fakes/fake_metricreporter.go

+39
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

metrics/metricsreporter.go

+10
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,16 @@ func (m *Metrics) CaptureRoutingResponse(statusCode int) {
7777
m.Batcher.BatchIncrementCounter("responses")
7878
}
7979

80+
func (m *Metrics) CaptureGorouterTime(gorouterTime float64) {
81+
if m.PerRequestMetricsReporting {
82+
unit := "ms"
83+
err := m.Sender.SendValue("gorouter_time", gorouterTime*1000, unit)
84+
if err != nil {
85+
m.Logger.Debug("failed-sending-metric", log.ErrAttr(err), slog.String("metric", "gorouter_time"))
86+
}
87+
}
88+
}
89+
8090
func (m *Metrics) CaptureRoutingResponseLatency(b *route.Endpoint, _ int, _ time.Time, d time.Duration) {
8191
if m.PerRequestMetricsReporting {
8292
//this function has extra arguments to match varz reporter

metrics/metricsreporter_test.go

+19
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,25 @@ var _ = Describe("MetricsReporter", func() {
439439
Expect(sender.SendValueCallCount()).To(Equal(0))
440440
})
441441

442+
It("sends the gorouter time", func() {
443+
metricReporter.CaptureGorouterTime(3)
444+
445+
Expect(sender.SendValueCallCount()).To(Equal(1))
446+
name, value, unit := sender.SendValueArgsForCall(0)
447+
Expect(name).To(Equal("gorouter_time"))
448+
Expect(value).To(BeEquivalentTo(3000))
449+
Expect(unit).To(Equal("ms"))
450+
451+
})
452+
453+
It("does not send the goroutertime if switched off", func() {
454+
metricReporter.PerRequestMetricsReporting = false
455+
metricReporter.CaptureGorouterTime(3)
456+
457+
Expect(sender.SendValueCallCount()).To(Equal(0))
458+
459+
})
460+
442461
Context("sends route metrics", func() {
443462
var endpoint *route.Endpoint
444463

0 commit comments

Comments
 (0)