Skip to content

Commit d6ed101

Browse files
authored
Remove Logger parameter in adaptive/aggregator.go (jaegertracing#6586)
## Which problem is this PR solving? - jaegertracing#6584 ## Description of the changes - edit `HandleRootSpan` and it's mocks to use `aggregator` parameter logger ## How was this change tested? - `make lint test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` Signed-off-by: nabil salah <[email protected]>
1 parent 156a59d commit d6ed101

File tree

7 files changed

+15
-17
lines changed

7 files changed

+15
-17
lines changed

cmd/collector/app/collector.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func (c *Collector) Start(options *flags.CollectorOptions) error {
9191
var additionalProcessors []ProcessSpan
9292
if c.samplingAggregator != nil {
9393
additionalProcessors = append(additionalProcessors, func(span *model.Span, _ /* tenant */ string) {
94-
c.samplingAggregator.HandleRootSpan(span, c.logger)
94+
c.samplingAggregator.HandleRootSpan(span)
9595
})
9696
}
9797

cmd/collector/app/collector_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func (t *mockAggregator) RecordThroughput(string /* service */, string /* operat
9292
t.callCount.Add(1)
9393
}
9494

95-
func (t *mockAggregator) HandleRootSpan(*model.Span, *zap.Logger) {
95+
func (t *mockAggregator) HandleRootSpan(*model.Span) {
9696
t.callCount.Add(1)
9797
}
9898

cmd/jaeger/internal/processors/adaptivesampling/processor.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func (tp *traceProcessor) processTraces(_ context.Context, td ptrace.Traces) (pt
7171
if span.Process == nil {
7272
span.Process = batch.Process
7373
}
74-
tp.aggregator.HandleRootSpan(span, tp.telset.Logger)
74+
tp.aggregator.HandleRootSpan(span)
7575
}
7676
}
7777
return td, nil

cmd/jaeger/internal/processors/adaptivesampling/processor_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ type notClosingAgg struct{}
140140

141141
func (*notClosingAgg) Close() error { return errors.New("not closing") }
142142

143-
func (*notClosingAgg) HandleRootSpan(*model.Span, *zap.Logger) {}
143+
func (*notClosingAgg) HandleRootSpan(*model.Span) {}
144144
func (*notClosingAgg) RecordThroughput(string, string, model.SamplerType, float64) {}
145145
func (*notClosingAgg) Start() {}
146146

internal/sampling/samplingstrategy/adaptive/aggregator.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ func (a *aggregator) Close() error {
140140
return nil
141141
}
142142

143-
func (a *aggregator) HandleRootSpan(span *span_model.Span, logger *zap.Logger) {
143+
func (a *aggregator) HandleRootSpan(span *span_model.Span) {
144144
// simply checking parentId to determine if a span is a root span is not sufficient. However,
145145
// we can be sure that only a root span will have sampler tags.
146146
if span.ParentSpanID() != span_model.NewSpanID(0) {
@@ -150,7 +150,7 @@ func (a *aggregator) HandleRootSpan(span *span_model.Span, logger *zap.Logger) {
150150
if service == "" || span.OperationName == "" {
151151
return
152152
}
153-
samplerType, samplerParam := span.GetSamplerParams(logger)
153+
samplerType, samplerParam := span.GetSamplerParams(a.postAggregator.logger)
154154
if samplerType == span_model.SamplerTypeUnrecognized {
155155
return
156156
}

internal/sampling/samplingstrategy/adaptive/aggregator_test.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -121,28 +121,28 @@ func TestRecordThroughput(t *testing.T) {
121121

122122
// Testing non-root span
123123
span := &model.Span{References: []model.SpanRef{{SpanID: model.NewSpanID(1), RefType: model.ChildOf}}}
124-
a.HandleRootSpan(span, logger)
124+
a.HandleRootSpan(span)
125125
require.Empty(t, a.(*aggregator).currentThroughput)
126126

127127
// Testing span with service name but no operation
128128
span.References = []model.SpanRef{}
129129
span.Process = &model.Process{
130130
ServiceName: "A",
131131
}
132-
a.HandleRootSpan(span, logger)
132+
a.HandleRootSpan(span)
133133
require.Empty(t, a.(*aggregator).currentThroughput)
134134

135135
// Testing span with service name and operation but no probabilistic sampling tags
136136
span.OperationName = http.MethodGet
137-
a.HandleRootSpan(span, logger)
137+
a.HandleRootSpan(span)
138138
require.Empty(t, a.(*aggregator).currentThroughput)
139139

140140
// Testing span with service name, operation, and probabilistic sampling tags
141141
span.Tags = model.KeyValues{
142142
model.String("sampler.type", "probabilistic"),
143143
model.String("sampler.param", "0.001"),
144144
}
145-
a.HandleRootSpan(span, logger)
145+
a.HandleRootSpan(span)
146146
assert.EqualValues(t, 1, a.(*aggregator).currentThroughput["A"][http.MethodGet].Count)
147147
}
148148

@@ -162,27 +162,27 @@ func TestRecordThroughputFunc(t *testing.T) {
162162

163163
// Testing non-root span
164164
span := &model.Span{References: []model.SpanRef{{SpanID: model.NewSpanID(1), RefType: model.ChildOf}}}
165-
a.HandleRootSpan(span, logger)
165+
a.HandleRootSpan(span)
166166
require.Empty(t, a.(*aggregator).currentThroughput)
167167

168168
// Testing span with service name but no operation
169169
span.References = []model.SpanRef{}
170170
span.Process = &model.Process{
171171
ServiceName: "A",
172172
}
173-
a.HandleRootSpan(span, logger)
173+
a.HandleRootSpan(span)
174174
require.Empty(t, a.(*aggregator).currentThroughput)
175175

176176
// Testing span with service name and operation but no probabilistic sampling tags
177177
span.OperationName = http.MethodGet
178-
a.HandleRootSpan(span, logger)
178+
a.HandleRootSpan(span)
179179
require.Empty(t, a.(*aggregator).currentThroughput)
180180

181181
// Testing span with service name, operation, and probabilistic sampling tags
182182
span.Tags = model.KeyValues{
183183
model.String("sampler.type", "probabilistic"),
184184
model.String("sampler.param", "0.001"),
185185
}
186-
a.HandleRootSpan(span, logger)
186+
a.HandleRootSpan(span)
187187
assert.EqualValues(t, 1, a.(*aggregator).currentThroughput["A"][http.MethodGet].Count)
188188
}

internal/sampling/samplingstrategy/aggregator.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ package samplingstrategy
66
import (
77
"io"
88

9-
"go.uber.org/zap"
10-
119
"github.com/jaegertracing/jaeger/model"
1210
)
1311

@@ -18,7 +16,7 @@ type Aggregator interface {
1816

1917
// The HandleRootSpan function processes a span, checking if it's a root span.
2018
// If it is, it extracts sampler parameters, then calls RecordThroughput.
21-
HandleRootSpan(span *model.Span, logger *zap.Logger)
19+
HandleRootSpan(span *model.Span)
2220

2321
// RecordThroughput records throughput for an operation for aggregation.
2422
RecordThroughput(service, operation string, samplerType model.SamplerType, probability float64)

0 commit comments

Comments
 (0)