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.

Sign up for GitHub

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

Jump to bottom

emitBatchOverhead should only be used for splitting spans into batches #2512

Merged
merged 3 commits into from Jan 18, 2022
Merged

emitBatchOverhead should only be used for splitting spans into batches #2512

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
baa4427
emitBatchOverhead should only be used for splitting spans into batche…
ken8203 Jan 14, 2022
765ea21
limit max packet size parameter
ken8203 Jan 14, 2022
3f414cb
Merge branch 'main' into bugfix/exports-jaeger-max-packet-size
ken8203 Jan 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Expand Up @@ -15,7 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Changed

- Jaeger exporter takes into additional 70 bytes overhead into consideration when sending UDP packets (#2489)
- Jaeger exporter takes into additional 70 bytes overhead into consideration when sending UDP packets (#2489, #2512)

### Deprecated

Expand Down
13 changes: 9 additions & 4 deletions exporters/jaeger/agent.go
View file
Open in desktop
Expand Up @@ -71,8 +71,8 @@ func newAgentClientUDP(params agentClientUDPParams) (*agentClientUDP, error) {
return nil, err
}

if params.MaxPacketSize <= 0 {
params.MaxPacketSize = udpPacketMaxLength - emitBatchOverhead
if params.MaxPacketSize <= 0 || params.MaxPacketSize > udpPacketMaxLength {
params.MaxPacketSize = udpPacketMaxLength
}

if params.AttemptReconnecting && params.AttemptReconnectInterval <= 0 {
Expand Down Expand Up @@ -126,6 +126,11 @@ func (a *agentClientUDP) EmitBatch(ctx context.Context, batch *gen.Batch) error
// drop the batch if serialization of process fails.
return err
}

maxPacketSize := a.maxPacketSize
if maxPacketSize > udpPacketMaxLength-emitBatchOverhead {
maxPacketSize = udpPacketMaxLength - emitBatchOverhead
}
totalSize := processSize
var spans []*gen.Span
for _, span := range batch.Spans {
Expand All @@ -134,12 +139,12 @@ func (a *agentClientUDP) EmitBatch(ctx context.Context, batch *gen.Batch) error
errs = append(errs, fmt.Errorf("thrift serialization failed: %v", span))
continue
}
if spanSize+processSize >= a.maxPacketSize {
if spanSize+processSize >= maxPacketSize {
// drop the span that exceeds the limit.
errs = append(errs, fmt.Errorf("span too large to send: %v", span))
continue
}
if totalSize+spanSize >= a.maxPacketSize {
if totalSize+spanSize >= maxPacketSize {
if err := a.flush(ctx, &gen.Batch{
Process: batch.Process,
Spans: spans,
Expand Down
4 changes: 2 additions & 2 deletions exporters/jaeger/agent_test.go
View file
Open in desktop
Expand Up @@ -73,7 +73,7 @@ func TestNewAgentClientUDPWithParamsDefaults(t *testing.T) {
})
assert.NoError(t, err)
assert.NotNil(t, agentClient)
assert.Equal(t, udpPacketMaxLength-emitBatchOverhead, agentClient.maxPacketSize)
assert.Equal(t, udpPacketMaxLength, agentClient.maxPacketSize)

if assert.IsType(t, &reconnectingUDPConn{}, agentClient.connUDP) {
assert.Equal(t, (*log.Logger)(nil), agentClient.connUDP.(*reconnectingUDPConn).logger)
Expand All @@ -97,7 +97,7 @@ func TestNewAgentClientUDPWithParamsReconnectingDisabled(t *testing.T) {
})
assert.NoError(t, err)
assert.NotNil(t, agentClient)
assert.Equal(t, udpPacketMaxLength-emitBatchOverhead, agentClient.maxPacketSize)
assert.Equal(t, udpPacketMaxLength, agentClient.maxPacketSize)

assert.IsType(t, &net.UDPConn{}, agentClient.connUDP)

Expand Down