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

Chore: Updates and Cleanup #89

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Chore: Updates and Cleanup #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9863366
chore: Update go to 1.22, update dependencies
hashworks Jul 3, 2024
6fefc26
chore: shellcheck build.sh
hashworks Jul 3, 2024
1702b8c
chore: go vet
hashworks Jul 3, 2024
22dd307
chore: Bump travis ci to 1.23.1
hashworks Sep 27, 2024
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
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ dist: focal
language: go

go:
- 1.17.4
- 1.23.1

before_script:
- sudo apt-get install varnish -y
Expand Down
4 changes: 1 addition & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ To aggregate all loaded VCLs into per-backend metric the following Prometheus [r

**One time setup**

This repot support go modules so out of `GOPATH` builds are supported. This makes development and buildings easier for go "novices".

You need go 1.11 or higher, otherwise you can keep using `GOPATH` based development ([see old README](https://github.com/jonnenauha/prometheus_varnish_exporter/blob/1.4.1/README.md#build)).
This repository uses go modules. This makes development and buildings easier for go "novices". You need go 1.22 or higher.

1. [Install latest go](https://golang.org/doc/install) or use OS repos `golang` package.

Expand Down
33 changes: 18 additions & 15 deletions build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#!/bin/bash
#!/bin/env bash
Copy link
Owner

Choose a reason for hiding this comment

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

so did you actually change any logic here or just run it through some auto formatter? i dont see the point, afaik the script was working fine.

Copy link
Author

Choose a reason for hiding this comment

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

Neither. Yes, /bin/bash will work fine on most systems, but in rare cases this path is not correct (NixOS comes to mind). /bin/env bash ensures it works across the board. It's just good practice.


set -e

if [ ! -e main.go ] ; then
if [ ! -e main.go ]; then
echo "Error: Script can only be ran on the root of the source tree"
exit 1
fi
Expand All @@ -23,44 +23,47 @@ if [ -z "$VERSION" ]; then
exit 1
fi

tar -cvzf ./bin/release/dashboards-$VERSION.tar.gz dashboards/* > /dev/null 2>&1
tar -cvzf "./bin/release/dashboards-${VERSION}.tar.gz" dashboards/* >/dev/null 2>&1

for goos in linux darwin windows freebsd openbsd netbsd ; do
for goos in linux darwin windows freebsd openbsd netbsd; do
# shellcheck disable=SC2043
for goarch in amd64; do
# path
file_versioned="prometheus_varnish_exporter-$VERSION.$goos-$goarch"
outdir="bin/build/$file_versioned"
path="$outdir/prometheus_varnish_exporter"
if [ $goos = windows ] ; then
if [ $goos = windows ]; then
path=$path.exe
fi

mkdir -p $outdir
cp LICENSE CHANGELOG.md README.md $outdir/
mkdir -p "$outdir"
cp LICENSE CHANGELOG.md README.md "$outdir/"

# build
echo -e "\nBuilding $goos/$goarch"
GOOS=$goos GOARCH=$goarch go build -o $path -ldflags "-X 'main.Version=$VERSION' -X 'main.VersionHash=$VERSION_HASH' -X 'main.VersionDate=$VERSION_DATE'"
echo " > `du -hc $path | awk 'NR==1{print $1;}'` `file $path`"
GOOS=$goos GOARCH=$goarch go build -o "$path" -ldflags "-X 'main.Version=$VERSION' -X 'main.VersionHash=$VERSION_HASH' -X 'main.VersionDate=$VERSION_DATE'"
echo " > $(du -hc "$path" | awk 'NR==1{print $1;}') $(file "$path")"

# compress (for unique filenames to github release files)
tar -C ./bin/build -cvzf ./bin/release/$file_versioned.tar.gz $file_versioned > /dev/null 2>&1
tar -C ./bin/build -cvzf "./bin/release/${file_versioned}.tar.gz" "$file_versioned" >/dev/null 2>&1
done
done

go env > .goenv
go env >.goenv
source .goenv
rm .goenv

echo -e "\nRelease done: $(./bin/build/prometheus_varnish_exporter-$VERSION.$GOOS-$GOARCH/prometheus_varnish_exporter --version)"
for goos in linux darwin windows freebsd openbsd netbsd ; do
# shellcheck disable=SC2153
echo -e "\nRelease done: $("./bin/build/prometheus_varnish_exporter-${VERSION}.${GOOS}-${GOARCH}/prometheus_varnish_exporter" --version)"
for goos in linux darwin windows freebsd openbsd netbsd; do
# shellcheck disable=SC2043
for goarch in amd64; do
file_versioned="prometheus_varnish_exporter-$VERSION.$goos-$goarch"
path=bin/release/$file_versioned.tar.gz
echo " > `du -hc $path | awk 'NR==1{print $1;}'` $path"
echo " > $(du -hc "$path" | awk 'NR==1{print $1;}') $path"
done
done

cd ./bin/release
shasum --algorithm 256 --binary ./* | sed -En "s/\*\.\/(.*)$/\1/p" > sha256sums.txt
shasum --algorithm 256 --binary ./* | sed -En "s/\*\.\/(.*)$/\1/p" >sha256sums.txt
cd ../..
15 changes: 13 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
module github.com/jonnenauha/prometheus_varnish_exporter

go 1.12
go 1.22
Copy link
Owner

@jonnenauha jonnenauha Sep 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why would go 1.22 be required? I get the version bump if deps require it, but im not using packages that need new APIs from later go releases.

Prom seems to require go 1.21 so we should target that. https://github.com/prometheus/client_golang/blob/main/go.mod#L3

Despite what the go.mod says here, whatever go version you have will be used to build the binary. Its not like if you have go 1.23 installed and build it, it's not going to be be like you built it with go 1.12. go mod tells you the minimum version you can use to build the code. So if you are using features released in later go versions you can enforce it here.

But I agree bumping the version up to 1.21 is sensible. It can probably enable some compiler features versus 1.12.

Copy link
Owner

Choose a reason for hiding this comment

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

One more note, the travis ci go version update is ofc importatnt as it pushes the offiicial releases to github. So that is great, I was more talking about the go.mod changes here.

Copy link
Author

Choose a reason for hiding this comment

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

That's just me bumping everything to latest, you are correct, we can set this to 1.21.

Copy link
Author

Choose a reason for hiding this comment

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

EDIT: Actually, since 1.23 has been released this is no longer correct. I wouldn't set 1.21 here since it is no longer supported:

Each major Go release is supported until there are two newer major releases.


require github.com/prometheus/client_golang v1.11.0
require github.com/prometheus/client_golang v1.19.1

require (
github.com/beorn7/perks v1.0.1 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/prometheus/client_model v0.6.1 // indirect
github.com/prometheus/common v0.55.0 // indirect
github.com/prometheus/procfs v0.15.1 // indirect
golang.org/x/sys v0.21.0 // indirect
google.golang.org/protobuf v1.34.2 // indirect
)
166 changes: 31 additions & 135 deletions go.sum
View file
Open in desktop

Large diffs are not rendered by default.

22 changes: 11 additions & 11 deletions prometheus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,25 +114,25 @@ type group struct {

var (
groups = []group{
group{name: "backend", prefixes: []string{
{name: "backend", prefixes: []string{
Copy link
Owner

Choose a reason for hiding this comment

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

what auto formatter are you using to remove the struct name here? i have never seen the official go fmt do this? I think this is an unnecessary change.

Copy link
Author

Choose a reason for hiding this comment

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

This is the result of gofmt -s, it "simplifies" code. Defining the struct type here is redundant.

"vbe.",
}},
group{name: "mempool", prefixes: []string{
{name: "mempool", prefixes: []string{
"mempool.",
}},
group{name: "lck", prefixes: []string{
{name: "lck", prefixes: []string{
"lck.",
}},
group{name: "sma", prefixes: []string{
{name: "sma", prefixes: []string{
"sma.",
}},
group{name: "smf", prefixes: []string{
{name: "smf", prefixes: []string{
"smf.",
}},
group{name: "mgt", prefixes: []string{
{name: "mgt", prefixes: []string{
"mgt.",
}},
group{name: "main", prefixes: []string{
{name: "main", prefixes: []string{
"main.",
}},
}
Expand All @@ -148,18 +148,18 @@ type grouping struct {

var (
fqGroupPrefixes = []*grouping{
&grouping{
{
prefix: "main_fetch",
total: "main_s_fetch",
desc: "Number of fetches",
},
&grouping{
{
newPrefix: "main_sessions",
prefix: "main_sess",
total: "main_s_sess",
desc: "Number of sessions",
},
&grouping{
{
newPrefix: "main_worker_threads",
prefix: "main_n_wrk",
total: "main_n_wrk",
Expand Down Expand Up @@ -229,7 +229,7 @@ func cleanBackendName(name string) string {
if strings.HasPrefix(name, "reload_") {
dot := strings.Index(name, ".")
if dot != -1 {
name = name[dot + 1:]
name = name[dot+1:]
Copy link
Owner

Choose a reason for hiding this comment

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

this certainly is not go fmt either right?

Copy link
Author

Choose a reason for hiding this comment

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

That's just standard go fmt.

$ git status
On branch master
Your branch is up to date with 'origin/master'.
$ go fmt ./prometheus.go
$ git diff
  diff --git a/prometheus.go b/prometheus.go
  index c54f642..533051e 100644
  --- a/prometheus.go
  +++ b/prometheus.go
  @@ -229,7 +229,7 @@ func cleanBackendName(name string) string {
      if strings.HasPrefix(name, "reload_") {
          dot := strings.Index(name, ".")
          if dot != -1 {
  -           name = name[dot + 1:]
  +           name = name[dot+1:]
          }
      }

}
}

Expand Down
4 changes: 2 additions & 2 deletions varnish.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func ScrapeVarnishFrom(buf []byte, ch chan<- prometheus.Metric) ([]byte, error)
)
flag, _ := stringProperty(data, "flag")

if value, ok := data["description"]; ok && vErr == nil {
if value, ok := data["description"]; ok {
if vDescription, ok = value.(string); !ok {
vErr = fmt.Errorf("%s description it not a string", vName)
}
Expand Down Expand Up @@ -227,7 +227,7 @@ func executeVarnishstat(varnishstatExe string, params ...string) (*bytes.Buffer,
// 'VBE.reload_20191014_091124_78599' as by varnishreload in 6+
func findMostRecentVbeReloadPrefix(countersJSON map[string]interface{}) string {
var mostRecentVbeReloadPrefix string
for vName, _ := range countersJSON {
for vName := range countersJSON {
// Checking only the required ".happy" stat
if strings.HasPrefix(vName, vbeReload) && strings.HasSuffix(vName, ".happy") {
dotAfterPrefixIndex := len(vbeReload) + strings.Index(vName[len(vbeReload):], ".")
Expand Down
22 changes: 11 additions & 11 deletions varnish_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,31 @@ var testFileVersions = []string{"3.0.5", "4.0.5", "4.1.1", "5.2.0", "6.0.0", "6.

func Test_VarnishVersion(t *testing.T) {
tests := map[string]*varnishVersion{
"varnishstat (varnish-6.5.1 revision 1dae23376bb5ea7a6b8e9e4b9ed95cdc9469fb64)": &varnishVersion{
"varnishstat (varnish-6.5.1 revision 1dae23376bb5ea7a6b8e9e4b9ed95cdc9469fb64)": {
Major: 6, Minor: 5, Patch: 1, Revision: "1dae23376bb5ea7a6b8e9e4b9ed95cdc9469fb64",
},
"varnishstat (varnish-6.0.0 revision a068361dff0d25a0d85cf82a6e5fdaf315e06a7d)": &varnishVersion{
"varnishstat (varnish-6.0.0 revision a068361dff0d25a0d85cf82a6e5fdaf315e06a7d)": {
Major: 6, Minor: 0, Patch: 0, Revision: "a068361dff0d25a0d85cf82a6e5fdaf315e06a7d",
},
"varnishstat (varnish-5.2.0 revision 4c4875cbf)": &varnishVersion{
"varnishstat (varnish-5.2.0 revision 4c4875cbf)": {
Major: 5, Minor: 2, Patch: 0, Revision: "4c4875cbf",
},
"varnishstat (varnish-4.1.10 revision 1d090c5a08f41c36562644bafcce9d3cb85d824f)": &varnishVersion{
"varnishstat (varnish-4.1.10 revision 1d090c5a08f41c36562644bafcce9d3cb85d824f)": {
Major: 4, Minor: 1, Patch: 10, Revision: "1d090c5a08f41c36562644bafcce9d3cb85d824f",
},
"varnishstat (varnish-4.1.0 revision 3041728)": &varnishVersion{
"varnishstat (varnish-4.1.0 revision 3041728)": {
Major: 4, Minor: 1, Patch: 0, Revision: "3041728",
},
"varnishstat (varnish-4 revision)": &varnishVersion{
"varnishstat (varnish-4 revision)": {
Major: 4, Minor: -1, Patch: -1,
},
"varnishstat (varnish-3.0.5 revision 1a89b1f)": &varnishVersion{
"varnishstat (varnish-3.0.5 revision 1a89b1f)": {
Major: 3, Minor: 0, Patch: 5, Revision: "1a89b1f",
},
"varnish 2.0": &varnishVersion{
"varnish 2.0": {
Major: 2, Minor: 0, Patch: -1,
},
"varnish 1": &varnishVersion{
"varnish 1": {
Major: 1, Minor: -1, Patch: -1,
},
}
Expand Down Expand Up @@ -114,7 +114,7 @@ func Test_VarnishBackendNames(t *testing.T) {
vIdentifier string
vErr error
)
if value, ok := data["description"]; ok && vErr == nil {
if value, ok := data["description"]; ok {
if vDescription, ok = value.(string); !ok {
vErr = fmt.Errorf("%s description it not a string", vName)
}
Expand Down Expand Up @@ -266,7 +266,7 @@ func (tc *testCollector) Describe(ch chan<- *prometheus.Desc) {
}

func (tc *testCollector) Collect(ch chan<- prometheus.Metric) {
buf, err := ioutil.ReadFile(tc.filepath)
buf, err := os.ReadFile(tc.filepath)
if err != nil {
tc.t.Fatal(err.Error())
}
Expand Down