Skip to content

Commit 1e1a945

Browse files
authored
[cmd/telemetrygen] ensure validate is called (#35745)
Prior to this change, the Validate func wasn't being called resulting in wonky error messages like the following: ``` 2024-10-10T15:39:40.139-0700 INFO logs/worker.go:49 stopping the exporter {"worker": 0} panic: runtime error: cannot convert slice with length 8 to array or pointer to array with length 16 goroutine 6 [running]: github.com/open-telemetry/opentelemetry-collector-contrib/cmd/telemetrygen/internal/logs.worker.simulateLogs({0x140002404c4, 0x1, {0x100dd3152, 0xb}, 0x9, {0x100dccf6b, 0x4}, 0x0, 0x7fefffffffffffff, 0x140002404e0, ...}, ...) github.com/open-telemetry/opentelemetry-collector-contrib/cmd/telemetrygen/internal/logs/worker.go:70 +0xa34 created by github.com/open-telemetry/opentelemetry-collector-contrib/cmd/telemetrygen/internal/logs.Run in goroutine 1 ``` With the validation in place, we now get a useful error message: ``` Error: TraceID must be a 32 character hex string, like: 'ae87dadd90e9935a4bc9660628efd569' ``` --------- Signed-off-by: Alex Boten <[email protected]>
1 parent b6a28a3 commit 1e1a945

File tree

10 files changed

+197
-8
lines changed

10 files changed

+197
-8
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: telemetrygen
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: ensure validate is called
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [35745]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

cmd/telemetrygen/internal/logs/config.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package logs
55

66
import (
7+
"fmt"
8+
79
"github.com/spf13/pflag"
810

911
"github.com/open-telemetry/opentelemetry-collector-contrib/cmd/telemetrygen/internal/common"
@@ -36,6 +38,10 @@ func (c *Config) Flags(fs *pflag.FlagSet) {
3638

3739
// Validate validates the test scenario parameters.
3840
func (c *Config) Validate() error {
41+
if c.TotalDuration <= 0 && c.NumLogs <= 0 {
42+
return fmt.Errorf("either `logs` or `duration` must be greater than 0")
43+
}
44+
3945
if c.TraceID != "" {
4046
if err := common.ValidateTraceID(c.TraceID); err != nil {
4147
return err

cmd/telemetrygen/internal/logs/logs.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ func Start(cfg *Config) error {
6060
}
6161

6262
if err = Run(cfg, expFunc, logger); err != nil {
63-
logger.Error("failed to stop the exporter", zap.Error(err))
6463
return err
6564
}
6665

@@ -69,10 +68,12 @@ func Start(cfg *Config) error {
6968

7069
// Run executes the test scenario.
7170
func Run(c *Config, exp func() (sdklog.Exporter, error), logger *zap.Logger) error {
71+
if err := c.Validate(); err != nil {
72+
return err
73+
}
74+
7275
if c.TotalDuration > 0 {
7376
c.NumLogs = 0
74-
} else if c.NumLogs <= 0 {
75-
return fmt.Errorf("either `logs` or `duration` must be greater than 0")
7677
}
7778

7879
limit := rate.Limit(c.Rate)

cmd/telemetrygen/internal/logs/worker_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,58 @@ func TestLogsWithTraceIDAndSpanID(t *testing.T) {
229229
}
230230
}
231231

232+
func TestValidate(t *testing.T) {
233+
tests := []struct {
234+
name string
235+
cfg *Config
236+
wantErrMessage string
237+
}{
238+
{
239+
name: "No duration or NumLogs",
240+
cfg: &Config{
241+
Config: common.Config{
242+
WorkerCount: 1,
243+
},
244+
TraceID: "123",
245+
},
246+
wantErrMessage: "either `logs` or `duration` must be greater than 0",
247+
},
248+
{
249+
name: "TraceID invalid",
250+
cfg: &Config{
251+
Config: common.Config{
252+
WorkerCount: 1,
253+
},
254+
NumLogs: 5,
255+
TraceID: "123",
256+
},
257+
wantErrMessage: "TraceID must be a 32 character hex string, like: 'ae87dadd90e9935a4bc9660628efd569'",
258+
},
259+
{
260+
name: "SpanID invalid",
261+
cfg: &Config{
262+
Config: common.Config{
263+
WorkerCount: 1,
264+
},
265+
NumLogs: 5,
266+
TraceID: "ae87dadd90e9935a4bc9660628efd569",
267+
SpanID: "123",
268+
},
269+
wantErrMessage: "SpanID must be a 16 character hex string, like: '5828fa4960140870'",
270+
},
271+
}
272+
for _, tt := range tests {
273+
t.Run(tt.name, func(t *testing.T) {
274+
m := &mockExporter{}
275+
expFunc := func() (sdklog.Exporter, error) {
276+
return m, nil
277+
}
278+
logger, _ := zap.NewDevelopment()
279+
require.EqualError(t, Run(tt.cfg, expFunc, logger), tt.wantErrMessage)
280+
})
281+
}
282+
}
283+
232284
func configWithNoAttributes(qty int, body string) *Config {
233285
return &Config{
234286
Body: body,

cmd/telemetrygen/internal/metrics/config.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package metrics
55

66
import (
7+
"fmt"
8+
79
"github.com/spf13/pflag"
810

911
"github.com/open-telemetry/opentelemetry-collector-contrib/cmd/telemetrygen/internal/common"
@@ -39,6 +41,10 @@ func (c *Config) Flags(fs *pflag.FlagSet) {
3941

4042
// Validate validates the test scenario parameters.
4143
func (c *Config) Validate() error {
44+
if c.TotalDuration <= 0 && c.NumMetrics <= 0 {
45+
return fmt.Errorf("either `metrics` or `duration` must be greater than 0")
46+
}
47+
4248
if c.TraceID != "" {
4349
if err := common.ValidateTraceID(c.TraceID); err != nil {
4450
return err

cmd/telemetrygen/internal/metrics/metrics.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ func Start(cfg *Config) error {
6262
}
6363

6464
if err = Run(cfg, expFunc, logger); err != nil {
65-
logger.Error("failed to stop the exporter", zap.Error(err))
6665
return err
6766
}
6867

@@ -71,10 +70,12 @@ func Start(cfg *Config) error {
7170

7271
// Run executes the test scenario.
7372
func Run(c *Config, exp func() (sdkmetric.Exporter, error), logger *zap.Logger) error {
73+
if err := c.Validate(); err != nil {
74+
return err
75+
}
76+
7477
if c.TotalDuration > 0 {
7578
c.NumMetrics = 0
76-
} else if c.NumMetrics <= 0 {
77-
return fmt.Errorf("either `metrics` or `duration` must be greater than 0")
7879
}
7980

8081
limit := rate.Limit(c.Rate)

cmd/telemetrygen/internal/metrics/worker_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,61 @@ func TestGaugeMultipleTelemetryAttr(t *testing.T) {
300300
}
301301
}
302302

303+
func TestValidate(t *testing.T) {
304+
tests := []struct {
305+
name string
306+
cfg *Config
307+
wantErrMessage string
308+
}{
309+
{
310+
name: "No duration or NumMetrics",
311+
cfg: &Config{
312+
Config: common.Config{
313+
WorkerCount: 1,
314+
},
315+
MetricType: metricTypeSum,
316+
TraceID: "123",
317+
},
318+
wantErrMessage: "either `metrics` or `duration` must be greater than 0",
319+
},
320+
{
321+
name: "TraceID invalid",
322+
cfg: &Config{
323+
Config: common.Config{
324+
WorkerCount: 1,
325+
},
326+
NumMetrics: 5,
327+
MetricType: metricTypeSum,
328+
TraceID: "123",
329+
},
330+
wantErrMessage: "TraceID must be a 32 character hex string, like: 'ae87dadd90e9935a4bc9660628efd569'",
331+
},
332+
{
333+
name: "SpanID invalid",
334+
cfg: &Config{
335+
Config: common.Config{
336+
WorkerCount: 1,
337+
},
338+
NumMetrics: 5,
339+
MetricType: metricTypeSum,
340+
TraceID: "ae87dadd90e9935a4bc9660628efd569",
341+
SpanID: "123",
342+
},
343+
wantErrMessage: "SpanID must be a 16 character hex string, like: '5828fa4960140870'",
344+
},
345+
}
346+
for _, tt := range tests {
347+
t.Run(tt.name, func(t *testing.T) {
348+
m := &mockExporter{}
349+
expFunc := func() (sdkmetric.Exporter, error) {
350+
return m, nil
351+
}
352+
logger, _ := zap.NewDevelopment()
353+
require.EqualError(t, Run(tt.cfg, expFunc, logger), tt.wantErrMessage)
354+
})
355+
}
356+
}
357+
303358
func configWithNoAttributes(metric metricType, qty int) *Config {
304359
return &Config{
305360
Config: common.Config{

cmd/telemetrygen/internal/traces/config.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package traces
55

66
import (
7+
"fmt"
78
"time"
89

910
"github.com/spf13/pflag"
@@ -40,3 +41,11 @@ func (c *Config) Flags(fs *pflag.FlagSet) {
4041
fs.IntVar(&c.LoadSize, "size", 0, "Desired minimum size in MB of string data for each trace generated. This can be used to test traces with large payloads, i.e. when testing the OTLP receiver endpoint max receive size.")
4142
fs.DurationVar(&c.SpanDuration, "span-duration", 123*time.Microsecond, "The duration of each generated span.")
4243
}
44+
45+
// Validate validates the test scenario parameters.
46+
func (c *Config) Validate() error {
47+
if c.TotalDuration <= 0 && c.NumTraces <= 0 {
48+
return fmt.Errorf("either `traces` or `duration` must be greater than 0")
49+
}
50+
return nil
51+
}

cmd/telemetrygen/internal/traces/traces.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,12 @@ func Start(cfg *Config) error {
102102

103103
// Run executes the test scenario.
104104
func Run(c *Config, logger *zap.Logger) error {
105+
if err := c.Validate(); err != nil {
106+
return err
107+
}
108+
105109
if c.TotalDuration > 0 {
106110
c.NumTraces = 0
107-
} else if c.NumTraces <= 0 {
108-
return fmt.Errorf("either `traces` or `duration` must be greater than 0")
109111
}
110112

111113
limit := rate.Limit(c.Rate)

cmd/telemetrygen/internal/traces/worker_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,36 @@ func TestSpansWithMultipleAttrs(t *testing.T) {
310310
}
311311
}
312312

313+
func TestValidate(t *testing.T) {
314+
tests := []struct {
315+
name string
316+
cfg *Config
317+
wantErrMessage string
318+
}{
319+
{
320+
name: "No duration or NumTraces",
321+
cfg: &Config{
322+
Config: common.Config{
323+
WorkerCount: 1,
324+
},
325+
},
326+
wantErrMessage: "either `traces` or `duration` must be greater than 0",
327+
},
328+
}
329+
for _, tt := range tests {
330+
t.Run(tt.name, func(t *testing.T) {
331+
syncer := &mockSyncer{}
332+
333+
tracerProvider := sdktrace.NewTracerProvider()
334+
sp := sdktrace.NewSimpleSpanProcessor(syncer)
335+
tracerProvider.RegisterSpanProcessor(sp)
336+
otel.SetTracerProvider(tracerProvider)
337+
logger, _ := zap.NewDevelopment()
338+
require.EqualError(t, Run(tt.cfg, logger), tt.wantErrMessage)
339+
})
340+
}
341+
}
342+
313343
var _ sdktrace.SpanExporter = (*mockSyncer)(nil)
314344

315345
type mockSyncer struct {

0 commit comments

Comments
 (0)