Skip to content
This repository has been archived by the owner on Aug 24, 2022. It is now read-only.

PMM-5680 store logs #342

Open
wants to merge 70 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 66 commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
167cbf3
Pmm-5680 store logs
qwest812 Mar 20, 2022
29764cc
Pmm-5680 store logs
qwest812 Mar 23, 2022
c8d2f68
Merge branch 'main' into PMM-5680-store-logs
qwest812 Mar 26, 2022
b673eaa
Pmm-5680 store logs
qwest812 Mar 27, 2022
9eda60e
Pmm-5680 store logs
qwest812 Mar 27, 2022
a6faa0b
Pmm-5680 store logs
qwest812 Apr 2, 2022
6f61509
Pmm-5680 store logs
qwest812 Apr 2, 2022
20a3ee2
Pmm-5680 store logs
qwest812 Apr 2, 2022
229845d
Pmm-5680 save logs process Done
qwest812 Apr 2, 2022
9161345
Pmm-5680 save logs process
qwest812 Apr 6, 2022
aa77af2
Pmm-5680 change dependency
qwest812 Apr 14, 2022
c6281e4
Merge branch 'main' into PMM-5680-store-logs
qwest812 Apr 14, 2022
5163ca9
Pmm-5680 update dependency
qwest812 Apr 14, 2022
e35cbc4
Pmm-5680 add mutex
qwest812 Apr 14, 2022
89a98da
Pmm-5680 fix tests
qwest812 Apr 14, 2022
989bb4f
Pmm-5680 fix tests
qwest812 Apr 14, 2022
9169b92
Pmm-5680 fix tests
qwest812 Apr 14, 2022
8562fbc
Pmm-5680 fix tests
qwest812 Apr 14, 2022
f0b2d6a
Pmm-5680 change process_child.go
qwest812 Apr 14, 2022
7a4863a
Pmm-5680 fix process_child.go
qwest812 Apr 14, 2022
c20a64c
Pmm-5680 fix mysql_show_table_status_action_test.go
qwest812 Apr 14, 2022
f2d3a40
Pmm-5680 gen
qwest812 Apr 14, 2022
50ebcaf
Pmm-5680 fix
qwest812 Apr 14, 2022
a0a6d44
Pmm-5680 update dependency
qwest812 Apr 14, 2022
a4bffaf
Pmm-5680 change name for logs
qwest812 Apr 14, 2022
fb64846
Pmm-5680 fix git actions
qwest812 Apr 15, 2022
fdc9f1a
Pmm-5680 fix git actions
qwest812 Apr 15, 2022
60dc79d
Pmm-5680 fix test
qwest812 Apr 15, 2022
8312f49
Pmm-5680 fix test
qwest812 Apr 15, 2022
3daddce
Pmm-5680 fix test
qwest812 Apr 15, 2022
dfc1597
Pmm-5680 fix lint
qwest812 Apr 16, 2022
cd38bd3
PMM-5680 Improvements
BupycHuk Apr 20, 2022
9ff8dc0
PMM-5680 Improvements
BupycHuk Apr 20, 2022
221a0b9
Pmm-5680 update storelogs
qwest812 Apr 20, 2022
729e3f1
Pmm-5680 remove function countLogs
qwest812 Apr 21, 2022
b24f6b5
Merge branch 'main' into PMM-5680-store-logs
qwest812 Apr 26, 2022
cb9c6b1
Merge branch 'PMM-5680-code-improvement-suggestions' into PMM-5680-st…
qwest812 Apr 26, 2022
7bd784b
Pmm-85680 update dependency
qwest812 Apr 27, 2022
b873ecc
Merge branch 'main' into PMM-5680-store-logs
qwest812 May 2, 2022
c53f291
Pmm-5680 response zipfile
qwest812 May 2, 2022
847932f
Pmm-5680 response zipfile
qwest812 May 2, 2022
77c8c83
Pmm-5680 fix mistake
qwest812 May 3, 2022
c686b5e
Pmm-5680 fix test
qwest812 May 3, 2022
c0bb005
Pmm-5680 fix test
qwest812 May 3, 2022
a709904
Pmm-5680 fix test
qwest812 May 3, 2022
e1da1ac
Pmm-5680 pretty
qwest812 May 3, 2022
5202e83
Pmm-5680 fix bug
qwest812 May 3, 2022
e44958a
Pmm-5680 fix bugs
qwest812 May 4, 2022
1657e70
Pmm-5680 change JSON to text
qwest812 May 9, 2022
c18be41
Pmm-5680 revert test
qwest812 May 9, 2022
1d77203
Pmm-5680 store serve logs
qwest812 May 9, 2022
c4151d3
Merge branch 'main' into PMM-5680-store-logs
qwest812 May 10, 2022
858e68c
Pmm-5680 store serve logs
qwest812 May 10, 2022
611c10e
Pmm-5680 fix test
qwest812 May 10, 2022
49cc0e6
Pmm-5680 remove struct AgentLogs
qwest812 May 10, 2022
60a0db6
Pmm-5680 remove id from map key
qwest812 May 10, 2022
ed5a7ce
Pmm-5680 set const number logs
qwest812 May 10, 2022
9d688bb
Pmm-5680 change function AgentsLogs
qwest812 May 10, 2022
545883a
Pmm-5680 remove comment
qwest812 May 11, 2022
96f10b0
Pmm-5680 fix linn err
qwest812 May 11, 2022
9880641
Pmm-5680 add small changes
qwest812 May 11, 2022
60e10a4
Pmm-5680 create test
qwest812 May 16, 2022
a0b10af
Pmm-5680 фвв еуые яшз ашду
qwest812 May 19, 2022
729bc50
Merge branch 'main' into PMM-5680-store-logs
qwest812 May 19, 2022
ecbac3d
Pmm-5680 make gen
qwest812 May 19, 2022
592965b
Pmm-5680 fix test
qwest812 May 20, 2022
f379d59
Pmm-5680 fix test
qwest812 May 25, 2022
29028f2
Pmm-5680 fix test
qwest812 May 25, 2022
f43cd91
Pmm-5680 fix test
qwest812 May 25, 2022
796c46d
pmm-5680 little changes
qwest812 Jun 1, 2022
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
62 changes: 59 additions & 3 deletions agentlocal/agent_local.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,13 @@
package agentlocal

import (
"archive/zip"
"bytes"
"context"
_ "expvar" // register /debug/vars
"fmt"
"html/template"
"io"
"log"
"net"
"net/http"
Expand Down Expand Up @@ -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 (
Expand All @@ -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.
//

Copy link
Member

Choose a reason for hiding this comment

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

let's revert it.

// 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,
}
}

Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci-lint] reported by reviewdog 🐶
exported method Server.Zip should have comment or be unexported (golint)

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())
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to rename it to pmm-agent.txt or http-server if it contains only http server logs.

Suggested change
addData(writer, "server.txt", b.Bytes())
addData(writer, "http-server.txt", b.Bytes())


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)
}
}
100 changes: 98 additions & 2 deletions agentlocal/agent_local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@
package agentlocal

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

Expand All @@ -28,6 +33,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) {
Expand Down Expand Up @@ -61,7 +67,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})
Expand All @@ -87,7 +94,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})
Expand All @@ -108,3 +116,91 @@ func TestServerStatus(t *testing.T) {
assert.Equal(t, expected, actual)
})
}

func TestGetZipFile(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci-lint] reported by reviewdog 🐶
Function TestGetZipFile missing the call to method parallel (paralleltest)

setup := func(t *testing.T) ([]*agentlocalpb.AgentInfo, *mockSupervisor, *mockClient, *config.Config) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci-lint] reported by reviewdog 🐶
test helper function should start from t.Helper() (thelper)

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)
// fmt.Sprintf("%v %v", actual, err)
rec := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/logs.zip", nil)
s.Zip(rec, req)
b, err := ioutil.ReadAll(rec.Body)
require.NoError(t, err)

expectedFile, err := generateTestZip(s)
Copy link
Contributor

Choose a reason for hiding this comment

The 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.
I think it would be better to unzip received file, and verify content.
Here is example of equality check failing:
image

The failure message is impossible to understand, because it is a diff between two binary files

require.NoError(t, err)
assert.Equal(t, expectedFile, b)
})
}

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
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci-lint] reported by reviewdog 🐶
error returned from external package is unwrapped: sig: func (*bytes.Buffer).WriteString(s string) (n int, err error) (wrapcheck)

}
}
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
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci-lint] reported by reviewdog 🐶
error returned from external package is unwrapped: sig: func (*bytes.Buffer).WriteString(s string) (n int, err error) (wrapcheck)

}
}
addData(writer, fmt.Sprintf("%s.txt", id), b.Bytes())
}
err := writer.Close()
if err != nil {
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci-lint] reported by reviewdog 🐶
error returned from external package is unwrapped: sig: func (*archive/zip.Writer).Close() error (wrapcheck)

}
return buf.Bytes(), nil
}
1 change: 1 addition & 0 deletions agentlocal/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@ type client interface {
// We use it instead of real type for testing and to avoid dependency cycle.
type supervisor interface {
AgentsList() []*agentlocalpb.AgentInfo
AgentsLogs() map[string][]string
}
16 changes: 16 additions & 0 deletions agentlocal/mock_supervisor_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions agents/process/process_child.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"context"
"flag"
"fmt"
"io"
"io/ioutil"
"time"

"github.com/percona/pmm/api/inventorypb"
Expand All @@ -37,9 +37,8 @@ import (
func main() {
flag.Parse()
logger := logrus.New()
logger.SetOutput(io.Discard)
logger.SetOutput(ioutil.Discard)
l := logrus.NewEntry(logger)

p := process.New(&process.Params{Path: "sleep", Args: []string{"100500"}}, nil, l)
go p.Run(context.Background())

Expand Down
5 changes: 3 additions & 2 deletions agents/process/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package process

import (
"context"
"io/ioutil"
"os"
"os/exec"
"runtime"
Expand Down Expand Up @@ -123,7 +124,7 @@ func TestProcess(t *testing.T) {
})

t.Run("Killed", func(t *testing.T) {
f, err := os.CreateTemp("", "pmm-agent-process-test-noterm")
f, err := ioutil.TempFile("", "pmm-agent-process-test-noterm")
require.NoError(t, err)
require.NoError(t, f.Close())
defer func() {
Expand All @@ -146,7 +147,7 @@ func TestProcess(t *testing.T) {
t.Skip("Pdeathsig is implemented only on Linux")
}

f, err := os.CreateTemp("", "pmm-agent-process-test-child")
f, err := ioutil.TempFile("", "pmm-agent-process-test-child")
require.NoError(t, err)
require.NoError(t, f.Close())
defer func() {
Expand Down
Loading