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

PMM-5680 store agents logs #197

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

PMM-5680 store agents logs #197

wants to merge 15 commits into from

Conversation

qwest812
Copy link
Contributor

@qwest812 qwest812 commented Apr 2, 2022

@BupycHuk
Copy link
Member

@qwest812 please make PR is ready for review if it's ready

logrus.Debugf("%s", err)
b = []byte(err.Error())
}
b = append(b, '\n')

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 🐶
append only allowed to cuddle with appended value (wsl)

@qwest812 qwest812 mentioned this pull request Apr 14, 2022
2 tasks
@qwest812 qwest812 marked this pull request as ready for review April 14, 2022 21:20
b = []byte(err.Error())
}
b = append(b, '\n')
addData(zipW, "logs/"+pointer.GetString(agent.AgentType)+".json", now, bytes.NewReader(b))
Copy link
Member

@BupycHuk BupycHuk Apr 18, 2022

Choose a reason for hiding this comment

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

what is the reason to make it json?
I think it would be much easier to read if it would be just a text with some separator.
something like

agent id: /agent_id/xxxx1
agent type: mongodb_exporter
lorem ipsum log texts begin
...
lorem ipsum log texts
===============================================
agent id: /agent_id/xxxx2
agent type: mysqld_exporter
lorem ipsum log texts begin
...
lorem ipsum log texts

@ShashankSinha252 ShashankSinha252 self-requested a review April 21, 2022 10:45
Comment on lines 146 to 153
fileName := "pmm-admin-summary.zip"
err = downloadFile(fmt.Sprintf("http://%s:%d/logs.zip", agentlocal.Localhost, agentlocal.DefaultPMMAgentListenPort), fileName)
if err != nil {
logrus.Debugf("%s", err)
b = []byte(err.Error())
}
logrus.Info("pmm-admin-summary.zip created")

Copy link
Member

Choose a reason for hiding this comment

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

we are on a right track, as a next step let's include files from logs.zip into main zip file

Copy link
Member

Choose a reason for hiding this comment

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

you can find example of it in addServerData

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did it

@qwest812 qwest812 requested a review from BupycHuk May 4, 2022 09:44
Copy link
Member

@BupycHuk BupycHuk left a comment

Choose a reason for hiding this comment

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

LGTM, a few minor changes

@@ -145,6 +143,11 @@ func addClientData(ctx context.Context, zipW *zip.Writer) {

addData(zipW, "client/pmm-admin-version.txt", now, bytes.NewReader([]byte(version.FullInfo())))

err = downloadFile(zipW, fmt.Sprintf("http://%s:%d/logs.zip", agentlocal.Localhost, agentlocal.DefaultPMMAgentListenPort), "pmm-admin logs")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
err = downloadFile(zipW, fmt.Sprintf("http://%s:%d/logs.zip", agentlocal.Localhost, agentlocal.DefaultPMMAgentListenPort), "pmm-admin logs")
err = downloadFile(zipW, fmt.Sprintf("http://%s:%d/logs.zip", agentlocal.Localhost, agentlocal.DefaultPMMAgentListenPort), "pmm-agent")

@@ -145,6 +143,11 @@ func addClientData(ctx context.Context, zipW *zip.Writer) {

addData(zipW, "client/pmm-admin-version.txt", now, bytes.NewReader([]byte(version.FullInfo())))

err = downloadFile(zipW, fmt.Sprintf("http://%s:%d/logs.zip", agentlocal.Localhost, agentlocal.DefaultPMMAgentListenPort), "pmm-admin logs")
if err != nil {
logrus.Debugf("%s", err)
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 write it in warn level

@@ -220,6 +223,43 @@ func getURL(ctx context.Context, url string) ([]byte, error) {
return b, nil
}

// downloadFile download file to local destination
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// downloadFile download file to local destination
// downloadFile download file and includes into zip file

@@ -220,6 +223,43 @@ func getURL(ctx context.Context, url string) ([]byte, error) {
return b, nil
}

// downloadFile download file to local destination
func downloadFile(zipW *zip.Writer, url, fileName string) error {

Copy link
Member

Choose a reason for hiding this comment

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

please remove this empty line, it makes CI fail

if err != nil {
return err
}
defer response.Body.Close()
Copy link

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 return value of response.Body.Close is not checked (errcheck)

}
addData(zipW, path.Join(fileName, rf.Name), rf.Modified, rc)

rc.Close()
Copy link

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 return value of rc.Close is not checked (errcheck)

@@ -220,6 +223,42 @@ func getURL(ctx context.Context, url string) ([]byte, error) {
return b, nil
}

// downloadFile download file and includes into zip file
func downloadFile(zipW *zip.Writer, url, fileName string) error {
response, err := http.Get(url)
Copy link

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 🐶
G107: Potential HTTP request made with variable url (gosec)

logrus.Errorf("%s", err)
continue
}
addData(zipW, path.Join(fileName, rf.Name), rf.Modified, rc)
Copy link

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 🐶
G305: File traversal when extracting zip/tar archive (gosec)

}
defer response.Body.Close()

if response.StatusCode != 200 {
Copy link

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 🐶
mnd: Magic number: 200, in detected (gomnd)

rc, err := rf.Open()
if err != nil {
logrus.Errorf("%s", err)
continue
Copy link

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 🐶
continue with no blank line before (nlreturn)

commands/summary.go Show resolved Hide resolved
commands/summary.go Show resolved Hide resolved
func downloadFile(zipW *zip.Writer, url, fileName string) error {
response, err := http.Get(url)
if err != nil {
return err
Copy link

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 net/http.Get(url string) (resp *net/http.Response, err error) (wrapcheck)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants