Skip to content

Commit

Permalink
[LS-60884] remove uses of rand.Seed (#71)
Browse files Browse the repository at this point in the history
## What is the current behavior?
`rand.Seed` is deprecated, and this deprecated method being in use is
preventing the linter from allowing changes to this repo


## What is the new behavior?
All uses of `rand.Seed(someseed)` are removed. This is replaced with
`rand.New(rand.NewSource(someseed))` which is the preferred way to
handle this according to doc strings. Instead of using the global `rand`
in random generation, the preferred method seems to be to pass down the
generator created by `rand.New` so this also passed down generators into
any method that uses them

There is a question here of where we should pass generators as function
args and where we should just put the generator as a field in the
structs, but a bunch of these structs have json annotations that suggest
they are read from files and i do not really know how to make sure
`rand` gets populated in the case where some of these structs are
constructed from json

---------

Co-authored-by: jason.crouse <[email protected]>
  • Loading branch information
jdcrouse and jason.crouse authored Sep 9, 2024
1 parent ab5483b commit 91429ba
Show file tree
Hide file tree
Showing 15 changed files with 81 additions and 69 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
build
vendor/
dist/
.idea/
19 changes: 10 additions & 9 deletions generatorreceiver/generator_receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ func (g generatorReceiver) Start(ctx context.Context, host component.Host) error
// rand is used to generate seeds the underlying *rand.Rand
generatorRand := rand.New(rand.NewSource(g.randomSeed))

// Metrics generator uses the global rand.Rand
// TODO: LS-60180 - rand.Seed is deprecated, use rand.NewSource
rand.Seed(generatorRand.Int63())

if g.server != nil {
err := g.server.Start(ctx, host)
if err != nil {
Expand All @@ -79,7 +75,7 @@ func (g generatorReceiver) Start(ctx context.Context, host component.Host) error
continue
}
k.Cfg = topoFile.Config
k.CreatePods(s.ServiceName)
k.CreatePods(s.ServiceName, generatorRand)
}
}

Expand All @@ -88,7 +84,7 @@ func (g generatorReceiver) Start(ctx context.Context, host component.Host) error

// Service defined metrics
for _, m := range s.Metrics {
metricTicker := g.startMetricGenerator(ctx, s.ServiceName, m)
metricTicker := g.startMetricGenerator(ctx, s.ServiceName, m, generatorRand)
g.tickers = append(g.tickers, metricTicker)
}

Expand All @@ -104,7 +100,7 @@ func (g generatorReceiver) Start(ctx context.Context, host component.Host) error
// keep the same flags as the resources.
k8sMetrics[i].EmbeddedFlags = resource.EmbeddedFlags

metricTicker := g.startMetricGenerator(ctx, s.ServiceName, k8sMetrics[i])
metricTicker := g.startMetricGenerator(ctx, s.ServiceName, k8sMetrics[i], generatorRand)
g.tickers = append(g.tickers, metricTicker)
}
}
Expand Down Expand Up @@ -150,14 +146,19 @@ func (g generatorReceiver) Start(ctx context.Context, host component.Host) error
return nil
}

func (g *generatorReceiver) startMetricGenerator(ctx context.Context, serviceName string, m topology.Metric) *time.Ticker {
func (g *generatorReceiver) startMetricGenerator(
ctx context.Context,
serviceName string,
m topology.Metric,
random *rand.Rand,
) *time.Ticker {
// TODO: do we actually need to generate every second?
metricTicker := time.NewTicker(topology.DefaultMetricTickerPeriod)
go func() {
g.logger.Info("generating metrics", zap.String("service", serviceName), zap.String("name", m.Name), zap.String("flag_set", m.EmbeddedFlags.FlagSet), zap.String("flag_unset", m.EmbeddedFlags.FlagUnset))
metricGen := generator.NewMetricGenerator(g.randomSeed)
for range metricTicker.C {
m.Pod.RestartIfNeeded(m.EmbeddedFlags, g.logger)
m.Pod.RestartIfNeeded(m.EmbeddedFlags, g.logger, random)

if metrics, report := metricGen.Generate(&m, serviceName); report {
err := g.metricConsumer.ConsumeMetrics(ctx, metrics)
Expand Down
8 changes: 4 additions & 4 deletions generatorreceiver/internal/generator/trace_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (g *TraceGenerator) createSpanForServiceRouteCall(traces *ptrace.Traces, se

resourceAttributeSet := serviceTier.GetResourceAttributeSet(traceId)
attrs := resource.Attributes()
resourceAttributeSet.GetAttributes().InsertTags(&attrs)
resourceAttributeSet.GetAttributes(g.random).InsertTags(&attrs, g.random)

rspan.ScopeSpans()
ils := rspan.ScopeSpans().AppendEmpty()
Expand All @@ -94,7 +94,7 @@ func (g *TraceGenerator) createSpanForServiceRouteCall(traces *ptrace.Traces, se

ts := serviceTier.GetTagSet(routeName, traceId) // ts is single TagSet consisting of tags from the service AND route
attr := span.Attributes()
ts.Tags.InsertTags(&attr) // add service and route tags to span attributes
ts.Tags.InsertTags(&attr, g.random) // add service and route tags to span attributes

for _, tg := range ts.TagGenerators {
tg.Init(g.random)
Expand All @@ -106,9 +106,9 @@ func (g *TraceGenerator) createSpanForServiceRouteCall(traces *ptrace.Traces, se
// TODO: this is still a bit weird - we're calling each downstream route
// after a sample of the current route's latency, which doesn't really
// make sense - but maybe it's realistic enough?
endTime := startTimeNanos + route.SampleLatency(traceId)
endTime := startTimeNanos + route.SampleLatency(traceId, g.random)
for _, c := range route.DownstreamCalls {
var childStartTimeNanos = startTimeNanos + route.SampleLatency(traceId)
var childStartTimeNanos = startTimeNanos + route.SampleLatency(traceId, g.random)

childSpan := g.createSpanForServiceRouteCall(traces, c.Service, c.Route, childStartTimeNanos, traceId, newSpanId)
val, ok := childSpan.Attributes().Get("error")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestTraceGenerator_createSpanForServiceRouteCall2(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
traces := ptrace.NewTraces()
g := NewTraceGenerator(testTopology, rand.New(rand.NewSource(rand.Int63())), tt.args.serviceName, tt.args.routeName)
g := NewTraceGenerator(testTopology, rand.New(rand.NewSource(123)), tt.args.serviceName, tt.args.routeName)
genTraceID := g.genTraceId()
rootSpan := g.createSpanForServiceRouteCall(&traces, g.service, g.route, tt.args.startTimeNanos, genTraceID, pcommon.NewSpanIDEmpty())
convertedSpanStartTime := pcommon.NewTimestampFromTime(time.Unix(0, tt.args.startTimeNanos))
Expand Down
38 changes: 19 additions & 19 deletions generatorreceiver/internal/topology/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,20 @@ type ResourceUsage struct {
Jitter float64 `json:"jitter" yaml:"jitter"`
}

func (k *Kubernetes) CreatePods(serviceName string) {
func (k *Kubernetes) CreatePods(serviceName string, random *rand.Rand) {
k.mutex.Lock()
defer k.mutex.Unlock()
k.ReplicaSetName = serviceName + "-" + generateK8sName(10)
k.ReplicaSetName = serviceName + "-" + generateK8sName(10, random)
k.Namespace = serviceName
k.Service = serviceName
k.pods = make([]*Pod, k.GetPodCount())
for i := 0; i < len(k.pods); i++ {
k.pods[i] = &Pod{
StartTime: time.Now(),
PodName: k.ReplicaSetName + "-" + generateK8sName(5),
PodName: k.ReplicaSetName + "-" + generateK8sName(5, random),
Container: serviceName,
Kubernetes: k,
RestartDuration: k.RestartDurationWithJitter(),
RestartDuration: k.RestartDurationWithJitter(random),
}
}
}
Expand All @@ -103,7 +103,7 @@ func (k *Kubernetes) GetPodCount() int {
}
}

func (p *Pod) RestartIfNeeded(flags flags.EmbeddedFlags, logger *zap.Logger) bool {
func (p *Pod) RestartIfNeeded(flags flags.EmbeddedFlags, logger *zap.Logger, random *rand.Rand) bool {
if p == nil || p.Kubernetes.Restart.Every == 0 {
return false
}
Expand All @@ -115,35 +115,35 @@ func (p *Pod) RestartIfNeeded(flags flags.EmbeddedFlags, logger *zap.Logger) boo
if flagTime.After(p.StartTime) {
// consider that the pod started at the time that a flag was enabled/disabled.
// TODO: restart with some jitter
p.restart(logger)
p.restart(logger, random)
return true
} else if time.Since(p.StartTime) >= p.RestartDuration {
// TODO: restart with some jitter
p.restart(logger)
p.restart(logger, random)
return true
}
return false

}

func (p *Pod) restart(logger *zap.Logger) {
func (p *Pod) restart(logger *zap.Logger, random *rand.Rand) {
// this is locked by RestartIfNeeded
p.StartTime = time.Now()
p.RestartDuration = p.Kubernetes.RestartDurationWithJitter()
p.PodName = p.Kubernetes.ReplicaSetName + "-" + generateK8sName(5)
p.RestartDuration = p.Kubernetes.RestartDurationWithJitter(random)
p.PodName = p.Kubernetes.ReplicaSetName + "-" + generateK8sName(5, random)
logger.Info("pod restarted", zap.String("service", p.Kubernetes.Service), zap.String("pod", p.PodName))
}

func (k *Kubernetes) randomPod() *Pod {
return k.pods[rand.Intn(len(k.pods))]
func (k *Kubernetes) randomPod(random *rand.Rand) *Pod {
return k.pods[random.Intn(len(k.pods))]
}

// only called from tag generator!
func (k *Kubernetes) GetRandomK8sTags() map[string]string {
func (k *Kubernetes) GetRandomK8sTags(random *rand.Rand) map[string]string {
k.mutex.Lock()
defer k.mutex.Unlock()

pod := k.randomPod()
pod := k.randomPod(random)
// ref: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/semantic_conventions/k8s.md
return k.GetK8sTags(pod)
}
Expand Down Expand Up @@ -187,8 +187,8 @@ func (p *Pod) ReplaceTags(tags map[string]string) map[string]string {
return replaced
}

func (k *Kubernetes) RestartDurationWithJitter() time.Duration {
return k.Restart.Every + time.Duration(float64(k.Restart.Jitter)*(rand.Float64()-0.5))
func (k *Kubernetes) RestartDurationWithJitter(random *rand.Rand) time.Duration {
return k.Restart.Every + time.Duration(float64(k.Restart.Jitter)*(random.Float64()-0.5))
}

func (k *Kubernetes) GenerateMetrics() []Metric {
Expand Down Expand Up @@ -664,12 +664,12 @@ func (k *Kubernetes) GenerateMetrics() []Metric {
return metrics
}

func generateK8sName(n int) string {
func generateK8sName(nameLength int, random *rand.Rand) string {
var letters = []rune("bcdfghjklmnpqrstvwxz2456789")

b := make([]rune, n)
b := make([]rune, nameLength)
for i := range b {
b[i] = letters[rand.Intn(len(letters))]
b[i] = letters[random.Intn(len(letters))]
}
return string(b)
}
10 changes: 6 additions & 4 deletions generatorreceiver/internal/topology/kubernetes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package topology

import (
"github.com/stretchr/testify/require"
"math/rand"
"testing"
"time"
)
Expand Down Expand Up @@ -59,13 +60,14 @@ func TestMultiPod(t *testing.T) {
},
PodCount: nPods,
}
k.CreatePods("some")
random := rand.New(rand.NewSource(123))
k.CreatePods("some", random)

// we should see more than one pod name in the tags
// (odds of this test failing randomly are 1 in 7**100 =~ 3 in 10^85
names := make(map[string]bool)
for i := 0; i < 100; i++ {
tags := k.GetRandomK8sTags()
tags := k.GetRandomK8sTags(random)
names[tags["k8s.pod.name"]] = true
}
require.Greater(t, len(names), 1, "multiple pod names should be generated")
Expand All @@ -90,7 +92,7 @@ func TestMultiPod(t *testing.T) {
},
},
}
k.CreatePods("some")
k.CreatePods("some", random)
require.Equal(t, 2, k.GetPodCount(), "pod count defaults to config value")
k = &Kubernetes{
ClusterName: "some-cluster",
Expand All @@ -99,6 +101,6 @@ func TestMultiPod(t *testing.T) {
Jitter: minute,
},
}
k.CreatePods("some")
k.CreatePods("some", random)
require.Equal(t, 1, k.GetPodCount(), "pod count defaults to 1 if no config value")
}
12 changes: 6 additions & 6 deletions generatorreceiver/internal/topology/latency_percentiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type LatencyPercentiles struct {
flags.EmbeddedFlags `json:",inline" yaml:",inline"`
}

func (l *LatencyPercentiles) Sample() int64 {
func (l *LatencyPercentiles) Sample(random *rand.Rand) int64 {
if l == nil {
// This results from having a list where
// items are !ShouldGenerate() which leaves
Expand All @@ -38,9 +38,9 @@ func (l *LatencyPercentiles) Sample() int64 {
uniform := func(timeA, timeB time.Duration) int64 {
minimum := float64(timeA.Nanoseconds())
maximum := float64(timeB.Nanoseconds())
return int64(minimum + (maximum-minimum)*rand.Float64())
return int64(minimum + (maximum-minimum)*random.Float64())
}
genNumber := rand.Float64()
genNumber := random.Float64()
switch {
case genNumber < 0.5:
return uniform(l.durations.p0, l.durations.p50)
Expand Down Expand Up @@ -90,7 +90,7 @@ func (l *LatencyPercentiles) loadDurations() error {

type LatencyConfigs []*LatencyPercentiles

func (lcfg *LatencyConfigs) Sample(traceID pcommon.TraceID) int64 {
func (lcfg *LatencyConfigs) Sample(traceID pcommon.TraceID, random *rand.Rand) int64 {
var defaultCfg *LatencyPercentiles
var enabled []*LatencyPercentiles
for _, cfg := range *lcfg {
Expand All @@ -104,8 +104,8 @@ func (lcfg *LatencyConfigs) Sample(traceID pcommon.TraceID) int64 {
picked := pickBasedOnWeight(enabled, traceID)

if picked != nil {
return picked.Sample()
return picked.Sample(random)
}
}
return defaultCfg.Sample()
return defaultCfg.Sample(random)
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package topology

import (
"math/rand"
"testing"
"time"

Expand Down Expand Up @@ -52,7 +53,7 @@ func TestLatencyPercentiles_WorksWithoutWeights(t *testing.T) {
err := cfg.loadDurations()
require.NoError(t, err)
}

latency := cfgs.Sample(pcommon.NewTraceIDEmpty())
random := rand.New(rand.NewSource(123))
latency := cfgs.Sample(pcommon.NewTraceIDEmpty(), random)
require.Equal(t, 100*time.Millisecond, time.Duration(latency))
}
11 changes: 5 additions & 6 deletions generatorreceiver/internal/topology/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,24 +62,23 @@ type Metric struct {
Shape Shape `json:"shape" yaml:"shape"`
ShapeInterface ShapeInterface `json:"-" yaml:"-"`
Tags map[string]string `json:"tags" yaml:"tags"`
TagGenerator TagGenerator `json:"tagGenerator,omitempty" yaml:"tagGenerator,omitempty"`
TagGenerator TagGenerator `json:"tagGenerator,omitempty" yaml:"tagGenerator,omitempty"`
Jitter float64 `json:"jitter" yaml:"jitter"`
flags.EmbeddedFlags `json:",inline" yaml:",inline"`
Pod *Pod
Random *rand.Rand
Random *rand.Rand
}

func (m *Metric) GetTags() map[string]string {
if m.Pod != nil {
return m.Pod.ReplaceTags(m.Tags)
}


tags := make(map[string]string)
for k,v := range m.Tags {
for k, v := range m.Tags {
tags[k] = v
}
for k,v := range m.TagGenerator.GetRefreshedTags() {
for k, v := range m.TagGenerator.GetRefreshedTags() {
tags[k] = v
}

Expand Down Expand Up @@ -159,7 +158,7 @@ func (m *Metric) GetValue() float64 {
v := m.Min + (m.Max-m.Min)*factor

// jitter deviation is calculated in percentage that ranges from [-m.Jitter/2, m.Jitter/2)%
j := 1 + rand.Float64()*m.Jitter - m.Jitter/2
j := 1 + m.Random.Float64()*m.Jitter - m.Jitter/2

v = v * j

Expand Down
5 changes: 3 additions & 2 deletions generatorreceiver/internal/topology/resource_attribute_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package topology

import (
"github.com/lightstep/telemetry-generator/generatorreceiver/internal/flags"
"math/rand"
)

type ResourceAttributeSet struct {
Expand All @@ -11,13 +12,13 @@ type ResourceAttributeSet struct {
flags.EmbeddedFlags `json:",inline" yaml:",inline"`
}

func (r *ResourceAttributeSet) GetAttributes() *TagMap {
func (r *ResourceAttributeSet) GetAttributes(random *rand.Rand) *TagMap {
tm := make(TagMap)
for k, v := range r.ResourceAttributes {
tm[k] = v
}
if k8s := r.Kubernetes; k8s != nil {
for k, v := range k8s.GetRandomK8sTags() {
for k, v := range k8s.GetRandomK8sTags(random) {
tm[k] = v
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,16 @@ func TestResourceAttributeSet_GetAttributes(t *testing.T) {
}

if k := resourceAttrSet.Kubernetes; k != nil {
rand.Seed(123)
k.CreatePods(tt.service)
random := rand.New(rand.NewSource(123))
k.CreatePods(tt.service, random)

// k8s.pod.name structure was copied from CreatePods()
rand.Seed(123)
tt.expected["k8s.pod.name"] = tt.service + "-" + generateK8sName(10) + "-" + generateK8sName(5)
random = rand.New(rand.NewSource(123))
tt.expected["k8s.pod.name"] = tt.service + "-" + generateK8sName(10, random) + "-" + generateK8sName(5, random)
}

require.Equal(t, tt.expected, *resourceAttrSet.GetAttributes())
random := rand.New(rand.NewSource(123))
require.Equal(t, tt.expected, *resourceAttrSet.GetAttributes(random))
})
}
}
Loading

0 comments on commit 91429ba

Please sign in to comment.