From 5516063f18c267d058d2ac671d445b12f66b27c5 Mon Sep 17 00:00:00 2001 From: songzhibin97 <718428482@qq.com> Date: Sun, 15 May 2022 16:00:48 +0800 Subject: [PATCH 1/5] feat:#96 --- holmes.go | 49 +++++++++---------- options.go | 8 +-- reporters/http_reporter/http_reporter.go | 11 ++--- reporters/http_reporter/http_reporter_test.go | 3 +- reporters/reporter_test.go | 4 +- 5 files changed, 32 insertions(+), 43 deletions(-) diff --git a/holmes.go b/holmes.go index 2c8a119..e1e5e21 100644 --- a/holmes.go +++ b/holmes.go @@ -72,7 +72,7 @@ type Holmes struct { } type ProfileReporter interface { - Report(pType string, buf []byte, reason string, eventID string) error + Report(pType string, dumpName string, reason string, eventID string) error } // New creates a holmes dumper. @@ -374,9 +374,8 @@ func (h *Holmes) goroutineProfile(gNum int, c grOptions) bool { var buf bytes.Buffer _ = pprof.Lookup("goroutine").WriteTo(&buf, int(h.opts.DumpProfileType)) // nolint: errcheck - h.writeProfileDataToFile(buf, goroutine, "") - h.ReportProfile(type2name[goroutine], buf.Bytes(), reason, "") + h.ReportProfile(type2name[goroutine], h.writeProfileDataToFile(buf, goroutine, ""), reason, "") return true } @@ -417,9 +416,7 @@ func (h *Holmes) memProfile(rss int, c typeOption) bool { var buf bytes.Buffer _ = pprof.Lookup("heap").WriteTo(&buf, int(h.opts.DumpProfileType)) // nolint: errcheck - h.writeProfileDataToFile(buf, mem, "") - - h.ReportProfile(type2name[mem], buf.Bytes(), reason, "") + h.ReportProfile(type2name[mem], h.writeProfileDataToFile(buf, mem, ""), reason, "") return true } @@ -522,15 +519,13 @@ func (h *Holmes) threadProfile(curThreadNum int, c typeOption) bool { var buf bytes.Buffer _ = pprof.Lookup("threadcreate").WriteTo(&buf, int(h.opts.DumpProfileType)) // nolint: errcheck - h.writeProfileDataToFile(buf, thread, eventID) - h.ReportProfile(type2name[thread], buf.Bytes(), reason, eventID) + h.ReportProfile(type2name[thread], h.writeProfileDataToFile(buf, thread, eventID), reason, eventID) buf.Reset() _ = pprof.Lookup("goroutine").WriteTo(&buf, int(h.opts.DumpProfileType)) // nolint: errcheck - h.writeProfileDataToFile(buf, goroutine, eventID) - h.ReportProfile(type2name[goroutine], buf.Bytes(), reason, eventID) + h.ReportProfile(type2name[goroutine], h.writeProfileDataToFile(buf, goroutine, eventID), reason, eventID) return true } @@ -592,16 +587,11 @@ func (h *Holmes) cpuProfile(curCPUUsage int, c typeOption) bool { h.Errorf("encounter error when dumping profile to logger, failed to read cpu profile file: %v", err) return true } - h.Infof("[Holmes] CPU profile:: \n" + string(bfCpy)) + h.Infof("[Holmes] CPU profile name : " + "::" + binFileName + " \n" + string(bfCpy)) } if opts := h.opts.GetReporterOpts(); opts.active == 1 { - bfCpy, err := ioutil.ReadFile(binFileName) - if err != nil { - h.Errorf("[holmes reporter] failed to read cpu profile file: %v", err) - return true - } - h.ReportProfile(type2name[cpu], bfCpy, reason, "") + h.ReportProfile(type2name[cpu], binFileName, reason, "") } return true @@ -716,17 +706,16 @@ func (h *Holmes) gcHeapProfile(gc int, force bool, c typeOption) bool { var buf bytes.Buffer _ = pprof.Lookup("heap").WriteTo(&buf, int(h.opts.DumpProfileType)) // nolint: errcheck - h.writeProfileDataToFile(buf, gcHeap, eventID) - h.ReportProfile(type2name[gcHeap], buf.Bytes(), reason, eventID) + h.ReportProfile(type2name[gcHeap], h.writeProfileDataToFile(buf, gcHeap, eventID), reason, eventID) return true } -func (h *Holmes) writeProfileDataToFile(data bytes.Buffer, dumpType configureType, eventID string) { +func (h *Holmes) writeProfileDataToFile(data bytes.Buffer, dumpType configureType, eventID string) string { fileName, err := writeFile(data, dumpType, h.opts.DumpOptions, eventID) if err != nil { h.Errorf("failed to write profile to file(%v), err: %s", fileName, err.Error()) - return + return "" } if h.opts.DumpOptions.DumpToLogger { @@ -734,6 +723,7 @@ func (h *Holmes) writeProfileDataToFile(data bytes.Buffer, dumpType configureTyp } h.Infof("[Holmes] pprof %v profile write to file %v successfully", check2name[dumpType], fileName) + return fileName } func (h *Holmes) initEnvironment() { @@ -778,7 +768,12 @@ func (h *Holmes) EnableProfileReporter() { atomic.StoreInt32(&h.opts.rptOpts.active, 1) } -func (h *Holmes) ReportProfile(pType string, buf []byte, reason string, eventID string) { +func (h *Holmes) ReportProfile(pType string, dumpName string, reason string, eventID string) { + if dumpName == "" { + h.Errorf("dump name is empty, type:%s, reason:%s, eventID:%s", pType, reason, eventID) + return + } + defer func() { if r := recover(); r != nil { h.Errorf("Panic during report profile: %v", r) @@ -795,10 +790,10 @@ func (h *Holmes) ReportProfile(pType string, buf []byte, reason string, eventID } msg := rptEvent{ - PType: pType, - Buf: buf, - Reason: reason, - EventID: eventID, + PType: pType, + DumpName: dumpName, + Reason: reason, + EventID: eventID, } // read channel should be atomic. @@ -827,7 +822,7 @@ func (h *Holmes) startReporter(ch chan rptEvent) { continue } // It's supposed to be sending judgment, isn't it? - err := opts.reporter.Report(evt.PType, evt.Buf, evt.Reason, evt.EventID) // nolint: errcheck + err := opts.reporter.Report(evt.PType, evt.DumpName, evt.Reason, evt.EventID) // nolint: errcheck if err != nil { h.Infof("reporter err:", err) diff --git a/options.go b/options.go index 4cda8c2..ffc648c 100644 --- a/options.go +++ b/options.go @@ -69,10 +69,10 @@ type options struct { // rptEvent stands of the args of report event type rptEvent struct { - PType string - Buf []byte - Reason string - EventID string + PType string + DumpName string + Reason string + EventID string } type ReporterOptions struct { diff --git a/reporters/http_reporter/http_reporter.go b/reporters/http_reporter/http_reporter.go index f03a23f..a6aa235 100644 --- a/reporters/http_reporter/http_reporter.go +++ b/reporters/http_reporter/http_reporter.go @@ -45,18 +45,13 @@ func NewReporter(token string, url string) holmes.ProfileReporter { } } -func (r *HttpReporter) Report(ptype string, buf []byte, reason string, eventID string) error { +func (r *HttpReporter) Report(ptype string, dumpName string, reason string, eventID string) error { body := &bytes.Buffer{} writer := multipart.NewWriter(body) - part, err := writer.CreateFormFile("profile", "go-pprof-profile") - if err != nil { - return fmt.Errorf("create form File err: %v", err) - } - if _, err := part.Write(buf); err != nil { - return fmt.Errorf("write buf to file part err: %v", err) - } + writer.WriteField("token", r.token) // nolint: errcheck writer.WriteField("profile_type", ptype) // nolint: errcheck + writer.WriteField("dumpName", dumpName) // nolint: errcheck writer.WriteField("event_id", eventID) // nolint: errcheck writer.WriteField("comment", reason) // nolint: errcheck writer.Close() // nolint: errcheck diff --git a/reporters/http_reporter/http_reporter_test.go b/reporters/http_reporter/http_reporter_test.go index df45d8d..0e4e884 100644 --- a/reporters/http_reporter/http_reporter_test.go +++ b/reporters/http_reporter/http_reporter_test.go @@ -30,8 +30,7 @@ func TestHttpReporter_Report(t *testing.T) { reporter := NewReporter("test", "http://127.0.0.1:8080/profile/upload") - buf := []byte("test-data") - if err := reporter.Report("goroutine", buf, "test", "test-id"); err != nil { + if err := reporter.Report("goroutine", "dumpName", "test", "test-id"); err != nil { log.Fatalf("failed to report: %v", err) } } diff --git a/reporters/reporter_test.go b/reporters/reporter_test.go index 978635d..aa9aff7 100644 --- a/reporters/reporter_test.go +++ b/reporters/reporter_test.go @@ -47,7 +47,7 @@ var cpuReportCount int type mockReporter struct { } -func (m *mockReporter) Report(pType string, buf []byte, reason string, eventID string) error { +func (m *mockReporter) Report(pType string, dumpName string, reason string, eventID string) error { log.Printf("call %s report \n", pType) switch pType { @@ -65,7 +65,7 @@ var grReopenReportCount int type mockReopenReporter struct { } -func (m *mockReopenReporter) Report(pType string, buf []byte, reason string, eventID string) error { +func (m *mockReopenReporter) Report(pType string, dumpName string, reason string, eventID string) error { log.Printf("call %s report \n", pType) switch pType { From 70c51ca7b214ddf6dfee9f6f6cef5ed839516332 Mon Sep 17 00:00:00 2001 From: songzhibin97 <718428482@qq.com> Date: Mon, 16 May 2022 10:42:01 +0800 Subject: [PATCH 2/5] feat:write file --- holmes.go | 10 +++++----- options.go | 2 +- reporters/http_reporter/http_reporter.go | 19 +++++++++++++++++-- reporters/http_reporter/http_reporter_test.go | 2 +- reporters/reporter_test.go | 7 ++++--- 5 files changed, 28 insertions(+), 12 deletions(-) diff --git a/holmes.go b/holmes.go index e1e5e21..a6b0845 100644 --- a/holmes.go +++ b/holmes.go @@ -72,7 +72,7 @@ type Holmes struct { } type ProfileReporter interface { - Report(pType string, dumpName string, reason string, eventID string) error + Report(pType string, filename string, reason string, eventID string) error } // New creates a holmes dumper. @@ -768,8 +768,8 @@ func (h *Holmes) EnableProfileReporter() { atomic.StoreInt32(&h.opts.rptOpts.active, 1) } -func (h *Holmes) ReportProfile(pType string, dumpName string, reason string, eventID string) { - if dumpName == "" { +func (h *Holmes) ReportProfile(pType string, filename string, reason string, eventID string) { + if filename == "" { h.Errorf("dump name is empty, type:%s, reason:%s, eventID:%s", pType, reason, eventID) return } @@ -791,7 +791,7 @@ func (h *Holmes) ReportProfile(pType string, dumpName string, reason string, eve msg := rptEvent{ PType: pType, - DumpName: dumpName, + FileName: filename, Reason: reason, EventID: eventID, } @@ -822,7 +822,7 @@ func (h *Holmes) startReporter(ch chan rptEvent) { continue } // It's supposed to be sending judgment, isn't it? - err := opts.reporter.Report(evt.PType, evt.DumpName, evt.Reason, evt.EventID) // nolint: errcheck + err := opts.reporter.Report(evt.PType, evt.FileName, evt.Reason, evt.EventID) // nolint: errcheck if err != nil { h.Infof("reporter err:", err) diff --git a/options.go b/options.go index ffc648c..a36299c 100644 --- a/options.go +++ b/options.go @@ -70,7 +70,7 @@ type options struct { // rptEvent stands of the args of report event type rptEvent struct { PType string - DumpName string + FileName string Reason string EventID string } diff --git a/reporters/http_reporter/http_reporter.go b/reporters/http_reporter/http_reporter.go index a6aa235..9b018cb 100644 --- a/reporters/http_reporter/http_reporter.go +++ b/reporters/http_reporter/http_reporter.go @@ -45,13 +45,28 @@ func NewReporter(token string, url string) holmes.ProfileReporter { } } -func (r *HttpReporter) Report(ptype string, dumpName string, reason string, eventID string) error { +func (r *HttpReporter) Report(ptype string, filename string, reason string, eventID string) error { body := &bytes.Buffer{} writer := multipart.NewWriter(body) + // read filename + data, err := ioutil.ReadFile(filename) + if err != nil { + return fmt.Errorf("read form File: %s err: %v", filename, err) + } + + part, err := writer.CreateFormFile("profile", "go-pprof-profile") + if err != nil { + return fmt.Errorf("create form File err: %v", err) + } + + if _, err := part.Write(data); err != nil { + return fmt.Errorf("write buf to file part err: %v", err) + } + writer.WriteField("token", r.token) // nolint: errcheck writer.WriteField("profile_type", ptype) // nolint: errcheck - writer.WriteField("dumpName", dumpName) // nolint: errcheck + writer.WriteField("filename", filename) // nolint: errcheck writer.WriteField("event_id", eventID) // nolint: errcheck writer.WriteField("comment", reason) // nolint: errcheck writer.Close() // nolint: errcheck diff --git a/reporters/http_reporter/http_reporter_test.go b/reporters/http_reporter/http_reporter_test.go index 0e4e884..224b3e6 100644 --- a/reporters/http_reporter/http_reporter_test.go +++ b/reporters/http_reporter/http_reporter_test.go @@ -30,7 +30,7 @@ func TestHttpReporter_Report(t *testing.T) { reporter := NewReporter("test", "http://127.0.0.1:8080/profile/upload") - if err := reporter.Report("goroutine", "dumpName", "test", "test-id"); err != nil { + if err := reporter.Report("goroutine", "filename", "test", "test-id"); err != nil { log.Fatalf("failed to report: %v", err) } } diff --git a/reporters/reporter_test.go b/reporters/reporter_test.go index aa9aff7..6737797 100644 --- a/reporters/reporter_test.go +++ b/reporters/reporter_test.go @@ -47,9 +47,10 @@ var cpuReportCount int type mockReporter struct { } -func (m *mockReporter) Report(pType string, dumpName string, reason string, eventID string) error { - log.Printf("call %s report \n", pType) +func (m *mockReporter) Report(pType string, filename string, reason string, eventID string) error { + log.Printf("call %s , filename %s report \n", pType, filename) + // read filename switch pType { case "goroutine": grReportCount++ @@ -65,7 +66,7 @@ var grReopenReportCount int type mockReopenReporter struct { } -func (m *mockReopenReporter) Report(pType string, dumpName string, reason string, eventID string) error { +func (m *mockReopenReporter) Report(pType string, filename string, reason string, eventID string) error { log.Printf("call %s report \n", pType) switch pType { From 6435d14863e116fecdbcf59b6b072416e2a68fcd Mon Sep 17 00:00:00 2001 From: songzhibin97 <718428482@qq.com> Date: Mon, 16 May 2022 10:49:30 +0800 Subject: [PATCH 3/5] fix:report read file --- reporters/http_reporter/http_reporter.go | 23 ++++++++++--------- reporters/http_reporter/http_reporter_test.go | 2 +- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/reporters/http_reporter/http_reporter.go b/reporters/http_reporter/http_reporter.go index 9b018cb..4955153 100644 --- a/reporters/http_reporter/http_reporter.go +++ b/reporters/http_reporter/http_reporter.go @@ -50,18 +50,19 @@ func (r *HttpReporter) Report(ptype string, filename string, reason string, even writer := multipart.NewWriter(body) // read filename - data, err := ioutil.ReadFile(filename) - if err != nil { - return fmt.Errorf("read form File: %s err: %v", filename, err) - } - - part, err := writer.CreateFormFile("profile", "go-pprof-profile") - if err != nil { - return fmt.Errorf("create form File err: %v", err) - } + if filename != "" { + data, err := ioutil.ReadFile(filename) + if err != nil { + return fmt.Errorf("read form File: %s err: %v", filename, err) + } + part, err := writer.CreateFormFile("profile", "go-pprof-profile") + if err != nil { + return fmt.Errorf("create form File err: %v", err) + } - if _, err := part.Write(data); err != nil { - return fmt.Errorf("write buf to file part err: %v", err) + if _, err := part.Write(data); err != nil { + return fmt.Errorf("write buf to file part err: %v", err) + } } writer.WriteField("token", r.token) // nolint: errcheck diff --git a/reporters/http_reporter/http_reporter_test.go b/reporters/http_reporter/http_reporter_test.go index 224b3e6..fb0439c 100644 --- a/reporters/http_reporter/http_reporter_test.go +++ b/reporters/http_reporter/http_reporter_test.go @@ -30,7 +30,7 @@ func TestHttpReporter_Report(t *testing.T) { reporter := NewReporter("test", "http://127.0.0.1:8080/profile/upload") - if err := reporter.Report("goroutine", "filename", "test", "test-id"); err != nil { + if err := reporter.Report("goroutine", "", "test", "test-id"); err != nil { log.Fatalf("failed to report: %v", err) } } From 604e9614465db83e73e3332034af29a9f5443cea Mon Sep 17 00:00:00 2001 From: songzhibin97 <718428482@qq.com> Date: Mon, 16 May 2022 12:10:55 +0800 Subject: [PATCH 4/5] fix:suggestion --- reporters/http_reporter/http_reporter.go | 26 +++++++++---------- reporters/http_reporter/http_reporter_test.go | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/reporters/http_reporter/http_reporter.go b/reporters/http_reporter/http_reporter.go index 4955153..85969b4 100644 --- a/reporters/http_reporter/http_reporter.go +++ b/reporters/http_reporter/http_reporter.go @@ -50,24 +50,24 @@ func (r *HttpReporter) Report(ptype string, filename string, reason string, even writer := multipart.NewWriter(body) // read filename - if filename != "" { - data, err := ioutil.ReadFile(filename) - if err != nil { - return fmt.Errorf("read form File: %s err: %v", filename, err) - } - part, err := writer.CreateFormFile("profile", "go-pprof-profile") - if err != nil { - return fmt.Errorf("create form File err: %v", err) - } + if filename == "" { + return fmt.Errorf("file name is empty") + } + data, err := ioutil.ReadFile(filename) + if err != nil { + return fmt.Errorf("read form File: %s err: %v", filename, err) + } + part, err := writer.CreateFormFile("profile", "go-pprof-profile") + if err != nil { + return fmt.Errorf("create form File err: %v", err) + } - if _, err := part.Write(data); err != nil { - return fmt.Errorf("write buf to file part err: %v", err) - } + if _, err := part.Write(data); err != nil { + return fmt.Errorf("write buf to file part err: %v", err) } writer.WriteField("token", r.token) // nolint: errcheck writer.WriteField("profile_type", ptype) // nolint: errcheck - writer.WriteField("filename", filename) // nolint: errcheck writer.WriteField("event_id", eventID) // nolint: errcheck writer.WriteField("comment", reason) // nolint: errcheck writer.Close() // nolint: errcheck diff --git a/reporters/http_reporter/http_reporter_test.go b/reporters/http_reporter/http_reporter_test.go index fb0439c..d289d68 100644 --- a/reporters/http_reporter/http_reporter_test.go +++ b/reporters/http_reporter/http_reporter_test.go @@ -30,7 +30,7 @@ func TestHttpReporter_Report(t *testing.T) { reporter := NewReporter("test", "http://127.0.0.1:8080/profile/upload") - if err := reporter.Report("goroutine", "", "test", "test-id"); err != nil { + if err := reporter.Report("goroutine", "reporter_filename_test.log", "test", "test-id"); err != nil { log.Fatalf("failed to report: %v", err) } } From c28b67da50a9e0cb6913404d28f90650b49d627e Mon Sep 17 00:00:00 2001 From: songzhibin97 <718428482@qq.com> Date: Mon, 16 May 2022 12:13:52 +0800 Subject: [PATCH 5/5] fix:test --- reporters/http_reporter/http_reporter_test.go | 2 +- reporters/http_reporter/reporter_filename_test | 0 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 reporters/http_reporter/reporter_filename_test diff --git a/reporters/http_reporter/http_reporter_test.go b/reporters/http_reporter/http_reporter_test.go index d289d68..ce17d33 100644 --- a/reporters/http_reporter/http_reporter_test.go +++ b/reporters/http_reporter/http_reporter_test.go @@ -30,7 +30,7 @@ func TestHttpReporter_Report(t *testing.T) { reporter := NewReporter("test", "http://127.0.0.1:8080/profile/upload") - if err := reporter.Report("goroutine", "reporter_filename_test.log", "test", "test-id"); err != nil { + if err := reporter.Report("goroutine", "reporter_filename_test", "test", "test-id"); err != nil { log.Fatalf("failed to report: %v", err) } } diff --git a/reporters/http_reporter/reporter_filename_test b/reporters/http_reporter/reporter_filename_test new file mode 100644 index 0000000..e69de29