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

fix makefile and log #1714

Merged
merged 3 commits into from
Jul 11, 2023
Merged

fix makefile and log #1714

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b28da8b
fix makefile and log
luky116 Jul 11, 2023
dc4dfdb
fix makefile and log
luky116 Jul 11, 2023
ec21257
fix makefile
luky116 Jul 11, 2023
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
5 changes: 4 additions & 1 deletion tools/pika_exporter/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# export PATH := $(PATH):$(GOPATH)/bin

UNAME := $(shell uname)
UNAME_M := $(shell uname -m)
# for mac
BRANCH := $(shell git branch | sed 's/* \(.*\)/\1/p')
# for Linux
Expand Down Expand Up @@ -65,8 +66,10 @@ all: build
build: deps
ifeq ($(UNAME), Linux)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to consider the situation of Linux and arm architecture?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o bin/$(PROJNAME)
else ifeq ($(UNAME), Darwin)
else ifeq ($(UNAME_M), x86_64)
CGO_ENABLED=0 GOOS=darwin GOARCH=arm64 go build -o bin/$(PROJNAME)
else ifeq ($(UNAME_M), arm64)
CGO_ENABLED=0 GOOS=darwin GOARCH=amd64 go build -o bin/$(PROJNAME)
endif

deps: generateVer
Copy link

Choose a reason for hiding this comment

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

The code patch you provided seems to handle the build process based on the operating system and architecture. Here's a brief review with some suggestions:

  1. Variable Naming: It's generally recommended to use lowercase letters for variable names in Makefiles. For example, consider changing UNAME to uname and UNAME_M to uname_m.

  2. Comment Clarity: The comment indicating the command for Linux in the build target can be clarified by mentioning that it's specifically for x86_64 architecture ("Linux x86_64").

  3. Error Handling: It's a good practice to include error handling in your code. In this case, if the operating system is neither Linux nor macOS, the code doesn't handle the situation. You might want to add an error message or fallback mechanism when the conditions are not met.

Apart from these suggestions, the code patch looks fine and should work as intended.

Copy link

Choose a reason for hiding this comment

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

The code patch includes a build target in a Makefile to handle cross-compilation for different operating systems and architectures. Here are some bug risks and improvement suggestions:

  1. Typo: In line 11, ARCH should be assigned the output of uname -m, not "uname -m".
  2. Typo: In line 14, BRANCH should use double backticks (``) instead of single quotes ('') as the sed command requires backticks.
  3. In line 65, there's a typo (arm6411) instead of arm64. Fixing that will ensure the correct architecture is used for building on Linux.
  4. Consider adding error handling for the shell commands. If any of the uname or git branch commands fail, it would be good to display an error message.

Apart from these minor issues, the code looks fine in terms of handling different operating systems (Linux and Darwin) and architectures (x86_64 and arm64). It sets the appropriate values for GOOS and GOARCH environment variables and executes the go build command accordingly.

Expand Down
2 changes: 1 addition & 1 deletion tools/pika_exporter/exporter/metrics/command_exec_count.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ var collectCommandExecCountMetrics = map[string]MetricConfig{
Parser: &regexParser{
name: "command_exec_count_command",
source: "commands_count",
reg: regexp.MustCompile(`(\r|\n)*(?P<command>[^:]+):(?P<count>[\d]*)`),
reg: regexp.MustCompile(`(\r|\n)*(?P<command>[^:]+):(?P<count>[\d]*)`),
Parser: &normalParser{},
},
},
Expand Down
5 changes: 3 additions & 2 deletions tools/pika_exporter/exporter/metrics/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func (p *timeParser) Parse(m MetricMeta, c Collector, opt ParseOption) {
} else {
t, err := convertTimeToUnix(v)
if err != nil {
log.Warnf("time is '0' and cannot be parsed", err)
log.Warnf("time is '0' and cannot be parsed", err)
}
metric.Value = float64(t)
}
Expand Down Expand Up @@ -264,7 +264,8 @@ func mustNewVersionConstraint(version string) *semver.Constraints {
func convertTimeToUnix(ts string) (int64, error) {
t, err := time.Parse(time.RFC3339, ts)
if err != nil {
return 0, err
log.Warnf("format time failed, ts: %d, err: %v", ts, err)
return 0, nil
}
return t.Unix(), nil
}
6 changes: 4 additions & 2 deletions tools/pika_exporter/exporter/parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package exporter

import (
"fmt"
"testing"

"github.com/Masterminds/semver"
"github.com/stretchr/testify/assert"

"github.com/OpenAtomFoundation/pika/tools/pika_exporter/exporter/metrics"
"github.com/OpenAtomFoundation/pika/tools/pika_exporter/exporter/test"
"github.com/stretchr/testify/assert"
"testing"
)

func mustNewVersionConstraint(version string) *semver.Constraints {
Expand Down
3 changes: 2 additions & 1 deletion tools/pika_exporter/exporter/pika.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import (

"github.com/OpenAtomFoundation/pika/tools/pika_exporter/discovery"

"github.com/OpenAtomFoundation/pika/tools/pika_exporter/exporter/metrics"
"github.com/prometheus/client_golang/prometheus"
log "github.com/sirupsen/logrus"

"github.com/OpenAtomFoundation/pika/tools/pika_exporter/exporter/metrics"
)

type dbKeyPair struct {
Expand Down
6 changes: 4 additions & 2 deletions tools/pika_exporter/exporter/pika_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package exporter

import (
"github.com/OpenAtomFoundation/pika/tools/pika_exporter/discovery"
"testing"

"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/assert"
"testing"

"github.com/OpenAtomFoundation/pika/tools/pika_exporter/discovery"
)

type fakeDiscovery struct {
Expand Down
2 changes: 1 addition & 1 deletion tools/pika_exporter/exporter/test/v3.3.5_master.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,4 @@ db0 Strings_keys=83922112, expires=0, invalid_keys=0
db0 Hashes_keys=4887344, expires=0, invalid_keys=0
db0 Lists_keys=1, expires=0, invalid_keys=0
db0 Zsets_keys=1, expires=0, invalid_keys=0
db0 Sets_keys=0, expires=0, invalid_keys=1`
db0 Sets_keys=0, expires=0, invalid_keys=1`
2 changes: 1 addition & 1 deletion tools/pika_exporter/exporter/test/v3.3.5_slave.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,4 @@ db0 Strings_keys=1000, expires=0, invalid_keys=0
db0 Hashes_keys=0, expires=0, invalid_keys=0
db0 Lists_keys=0, expires=0, invalid_keys=0
db0 Zsets_keys=0, expires=100, invalid_keys=0
db0 Sets_keys=0, expires=0, invalid_keys=0`
db0 Sets_keys=0, expires=0, invalid_keys=0`
2 changes: 1 addition & 1 deletion tools/pika_exporter/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ require (
github.com/prometheus/common v0.42.0 // indirect
github.com/prometheus/procfs v0.9.0 // indirect
github.com/rogpeppe/go-internal v1.10.0 // indirect
golang.org/x/sys v0.6.0 // indirect
golang.org/x/sys v0.10.0 // indirect
google.golang.org/protobuf v1.30.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
4 changes: 2 additions & 2 deletions tools/pika_exporter/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ
github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ=
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA=
golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
Expand Down
15 changes: 12 additions & 3 deletions tools/pika_exporter/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http"
"os"
"strconv"
"time"

"github.com/OpenAtomFoundation/pika/tools/pika_exporter/discovery"
"github.com/OpenAtomFoundation/pika/tools/pika_exporter/exporter"
Expand All @@ -28,7 +29,7 @@ var (
listenAddress = flag.String("web.listen-address", getEnv("PIKA_EXPORTER_WEB_LISTEN_ADDRESS", ":9121"), "Address to listen on for web interface and telemetry.")
metricPath = flag.String("web.telemetry-path", getEnv("PIKA_EXPORTER_WEB_TELEMETRY_PATH", "/metrics"), "Path under which to expose metrics.")
logLevel = flag.String("log.level", getEnv("PIKA_EXPORTER_LOG_LEVEL", "info"), "Log level, valid options: panic fatal error warn warning info debug.")
logFormat = flag.String("log.format", getEnv("PIKA_EXPORTER_LOG_FORMAT", "json"), "Log format, valid options: txt and json.")
logFormat = flag.String("log.format", getEnv("PIKA_EXPORTER_LOG_FORMAT", "text"), "Log format, valid options: txt and json.")
showVersion = flag.Bool("version", false, "Show version information and exit.")
)

Expand Down Expand Up @@ -63,9 +64,17 @@ func main() {
log.SetLevel(level)
switch *logFormat {
case "json":
log.SetFormatter(&log.JSONFormatter{})
log.SetFormatter(&log.JSONFormatter{
TimestampFormat: "2006-01-02 15:04:05.999999",
PrettyPrint: true,
})
default:
log.SetFormatter(&log.TextFormatter{})
log.SetFormatter(&log.TextFormatter{
ForceColors: true,
ForceQuote: true, //键值对加引号
TimestampFormat: time.RFC3339, //时间格式
FullTimestamp: true,
})
}

var dis discovery.Discovery
Expand Down