-
Notifications
You must be signed in to change notification settings - Fork 2.9k
tracegen: add capability to marshal spans to file #7691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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. | ||
| 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
|
||
| to the networking namespace of the container itself, so to export to another container, | ||
| the exporters need to be provided with appropriate location. | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,8 @@ import ( | |||||||||||
| "errors" | ||||||||||||
| "flag" | ||||||||||||
| "fmt" | ||||||||||||
| "os" | ||||||||||||
| "strings" | ||||||||||||
| "time" | ||||||||||||
|
|
||||||||||||
| "github.com/go-logr/zapr" | ||||||||||||
|
|
@@ -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) | ||||||||||||
| } | ||||||||||||
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 thisAlternatively, ensure the file close is registered in the shutdown list immediately after creation, before any operations that could fail. Spotted by Graphite Agent
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||||||||||||
|
|
@@ -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": | ||||||||||||
|
||||||||||||
| case "file": | |
| case "file": | |
| if fileWriter == nil { | |
| return nil, errors.New("file exporter requires a non-nil fileWriter") | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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") | ||
|
||
| } | ||
|
|
||
| // Run executes the test scenario. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why delete this?
There was a problem hiding this comment.
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-exporterfrom-exporteras requested