Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion cmd/tracegen/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ The binary is available from the Releases page, as well as a Docker image:
$ docker run jaegertracing/jaeger-tracegen -service abcd -traces 10
```

The generator can be configured to export traces in different formats, via `-exporter` flag.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why delete this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sorry, I misunderstood and removed the line entirely. I have restored it now and corrected the flag to -trace-exporter from -exporter as requested

The generator can be configured to export traces in different formats, via `-trace-exporter` flag.

By default, the exporters send data to `localhost`. If running in a container, this refers
Comment on lines +12 to 14
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

The README now references -trace-exporter, but it still doesn’t mention the newly added file:{filename} option or provide an example command. Since this feature is specifically for producing replayable JSON output, consider documenting the file: syntax and expected output format (e.g., one JSON object per line / NDJSON) to make it discoverable and reduce user error.

Copilot uses AI. Check for mistakes.
to the networking namespace of the container itself, so to export to another container,
the exporters need to be provided with appropriate location.
Expand Down
39 changes: 31 additions & 8 deletions cmd/tracegen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"errors"
"flag"
"fmt"
"os"
"strings"
"time"

"github.com/go-logr/zapr"
Expand Down Expand Up @@ -71,13 +73,27 @@ func createTracers(cfg *tracegen.Config, logger *zap.Logger) ([]trace.Tracer, fu
}
var shutdown []func(context.Context) error
var tracers []trace.Tracer

var file *os.File
exporterType := cfg.TraceExporter

if strings.HasPrefix(cfg.TraceExporter, "file:") {
filename := strings.TrimPrefix(cfg.TraceExporter, "file:")
var err error
file, err = os.Create(filename)
if err != nil {
logger.Sugar().Fatalf("cannot create output file %s: %s", filename, err)
}
exporterType = "file"
}

for s := 0; s < cfg.Services; s++ {
svc := cfg.Service
if cfg.Services > 1 {
svc = fmt.Sprintf("%s-%02d", svc, s)
}

exp, err := createOtelExporter(cfg.TraceExporter)
exp, err := createOtelExporter(exporterType, file)
if err != nil {
logger.Sugar().Fatalf("cannot create trace exporter %s: %s", cfg.TraceExporter, err)
}
Expand Down Expand Up @@ -113,6 +129,13 @@ func createTracers(cfg *tracegen.Config, logger *zap.Logger) ([]trace.Tracer, fu
tracers = append(tracers, tp.Tracer(cfg.Service))
shutdown = append(shutdown, tp.Shutdown)
}

if file != nil {
shutdown = append(shutdown, func(_ context.Context) error {
return file.Close()
})
}
Comment on lines +133 to +137
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resource leak on early error: If an error occurs after the file is created (line 83) but before this shutdown function is registered, the file handle will never be closed. For example, if createOtelExporter fails at line 96-98, the code calls Fatalf which exits the program without properly closing the file.

Fix: Use defer immediately after file creation:

file, err = os.Create(filename)
if err != nil {
    logger.Sugar().Fatalf("cannot create output file %s: %s", filename, err)
}
defer file.Close()  // Add this

Alternatively, ensure the file close is registered in the shutdown list immediately after creation, before any operations that could fail.

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

agreed, this logic for adding file close to shutdown should be done right after you successfully opened the file


return tracers, func(ctx context.Context) error {
var errs []error
for _, f := range shutdown {
Expand All @@ -122,21 +145,21 @@ func createTracers(cfg *tracegen.Config, logger *zap.Logger) ([]trace.Tracer, fu
}
}

func createOtelExporter(exporterType string) (sdktrace.SpanExporter, error) {
func createOtelExporter(exporterType string, fileWriter *os.File) (sdktrace.SpanExporter, error) {
var exporter sdktrace.SpanExporter
var err error
switch exporterType {
case "file":
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

createOtelExporter accepts exporterType == "file" but doesn’t validate fileWriter. If a user passes -trace-exporter=file (or if parsing changes later), this will call stdouttrace.WithWriter(nil) which can lead to panics or write failures. Consider rejecting "file" without a non-nil writer (return a clear error), or parse file:{filename}/file: inside createOtelExporter so the API cannot be misused.

Suggested change
case "file":
case "file":
if fileWriter == nil {
return nil, errors.New("file exporter requires a non-nil fileWriter")
}

Copilot uses AI. Check for mistakes.
return stdouttrace.New(
stdouttrace.WithWriter(fileWriter),
)
case "jaeger":
return nil, errors.New("jaeger exporter is no longer supported, please use otlp")
case "otlp", "otlp-http":
client := otlptracehttp.NewClient(
otlptracehttp.WithInsecure(),
)
client := otlptracehttp.NewClient(otlptracehttp.WithInsecure())
exporter, err = otlptrace.New(context.Background(), client)
case "otlp-grpc":
client := otlptracegrpc.NewClient(
otlptracegrpc.WithInsecure(),
)
client := otlptracegrpc.NewClient(otlptracegrpc.WithInsecure())
exporter, err = otlptrace.New(context.Background(), client)
case "stdout":
exporter, err = stdouttrace.New()
Expand Down
2 changes: 1 addition & 1 deletion internal/tracegen/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (c *Config) Flags(fs *flag.FlagSet) {
fs.DurationVar(&c.Duration, "duration", 0, "For how long to run the test if greater than 0s (overrides -traces).")
fs.StringVar(&c.Service, "service", "tracegen", "Service name prefix to use")
fs.IntVar(&c.Services, "services", 1, "Number of unique suffixes to add to service name when generating traces, e.g. tracegen-01 (but only one service per trace)")
fs.StringVar(&c.TraceExporter, "trace-exporter", "otlp-http", "Trace exporter (otlp/otlp-http|otlp-grpc|stdout). Exporters can be additionally configured via environment variables, see https://github.com/jaegertracing/jaeger/blob/main/cmd/tracegen/README.md")
fs.StringVar(&c.TraceExporter, "trace-exporter", "otlp-http", "Trace exporter (otlp/otlp-http|otlp-grpc|stdout|file:{filename}). Exporters can be additionally configured via environment variables, see https://github.com/jaegertracing/jaeger/blob/main/cmd/tracegen/README.md")
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

The PR description says a new -marshal-to-file flag was added, but the code changes actually extend -trace-exporter to accept file:{filename} / file:. Please align the PR description with the implementation (or add the documented flag) so users and reviewers aren’t misled about the public CLI/API.

Copilot uses AI. Check for mistakes.
}

// Run executes the test scenario.
Expand Down
Loading