-
Notifications
You must be signed in to change notification settings - Fork 21
PMM-5680 store logs #342
base: main
Are you sure you want to change the base?
PMM-5680 store logs #342
Changes from 69 commits
167cbf3
29764cc
c8d2f68
b673eaa
9eda60e
a6faa0b
6f61509
20a3ee2
229845d
9161345
aa77af2
c6281e4
5163ca9
e35cbc4
89a98da
989bb4f
9169b92
8562fbc
f0b2d6a
7a4863a
c20a64c
f2d3a40
50ebcaf
a0a6d44
a4bffaf
fb64846
fdc9f1a
60dc79d
8312f49
3daddce
dfc1597
cd38bd3
9ff8dc0
221a0b9
729e3f1
b24f6b5
cb9c6b1
7bd784b
b873ecc
c53f291
847932f
77c8c83
c686b5e
c0bb005
a709904
e1da1ac
5202e83
e44958a
1657e70
c18be41
1d77203
c4151d3
858e68c
611c10e
49cc0e6
60a0db6
ed5a7ce
9d688bb
545883a
96f10b0
9880641
60e10a4
a0b10af
729bc50
ecbac3d
592965b
f379d59
29028f2
f43cd91
796c46d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -16,10 +16,13 @@ | |||||
package agentlocal | ||||||
|
||||||
import ( | ||||||
"archive/zip" | ||||||
"bytes" | ||||||
"context" | ||||||
_ "expvar" // register /debug/vars | ||||||
"fmt" | ||||||
"html/template" | ||||||
"io" | ||||||
"log" | ||||||
"net" | ||||||
"net/http" | ||||||
|
@@ -47,6 +50,7 @@ import ( | |||||
"google.golang.org/protobuf/types/known/durationpb" | ||||||
|
||||||
"github.com/percona/pmm-agent/config" | ||||||
"github.com/percona/pmm-agent/storelogs" | ||||||
) | ||||||
|
||||||
const ( | ||||||
|
@@ -61,23 +65,28 @@ type Server struct { | |||||
configFilepath string | ||||||
|
||||||
l *logrus.Entry | ||||||
ringLogs *storelogs.LogsStore | ||||||
reload chan struct{} | ||||||
reloadCloseOnce sync.Once | ||||||
|
||||||
agentlocalpb.UnimplementedAgentLocalServer | ||||||
} | ||||||
|
||||||
// NewServer creates new server. | ||||||
// | ||||||
|
||||||
// Caller should call Run. | ||||||
func NewServer(cfg *config.Config, supervisor supervisor, client client, configFilepath string) *Server { | ||||||
func NewServer(cfg *config.Config, supervisor supervisor, client client, configFilepath string, ringLog *storelogs.LogsStore) *Server { | ||||||
logger := logrus.New() | ||||||
logger.Out = io.MultiWriter(os.Stderr, ringLog) | ||||||
|
||||||
return &Server{ | ||||||
cfg: cfg, | ||||||
supervisor: supervisor, | ||||||
client: client, | ||||||
configFilepath: configFilepath, | ||||||
l: logrus.WithField("component", "local-server"), | ||||||
l: logger.WithField("component", "local-server"), | ||||||
reload: make(chan struct{}), | ||||||
ringLogs: ringLog, | ||||||
} | ||||||
} | ||||||
|
||||||
|
@@ -289,6 +298,7 @@ func (s *Server) runJSONServer(ctx context.Context, grpcAddress string) { | |||||
mux.Handle("/debug/", http.DefaultServeMux) | ||||||
mux.Handle("/debug", debugPageHandler) | ||||||
mux.Handle("/", proxyMux) | ||||||
mux.HandleFunc("/logs.zip", s.Zip) | ||||||
|
||||||
server := &http.Server{ | ||||||
Addr: address, | ||||||
|
@@ -314,7 +324,53 @@ func (s *Server) runJSONServer(ctx context.Context, grpcAddress string) { | |||||
_ = server.Close() // call Close() in all cases | ||||||
} | ||||||
|
||||||
// addData add data to zip file | ||||||
func addData(zipW *zip.Writer, name string, data []byte) { | ||||||
f, err := zipW.Create(name) | ||||||
if err != nil { | ||||||
log.Fatal(err) | ||||||
} | ||||||
_, err = f.Write(data) | ||||||
if err != nil { | ||||||
log.Fatal(err) | ||||||
} | ||||||
} | ||||||
|
||||||
// check interfaces | ||||||
var ( | ||||||
_ agentlocalpb.AgentLocalServer = (*Server)(nil) | ||||||
) | ||||||
|
||||||
func (s *Server) Zip(w http.ResponseWriter, r *http.Request) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 [golangci-lint] reported by reviewdog 🐶 |
||||||
buf := &bytes.Buffer{} | ||||||
writer := zip.NewWriter(buf) | ||||||
b := &bytes.Buffer{} | ||||||
for _, serverLog := range s.ringLogs.GetLogs() { | ||||||
_, err := b.WriteString(serverLog) | ||||||
if err != nil { | ||||||
log.Fatal(err) | ||||||
} | ||||||
} | ||||||
addData(writer, "server.txt", b.Bytes()) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's better to rename it to
Suggested change
|
||||||
|
||||||
for id, logs := range s.supervisor.AgentsLogs() { | ||||||
b := &bytes.Buffer{} | ||||||
for _, l := range logs { | ||||||
_, err := b.WriteString(l + "\n") | ||||||
if err != nil { | ||||||
log.Fatal(err) | ||||||
} | ||||||
} | ||||||
addData(writer, fmt.Sprintf("%s.txt", id), b.Bytes()) | ||||||
} | ||||||
err := writer.Close() | ||||||
if err != nil { | ||||||
log.Fatal(err) | ||||||
} | ||||||
w.Header().Set("Content-Type", "application/zip") | ||||||
w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=\"%s.zip\"", "logs")) | ||||||
_, err = w.Write(buf.Bytes()) | ||||||
if err != nil { | ||||||
log.Fatal(err) | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,14 @@ | |
package agentlocal | ||
|
||
import ( | ||
"archive/zip" | ||
"bufio" | ||
"bytes" | ||
"context" | ||
"fmt" | ||
"io/ioutil" | ||
"log" | ||
"net/http/httptest" | ||
"testing" | ||
"time" | ||
|
||
|
@@ -28,6 +35,7 @@ import ( | |
"google.golang.org/protobuf/types/known/durationpb" | ||
|
||
"github.com/percona/pmm-agent/config" | ||
"github.com/percona/pmm-agent/storelogs" | ||
) | ||
|
||
func TestServerStatus(t *testing.T) { | ||
|
@@ -61,7 +69,8 @@ func TestServerStatus(t *testing.T) { | |
agentInfo, supervisor, client, cfg := setup(t) | ||
defer supervisor.AssertExpectations(t) | ||
defer client.AssertExpectations(t) | ||
s := NewServer(cfg, supervisor, client, "/some/dir/pmm-agent.yaml") | ||
ringLog := storelogs.New(500) | ||
s := NewServer(cfg, supervisor, client, "/some/dir/pmm-agent.yaml", ringLog) | ||
|
||
// without network info | ||
actual, err := s.Status(context.Background(), &agentlocalpb.StatusRequest{GetNetworkInfo: false}) | ||
|
@@ -87,7 +96,8 @@ func TestServerStatus(t *testing.T) { | |
client.On("GetNetworkInformation").Return(latency, clockDrift, nil) | ||
defer supervisor.AssertExpectations(t) | ||
defer client.AssertExpectations(t) | ||
s := NewServer(cfg, supervisor, client, "/some/dir/pmm-agent.yaml") | ||
ringLog := storelogs.New(500) | ||
s := NewServer(cfg, supervisor, client, "/some/dir/pmm-agent.yaml", ringLog) | ||
|
||
// with network info | ||
actual, err := s.Status(context.Background(), &agentlocalpb.StatusRequest{GetNetworkInfo: true}) | ||
|
@@ -108,3 +118,128 @@ func TestServerStatus(t *testing.T) { | |
assert.Equal(t, expected, actual) | ||
}) | ||
} | ||
|
||
func TestGetZipFile(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 [golangci-lint] reported by reviewdog 🐶 |
||
setup := func(t *testing.T) ([]*agentlocalpb.AgentInfo, *mockSupervisor, *mockClient, *config.Config) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 [golangci-lint] reported by reviewdog 🐶 |
||
agentInfo := []*agentlocalpb.AgentInfo{{ | ||
AgentId: "/agent_id/00000000-0000-4000-8000-000000000002", | ||
AgentType: inventorypb.AgentType_NODE_EXPORTER, | ||
Status: inventorypb.AgentStatus_RUNNING, | ||
}} | ||
var supervisor mockSupervisor | ||
supervisor.Test(t) | ||
supervisor.On("AgentsList").Return(agentInfo) | ||
agentLogs := make(map[string][]string) | ||
agentLogs[inventorypb.AgentType_NODE_EXPORTER.String()] = []string{ | ||
"logs1", | ||
"logs2", | ||
} | ||
supervisor.On("AgentsLogs").Return(agentLogs) | ||
var client mockClient | ||
client.Test(t) | ||
client.On("GetServerConnectMetadata").Return(&agentpb.ServerConnectMetadata{ | ||
AgentRunsOnNodeID: "/node_id/00000000-0000-4000-8000-000000000003", | ||
ServerVersion: "2.0.0-dev", | ||
}) | ||
cfg := &config.Config{ | ||
ID: "/agent_id/00000000-0000-4000-8000-000000000001", | ||
Server: config.Server{ | ||
Address: "127.0.0.1:8443", | ||
Username: "username", | ||
Password: "password", | ||
}, | ||
} | ||
return agentInfo, &supervisor, &client, cfg | ||
} | ||
|
||
t.Run("test zip file", func(t *testing.T) { | ||
_, supervisor, client, cfg := setup(t) | ||
defer supervisor.AssertExpectations(t) | ||
defer client.AssertExpectations(t) | ||
ringLog := storelogs.New(10) | ||
s := NewServer(cfg, supervisor, client, "/some/dir/pmm-agent.yaml", ringLog) | ||
_, err := s.Status(context.Background(), &agentlocalpb.StatusRequest{GetNetworkInfo: false}) | ||
require.NoError(t, err) | ||
|
||
rec := httptest.NewRecorder() | ||
req := httptest.NewRequest("GET", "/logs.zip", nil) | ||
s.Zip(rec, req) | ||
existFile, err := ioutil.ReadAll(rec.Body) | ||
require.NoError(t, err) | ||
|
||
expectedFile, err := generateTestZip(s) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are generating zip file that you will later compare with, but what if it fails? The output will be not readable. The failure message is impossible to understand, because it is a diff between two binary files |
||
require.NoError(t, err) | ||
bufExp := bytes.NewReader(expectedFile) | ||
bufExs := bytes.NewReader(existFile) | ||
|
||
zipExp, err := zip.NewReader(bufExp, bufExp.Size()) | ||
require.NoError(t, err) | ||
zipExs, err := zip.NewReader(bufExp, bufExs.Size()) | ||
require.NoError(t, err) | ||
|
||
for i, ex := range zipExp.File { | ||
assert.Equal(t, ex.Name, zipExs.File[i].Name) | ||
deepCompare(t, ex, zipExs.File[i]) | ||
} | ||
require.NoError(t, err) | ||
assert.Equal(t, expectedFile, existFile) | ||
}) | ||
} | ||
|
||
// generateTestZip generate test zip file. | ||
func generateTestZip(s *Server) ([]byte, error) { | ||
agentLogs := make(map[string][]string) | ||
agentLogs[inventorypb.AgentType_NODE_EXPORTER.String()] = []string{ | ||
"logs1", | ||
"logs2", | ||
} | ||
buf := &bytes.Buffer{} | ||
writer := zip.NewWriter(buf) | ||
b := &bytes.Buffer{} | ||
for _, serverLog := range s.ringLogs.GetLogs() { | ||
_, err := b.WriteString(serverLog) | ||
if err != nil { | ||
return nil, err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 [golangci-lint] reported by reviewdog 🐶 |
||
} | ||
} | ||
addData(writer, "server.txt", b.Bytes()) | ||
|
||
for id, logs := range agentLogs { | ||
b := &bytes.Buffer{} | ||
for _, l := range logs { | ||
_, err := b.WriteString(l + "\n") | ||
if err != nil { | ||
return nil, err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 [golangci-lint] reported by reviewdog 🐶 |
||
} | ||
} | ||
addData(writer, fmt.Sprintf("%s.txt", id), b.Bytes()) | ||
} | ||
err := writer.Close() | ||
if err != nil { | ||
return nil, err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 [golangci-lint] reported by reviewdog 🐶 |
||
} | ||
return buf.Bytes(), nil | ||
} | ||
|
||
// deepCompare compare two zip files. | ||
func deepCompare(t *testing.T, file1, file2 *zip.File) bool { | ||
sf, err := file1.Open() | ||
if err != nil { | ||
log.Fatal(err) | ||
} | ||
|
||
df, err := file2.Open() | ||
if err != nil { | ||
log.Fatal(err) | ||
} | ||
|
||
sscan := bufio.NewScanner(sf) | ||
dscan := bufio.NewScanner(df) | ||
|
||
for sscan.Scan() { | ||
dscan.Scan() | ||
assert.Equal(t, sscan.Bytes(), dscan.Bytes()) | ||
} | ||
|
||
return false | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's revert it.