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

📊 add refresh period to monitor mw #1898

Merged
merged 1 commit into from May 16, 2022
Merged

📊 add refresh period to monitor mw #1898

Show file tree
Hide file tree
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions middleware/monitor/README.md
View file
Open in desktop
Expand Up @@ -48,6 +48,11 @@ type Config struct {
// Optional. Default: "Fiber Monitor"
Title string

// Refresh period
//
// Optional. Default: 3 seconds
Refresh time.Duration

// To disable serving HTML, you can make true this option.
//
// Optional. Default: false
Expand All @@ -65,6 +70,7 @@ type Config struct {
```go
var ConfigDefault = Config{
Title: "Fiber Monitor",
Refresh: 3 * time.Second,
APIOnly: false,
Next: nil,
}
Expand Down
52 changes: 31 additions & 21 deletions middleware/monitor/config.go
View file
Open in desktop
@@ -1,7 +1,7 @@
package monitor

import (
"strings"
"time"

"github.com/gofiber/fiber/v2"
)
Expand All @@ -13,6 +13,11 @@ type Config struct {
// Optional. Default: "Fiber Monitor"
Title string

// Refresh period
//
// Optional. Default: 3 seconds
Refresh time.Duration

// Whether the service should expose only the monitoring API.
//
// Optional. Default: false
Expand All @@ -23,33 +28,28 @@ type Config struct {
// Optional. Default: nil
Next func(c *fiber.Ctx) bool

// defaultTitle substituted with Title in indexHtml
// customized indexHtml
index string
}

var prevTitle = defaultTitle

var ConfigDefault = Config{
Title: defaultTitle,
Refresh: defaultRefresh,
APIOnly: false,
Next: nil,
index: indexHtml,
}

func newIndex(title string) string {
if title == defaultTitle {
return indexHtml
}
return strings.ReplaceAll(indexHtml, defaultTitle, title)
index: newIndex(defaultTitle, defaultRefresh),
}

func configDefault(config ...Config) Config {
// Users can change ConfigDefault.Title which then
// becomes incompatible with ConfigDefault.index
if prevTitle != ConfigDefault.Title {
prevTitle = ConfigDefault.Title
// update default index with new default title
ConfigDefault.index = newIndex(prevTitle)
// Users can change ConfigDefault.Title/Refresh which then
// become incompatible with ConfigDefault.index
if ConfigDefault.Title != defaultTitle || ConfigDefault.Refresh != defaultRefresh {

if ConfigDefault.Refresh < minRefresh {
ConfigDefault.Refresh = minRefresh
}
// update default index with new default title/refresh
ConfigDefault.index = newIndex(ConfigDefault.Title, ConfigDefault.Refresh)
Copy link
Member

Choose a reason for hiding this comment

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

why don't we do this below when refresh and title has been reset or contains the default values ?

Copy link
Contributor Author

@jfcg jfcg May 12, 2022 •
edited

Choose a reason for hiding this comment

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

If a user changes default config only, like:

monitor.ConfigDefault.Title = "New Title"
app.Get("/metrics", monitor.New())

then these lines will return default config directly:

	// Return default config if nothing provided
	if len(config) < 1 {
		return ConfigDefault
	}

so default config must be prepared beforehand 😉

Copy link
Member

Choose a reason for hiding this comment

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

Why should the users do that?
Then they have not understood the concept or we should not expose the default config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we can unexport default config, that would be easier but I think that decision should be considered for the whole project, not just for monitor mw.

Note that if something is exported to users, then it is part of the API and meant to be usable/modifiable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though there does not seem to be many pablic uses of monitor.DefaultConfig, as per Github search.

}

// Return default config if nothing provided
Expand All @@ -61,12 +61,22 @@ func configDefault(config ...Config) Config {
cfg := config[0]

// Set default values
if cfg.Title == "" || cfg.Title == ConfigDefault.Title {
if cfg.Title == "" {
cfg.Title = ConfigDefault.Title
}

if cfg.Refresh == 0 {
cfg.Refresh = ConfigDefault.Refresh
}

if cfg.Title == ConfigDefault.Title && cfg.Refresh == ConfigDefault.Refresh {
cfg.index = ConfigDefault.index
} else {
// update cfg.index with new title
cfg.index = newIndex(cfg.Title)
if cfg.Refresh < minRefresh {
cfg.Refresh = minRefresh
}
// update cfg.index with custom title/refresh
cfg.index = newIndex(cfg.Title, cfg.Refresh)
}

if cfg.Next == nil {
Expand Down
76 changes: 39 additions & 37 deletions middleware/monitor/index.go
View file
Open in desktop
@@ -1,16 +1,40 @@
package monitor

import (
"strconv"
"strings"
"time"
)

// returns index with new title/refresh
func newIndex(title string, refresh time.Duration) string {

timeout := refresh.Milliseconds() - timeoutDiff
if timeout < timeoutDiff {
timeout = timeoutDiff
}
ts := strconv.FormatInt(timeout, 10)

index := strings.ReplaceAll(indexHtml, "$TITLE", title)
return strings.ReplaceAll(index, "$TIMEOUT", ts)
}

const (
defaultTitle = "Fiber Monitor"

defaultRefresh = 3 * time.Second
timeoutDiff = 200 // timeout will be Refresh (in millisconds) - timeoutDiff
minRefresh = timeoutDiff * time.Millisecond

// parametrized by $TITLE and $TIMEOUT
indexHtml = `<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link href="https://fonts.googleapis.com/css2?family=Roboto:wght@400;900&display=swap" rel="stylesheet">
<script src="https://cdn.jsdelivr.net/npm/chart.js@2.9/dist/Chart.bundle.min.js"></script>
<title>` + defaultTitle + `</title>
<title>$TITLE</title>
<style>
body {
margin: 0;
Expand All @@ -35,12 +59,8 @@ const (
margin-bottom: 20px;
align-items: center;
}
.row .column:first-child {
width: 35%;
}
.row .column:last-child {
width: 65%;
}
.row .column:first-child { width: 35%; }
.row .column:last-child { width: 65%; }
.metric {
color: #777;
font-weight: 900;
Expand All @@ -54,12 +74,8 @@ const (
font-size: 12px;
color: #777;
}
h2 span.ram_os {
color: rgba(255, 150, 0, .8);
}
h2 span.ram_total {
color: rgba(0, 200, 0, .8);
}
h2 span.ram_os { color: rgba(255, 150, 0, .8); }
h2 span.ram_total { color: rgba(0, 200, 0, .8); }
canvas {
width: 200px;
height: 180px;
Expand All @@ -68,7 +84,7 @@ const (
</head>
<body>
<section class="wrapper">
<div class="title"><h1>` + defaultTitle + `</h1></div>
<div class="title"><h1>$TITLE</h1></div>
<section class="charts">
<div class="row">
<div class="column">
Expand Down Expand Up @@ -130,25 +146,17 @@ const (

const options = {
scales: {
yAxes: [{
ticks: {
beginAtZero: true
}
}],
yAxes: [{ ticks: { beginAtZero: true }}],
xAxes: [{
type: 'time',
time: {
unitStepSize: 30,
unit: 'second'
},
gridlines: {
display: false
}
gridlines: { display: false }
}]
},
tooltips: {
enabled: false
},
tooltips: { enabled: false },
responsive: true,
maintainAspectRatio: false,
animation: false
Expand Down Expand Up @@ -204,7 +212,8 @@ const (
cpuOS = json.os.cpu.toFixed(1);

cpuMetric.innerHTML = cpu + '% <span>' + cpuOS + '%</span>';
ramMetric.innerHTML = formatBytes(json.pid.ram) + '<span> / </span><span class="ram_os">' + formatBytes(json.os.ram) + '<span><span> / </span><span class="ram_total">' + formatBytes(json.os.total_ram) + '</span>';
ramMetric.innerHTML = formatBytes(json.pid.ram) + '<span> / </span><span class="ram_os">' + formatBytes(json.os.ram) +
'<span><span> / </span><span class="ram_total">' + formatBytes(json.os.total_ram) + '</span>';
rtimeMetric.innerHTML = rtime + 'ms <span>client</span>';
connsMetric.innerHTML = json.pid.conns + ' <span>' + json.os.conns + '</span>';

Expand All @@ -219,33 +228,26 @@ const (

charts.forEach(chart => {
if (chart.data.labels.length > 50) {
chart.data.datasets.forEach(function (dataset) {
dataset.data.shift();
});
chart.data.datasets.forEach(function (dataset) { dataset.data.shift(); });
chart.data.labels.shift();
}

chart.data.labels.push(timestamp);
chart.update();
});
setTimeout(fetchJSON, 750)
setTimeout(fetchJSON, $TIMEOUT)
}
function fetchJSON() {
var t1 = ''
var t0 = performance.now()
fetch(window.location.href, {
headers: {
'Accept': 'application/json'
},
headers: { 'Accept': 'application/json' },
credentials: 'same-origin'
})
.then(res => {
t1 = performance.now()
return res.json()
})
.then(res => {
update(res, Math.round(t1 - t0))
})
.then(res => { update(res, Math.round(t1 - t0)) })
.catch(console.error);
}
fetchJSON()
Expand Down
4 changes: 2 additions & 2 deletions middleware/monitor/monitor.go
View file
Open in desktop
Expand Up @@ -64,9 +64,9 @@ func New(config ...Config) fiber.Handler {

go func() {
for {
updateStatistics(p)
time.Sleep(cfg.Refresh)

time.Sleep(1 * time.Second)
updateStatistics(p)
}
}()
})
Expand Down
30 changes: 26 additions & 4 deletions middleware/monitor/monitor_test.go
View file
Open in desktop
Expand Up @@ -2,9 +2,11 @@ package monitor

import (
"bytes"
"fmt"
"io/ioutil"
"net/http/httptest"
"testing"
"time"

"github.com/gofiber/fiber/v2"
"github.com/gofiber/fiber/v2/utils"
Expand All @@ -28,16 +30,36 @@ func Test_Monitor_Html(t *testing.T) {

app := fiber.New()

// defaults
app.Get("/", New())

resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil))

utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, 200, resp.StatusCode)
utils.AssertEqual(t, fiber.MIMETextHTMLCharsetUTF8, resp.Header.Get(fiber.HeaderContentType))
utils.AssertEqual(t, fiber.MIMETextHTMLCharsetUTF8,
resp.Header.Get(fiber.HeaderContentType))
buf, err := ioutil.ReadAll(resp.Body)
utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, true, bytes.Contains(buf, []byte("<title>"+defaultTitle+"</title>")))
timeoutLine := fmt.Sprintf("setTimeout(fetchJSON, %d)",
Copy link
Member

Choose a reason for hiding this comment

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

please also create a test where the setting of the configuration and its modification is checked

Copy link
Contributor Author

Choose a reason for hiding this comment

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

extended Test_Monitor_Html to also check custom config.

defaultRefresh.Milliseconds()-timeoutDiff)
utils.AssertEqual(t, true, bytes.Contains(buf, []byte(timeoutLine)))

b, err := ioutil.ReadAll(resp.Body)
// custom config
conf := Config{Title: "New " + defaultTitle, Refresh: defaultRefresh + time.Second}
app.Get("/custom", New(conf))
resp, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/custom", nil))

utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, 200, resp.StatusCode)
utils.AssertEqual(t, fiber.MIMETextHTMLCharsetUTF8,
resp.Header.Get(fiber.HeaderContentType))
buf, err = ioutil.ReadAll(resp.Body)
utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, true, bytes.Contains(b, []byte("<title>"+defaultTitle+"</title>")))
utils.AssertEqual(t, true, bytes.Contains(buf, []byte("<title>"+conf.Title+"</title>")))
timeoutLine = fmt.Sprintf("setTimeout(fetchJSON, %d)",
conf.Refresh.Milliseconds()-timeoutDiff)
utils.AssertEqual(t, true, bytes.Contains(buf, []byte(timeoutLine)))
}

// go test -run Test_Monitor_JSON -race
Expand Down