Skip to content

Commit

Permalink
change: keep metric names consistent to routing pkg metrics
Browse files Browse the repository at this point in the history
Signed-off-by: Sandor Szücs <[email protected]>

Use `%q` instead of `%s` for strings with quotes (#2700)

Signed-off-by: Mustafa Abdelrahman <[email protected]>
  • Loading branch information
szuecs authored and MustafaSaber committed Oct 27, 2023
1 parent 0dd3bbc commit db6384f
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 27 deletions.
13 changes: 6 additions & 7 deletions routesrv/polling.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (p *poller) poll(wg *sync.WaitGroup) {
case err != nil:
log.WithError(err).Error(LogRoutesFetchingFailed)

p.metrics.IncCounter("routes_fetch_errors")
p.metrics.IncCounter("routes.fetch_errors")

span.SetTag("error", true)
span.LogKV(
Expand All @@ -71,7 +71,7 @@ func (p *poller) poll(wg *sync.WaitGroup) {
)
case routesCount == 0:
log.Error(LogRoutesEmpty)
p.metrics.IncCounter("routes_empty")
p.metrics.IncCounter("routes.empty")

span.SetTag("error", true)
span.LogKV(
Expand All @@ -84,15 +84,14 @@ func (p *poller) poll(wg *sync.WaitGroup) {
if initialized {
logger.Info(LogRoutesInitialized)
span.SetTag("routes.initialized", true)
p.setGaugeToCurrentTime("routes_initialized_timestamp")
p.setGaugeToCurrentTime("routes.initialized_timestamp")
}
if updated {
logger.Info(LogRoutesUpdated)
span.SetTag("routes.updated", true)
p.setGaugeToCurrentTime("routes_updated_timestamp")
p.metrics.UpdateGauge("routes_count", float64(routesCount))
p.metrics.UpdateGauge("routes_byte", float64(routesBytes))

p.setGaugeToCurrentTime("routes.updated_timestamp")
p.metrics.UpdateGauge("routes.total", float64(routesCount))
p.metrics.UpdateGauge("routes.byte", float64(routesBytes))
}
span.SetTag("routes.count", routesCount)
span.SetTag("routes.bytes", routesBytes)
Expand Down
5 changes: 5 additions & 0 deletions routesrv/routesrv.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@ func newShutdownFunc(rs *RouteServer) func(delay time.Duration) {

log.Infof("shutting down the server in %s...", delay)
time.Sleep(delay)
if rs.supportServer != nil {
if err := rs.supportServer.Shutdown(context.Background()); err != nil {
log.Error("unable to shut down the support server: ", err)
}
}
if err := rs.server.Shutdown(context.Background()); err != nil {
log.Error("unable to shut down the server: ", err)
}
Expand Down
57 changes: 37 additions & 20 deletions routesrv/shutdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ func findAddress() (string, error) {
func TestServerShutdownHTTP(t *testing.T) {
o := skipper.Options{
SourcePollTimeout: 500 * time.Millisecond,
SupportListener: ":9911",
}
const shutdownDelay = 1 * time.Second

Expand All @@ -34,7 +35,15 @@ func TestServerShutdownHTTP(t *testing.T) {
}

o.Address, o.WaitForHealthcheckInterval = address, shutdownDelay
baseUrl := "http://" + address
baseURL := "http://" + address
// test support listener
host, _, err := net.SplitHostPort(address)
if err != nil {
t.Fatal(err)
}
supportBaseURL := "http://" + host + o.SupportListener
testEndpoints := []string{baseURL + "/routes", supportBaseURL + "/metrics"}

rs, err := New(o)
if err != nil {
t.Fatalf("Failed to create a routesrv: %v", err)
Expand All @@ -50,12 +59,15 @@ func TestServerShutdownHTTP(t *testing.T) {
}
}()

rsp, err := http.DefaultClient.Get(baseUrl + "/metrics")
if err != nil {
t.Fatalf("Failed to get routes: %v", err)
}
if rsp.StatusCode != 200 {
t.Fatalf("Failed to get expected status code 200, got: %d", rsp.StatusCode)
time.Sleep(o.SourcePollTimeout * 2)
for _, u := range testEndpoints {
rsp, err := http.DefaultClient.Get(u)
if err != nil {
t.Fatalf("Failed to get %q: %v", u, err)
}
if rsp.StatusCode != 200 {
t.Fatalf("Failed to get expected status code 200 for %q, got: %d", u, rsp.StatusCode)
}
}

// initiate shutdown
Expand All @@ -64,23 +76,28 @@ func TestServerShutdownHTTP(t *testing.T) {
// test that we can fetch even within termination
time.Sleep(shutdownDelay / 2)

rsp, err = http.DefaultClient.Get(baseUrl + "/metrics")
if err != nil {
t.Fatalf("Failed to get routes after SIGTERM: %v", err)
}
if rsp.StatusCode != 200 {
t.Fatalf("Failed to get expected status code 200 after SIGTERM, got: %d", rsp.StatusCode)
for _, u := range testEndpoints {
rsp, err := http.DefaultClient.Get(u)
if err != nil {
t.Fatalf("Failed to get %q after SIGTERM: %v", u, err)
}
if rsp.StatusCode != 200 {
t.Fatalf("Failed to get expected status code 200 for %q after SIGTERM, got: %d", u, rsp.StatusCode)
}
}

// test that we get connection refused after shutdown
time.Sleep(shutdownDelay / 2)
_, err = http.DefaultClient.Get(baseUrl + "/metrics")
switch err {
case nil:
t.Fatal("Failed to get error as expected")
default:
if e := err.Error(); !strings.Contains(e, "refused") {
t.Fatalf("Failed to get connection refused, got: %s", e)

for _, u := range testEndpoints {
_, err = http.DefaultClient.Get(u)
switch err {
case nil:
t.Fatalf("Failed to get error as expected: %q", u)
default:
if e := err.Error(); !strings.Contains(e, "refused") {
t.Fatalf("Failed to get connection refused, got: %s", e)
}
}
}

Expand Down

0 comments on commit db6384f

Please sign in to comment.