Skip to content

Commit 75d2681

Browse files
committed
fix(printer): properly close output files to prevent data loss
The Close() methods for jsonEventPrinter, templateEventPrinter, and tableEventPrinter were empty stubs that never closed the underlying io.WriteCloser. This caused intermittent test failures when OS write buffers weren't flushed before process termination, resulting in lost events in the output files. Now all printer types properly close their output files, ensuring buffers are flushed and all events are written to disk.
1 parent 9ac65ff commit 75d2681

File tree

2 files changed

+140
-0
lines changed

2 files changed

+140
-0
lines changed

pkg/cmd/printer/printer.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,11 @@ func (p tableEventPrinter) Epilogue(stats metrics.Stats) {
349349
}
350350

351351
func (p tableEventPrinter) Close() {
352+
if p.out != nil {
353+
if err := p.out.Close(); err != nil {
354+
logger.Errorw("Error closing table output file", "error", err)
355+
}
356+
}
352357
}
353358

354359
type templateEventPrinter struct {
@@ -389,6 +394,11 @@ func (p templateEventPrinter) Print(event trace.Event) {
389394
func (p templateEventPrinter) Epilogue(stats metrics.Stats) {}
390395

391396
func (p templateEventPrinter) Close() {
397+
if p.out != nil {
398+
if err := p.out.Close(); err != nil {
399+
logger.Errorw("Error closing template output file", "error", err)
400+
}
401+
}
392402
}
393403

394404
type jsonEventPrinter struct {
@@ -410,6 +420,11 @@ func (p jsonEventPrinter) Print(event trace.Event) {
410420
func (p jsonEventPrinter) Epilogue(stats metrics.Stats) {}
411421

412422
func (p jsonEventPrinter) Close() {
423+
if p.out != nil {
424+
if err := p.out.Close(); err != nil {
425+
logger.Errorw("Error closing JSON output file", "error", err)
426+
}
427+
}
413428
}
414429

415430
// ignoreEventPrinter ignores events

pkg/cmd/printer/printer_test.go

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,3 +135,128 @@ func TestTemplateEventPrinterSprigFunctions(t *testing.T) {
135135
// Should contain timestamp
136136
assert.Contains(t, output, `1234567890`, "timestamp should be included")
137137
}
138+
139+
// TestPrinterCloseFlusheData tests that Close() properly flushes and closes output files
140+
func TestPrinterCloseFlushesData(t *testing.T) {
141+
t.Parallel()
142+
143+
testCases := []struct {
144+
name string
145+
printerKind string
146+
}{
147+
{
148+
name: "json printer",
149+
printerKind: "json",
150+
},
151+
{
152+
name: "table printer",
153+
printerKind: "table",
154+
},
155+
}
156+
157+
for _, tc := range testCases {
158+
tc := tc
159+
t.Run(tc.name, func(t *testing.T) {
160+
t.Parallel()
161+
162+
// Create a temporary file
163+
tempDir := t.TempDir()
164+
outputPath := filepath.Join(tempDir, "test_output.txt")
165+
166+
// Create the output file
167+
file, err := flags.CreateOutputFile(outputPath)
168+
require.NoError(t, err)
169+
170+
// Create printer config
171+
cfg := config.PrinterConfig{
172+
Kind: tc.printerKind,
173+
OutFile: file,
174+
}
175+
176+
// Create and initialize the printer
177+
p, err := printer.New(cfg)
178+
require.NoError(t, err)
179+
180+
// Create a sample event
181+
sampleEvent := trace.Event{
182+
ProcessName: "test_process",
183+
EventID: 1,
184+
EventName: "test_event",
185+
Timestamp: 1234567890,
186+
Args: []trace.Argument{
187+
{ArgMeta: trace.ArgMeta{Name: "arg1", Type: "string"}, Value: "value1"},
188+
},
189+
}
190+
191+
// Print an event
192+
p.Preamble()
193+
p.Print(sampleEvent)
194+
p.Close()
195+
196+
// Read the file content
197+
content, err := os.ReadFile(outputPath)
198+
require.NoError(t, err, "Should be able to read file after Close()")
199+
200+
// Verify data was written and flushed
201+
assert.NotEmpty(t, content, "File should contain data after Close()")
202+
assert.Contains(t, string(content), "test_process", "File should contain event data")
203+
204+
// Verify file is actually closed by trying to write to it
205+
// This should fail because Close() should have closed the file
206+
_, err = file.Write([]byte("test"))
207+
assert.Error(t, err, "Writing to file after Close() should fail")
208+
})
209+
}
210+
}
211+
212+
// TestTemplateEventPrinterCloseFlushesData tests template printer Close() behavior
213+
func TestTemplateEventPrinterCloseFlushesData(t *testing.T) {
214+
t.Parallel()
215+
216+
// Create a temporary template file
217+
templateContent := `{"event": "{{ .EventName }}", "process": "{{ .ProcessName }}"}`
218+
tempDir := t.TempDir()
219+
templatePath := filepath.Join(tempDir, "test.tmpl")
220+
outputPath := filepath.Join(tempDir, "test_output.txt")
221+
222+
err := os.WriteFile(templatePath, []byte(templateContent), 0644)
223+
require.NoError(t, err)
224+
225+
// Create the output file
226+
file, err := flags.CreateOutputFile(outputPath)
227+
require.NoError(t, err)
228+
229+
// Create printer config
230+
cfg := config.PrinterConfig{
231+
Kind: "gotemplate=" + templatePath,
232+
OutFile: file,
233+
}
234+
235+
// Create and initialize the printer
236+
p, err := printer.New(cfg)
237+
require.NoError(t, err)
238+
239+
// Create a sample event
240+
sampleEvent := trace.Event{
241+
ProcessName: "test_process",
242+
EventName: "test_event",
243+
}
244+
245+
// Print an event
246+
p.Preamble()
247+
p.Print(sampleEvent)
248+
p.Close()
249+
250+
// Read the file content
251+
content, err := os.ReadFile(outputPath)
252+
require.NoError(t, err, "Should be able to read file after Close()")
253+
254+
// Verify data was written and flushed
255+
assert.NotEmpty(t, content, "File should contain data after Close()")
256+
assert.Contains(t, string(content), "test_process", "File should contain event data")
257+
assert.Contains(t, string(content), "test_event", "File should contain event name")
258+
259+
// Verify file is actually closed
260+
_, err = file.Write([]byte("test"))
261+
assert.Error(t, err, "Writing to file after Close() should fail")
262+
}

0 commit comments

Comments
 (0)