Skip to content
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

testbed: refactor TestCase with broken out LoadGenerator interface #30303

Merged
merged 2 commits into from
Jan 10, 2024

Conversation

rmfitzpatrick
Copy link
Contributor

@rmfitzpatrick rmfitzpatrick commented Jan 4, 2024

Description:
Adding a feature - These changes break out the current testbed.LoadGenerator to an interface of the same name, migrating its (reference) implementation to a new testbed.ProviderSender type. This will allow future test cases (in arbitrary distributions) to use the existing* TestCase flow, but with arbitrary load generator implementations beyond the capabilities of the current one. They also provide a new testbed.NewLoadGeneratorTestCase similar to the existing testbed.NewTestCase that is now called indirectly. I chose this route to not force further signature changes.

In the process of this work, I found a few edge cases that can lead to* races and hanging processes in error scenarios. They've been addressed w/ some minimal synchronization.

Here's a partial* example of a LoadGenerator that can manage an arbitrary number of ProviderSender instances, each of varying telemetry types, that can now be used:

type CompositeLoadGenerator struct {
	startTime      time.Time
	loadGenerators []testbed.LoadGenerator
}

func NewCompositeLoadGenerator(loadGenerators ...testbed.LoadGenerator) testbed.LoadGenerator {
	return &CompositeLoadGenerator{
		loadGenerators: loadGenerators,
	}
}

func (o *CompositeLoadGenerator) Start(options testbed.LoadOptions) {
	o.startTime = time.Now()
	for _, lg := range o.loadGenerators {
		lg.Start(options)
	}
}

func (o *CompositeLoadGenerator) Stop() {
	for _, lg := range o.loadGenerators {
		lg.Stop()
	}
}

func (o *CompositeLoadGenerator) DataItemsSent() uint64 {
	var dataItemsSent uint64
	for _, lg := range o.loadGenerators {
		dataItemsSent += lg.DataItemsSent()
	}
	return dataItemsSent
}

<...>

Documentation:
Updated readme.

@@ -34,10 +34,9 @@ type TestCase struct {
// Agent process.
agentProc OtelcolRunner

Sender DataSender
Copy link
Contributor Author

@rmfitzpatrick rmfitzpatrick Jan 4, 2024

Choose a reason for hiding this comment

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

I think this is the main breaking change, with Sender now only being available on the ProviderSender instance, which is still available on the LoadGenerator field. In the cases where this was used, the tests already have* access to the sender directly so the indirect access is unnecessary to begin with. I decided to not address potential simplifications* in this initial pass to keep the changes localized to the involved types.

@dmitryax dmitryax merged commit fb6d694 into open-telemetry:main Jan 10, 2024
84 of 85 checks passed
@github-actions github-actions bot added this to the next release milestone Jan 10, 2024
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Jan 10, 2024
…pen-telemetry#30303)

**Description:**
Adding a feature - These changes break out the current
`testbed.LoadGenerator` to an interface of the same name, migrating its
(reference) implementation to a new `testbed.ProviderSender` type. This
will allow future test cases (in arbitrary distributions) to use the
existing* `TestCase` flow, but with arbitrary load generator
implementations beyond the capabilities of the current one. They also
provide a new `testbed.NewLoadGeneratorTestCase` similar to the existing
`testbed.NewTestCase` that is now called indirectly. I chose this route
to not force further signature changes.

In the process of this work, I found a few edge cases that can lead to*
races and hanging processes in error scenarios. They've been addressed
w/ some minimal synchronization.

Here's a partial* example of a LoadGenerator that can manage an
arbitrary number of `ProviderSender` instances, each of varying
telemetry types, that can now be used:

```go
type CompositeLoadGenerator struct {
	startTime      time.Time
	loadGenerators []testbed.LoadGenerator
}

func NewCompositeLoadGenerator(loadGenerators ...testbed.LoadGenerator) testbed.LoadGenerator {
	return &CompositeLoadGenerator{
		loadGenerators: loadGenerators,
	}
}

func (o *CompositeLoadGenerator) Start(options testbed.LoadOptions) {
	o.startTime = time.Now()
	for _, lg := range o.loadGenerators {
		lg.Start(options)
	}
}

func (o *CompositeLoadGenerator) Stop() {
	for _, lg := range o.loadGenerators {
		lg.Stop()
	}
}

func (o *CompositeLoadGenerator) DataItemsSent() uint64 {
	var dataItemsSent uint64
	for _, lg := range o.loadGenerators {
		dataItemsSent += lg.DataItemsSent()
	}
	return dataItemsSent
}

<...>
```

**Documentation:**
Updated readme.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants