Skip to content

Commit

Permalink
[chore] Nit-fix exporter.request.MergeSplit() (#12016)
Browse files Browse the repository at this point in the history
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This PR fixes how `capacityLimit` is updated in
`exporter.request.MergeSplit()`.

Before
```
		if srcReq.ld.LogRecordCount() <= capacityLeft {
			if destReq == nil {
				destReq = srcReq
			} else {
				srcReq.ld.ResourceLogs().MoveAndAppendTo(destReq.ld.ResourceLogs())
			}
			capacityLeft -= destReq.ld.LogRecordCount()
			continue
		}
```

After

```
		srcCount := srcReq.ld.LogRecordCount()
		if srcCount <= capacityLeft {
			if destReq == nil {
				destReq = srcReq
			} else {
				srcReq.ld.ResourceLogs().MoveAndAppendTo(destReq.ld.ResourceLogs())
			}
			capacityLeft -= srcCount
			continue
		}
```

With that said, the original implementation does not cause any bug
because "the larger for loop" loops through only two items. In the first
loop, `destReq` is guaranteed to be `nil`, so `capacityLeft` is updated
properly. In the second loop, we jump out of the loop immediately so the
accuracy of `capacityLeft` does not matter any more.

<!-- Issue number if applicable -->
#### Link to tracking issue
NA

<!--Describe what testing was performed and which tests were added.-->
#### Testing
NA

<!--Describe the documentation added.-->
#### Documentation
NA
<!--Please delete paragraphs that you did not use before submitting.-->
  • Loading branch information
sfc-gh-sili authored Jan 5, 2025
1 parent e5e12bf commit 57c6c15
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 8 deletions.
6 changes: 4 additions & 2 deletions exporter/exporterhelper/logs_batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ func (req *logsRequest) MergeSplit(_ context.Context, cfg exporterbatcher.MaxSiz
if srcReq == nil {
continue
}
if srcReq.ld.LogRecordCount() <= capacityLeft {

srcCount := srcReq.ld.LogRecordCount()
if srcCount <= capacityLeft {
if destReq == nil {
destReq = srcReq
} else {
srcReq.ld.ResourceLogs().MoveAndAppendTo(destReq.ld.ResourceLogs())
}
capacityLeft -= destReq.ld.LogRecordCount()
capacityLeft -= srcCount
continue
}

Expand Down
6 changes: 4 additions & 2 deletions exporter/exporterhelper/metrics_batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ func (req *metricsRequest) MergeSplit(_ context.Context, cfg exporterbatcher.Max
if srcReq == nil {
continue
}
if srcReq.md.DataPointCount() <= capacityLeft {

srcCount := srcReq.md.DataPointCount()
if srcCount <= capacityLeft {
if destReq == nil {
destReq = srcReq
} else {
srcReq.md.ResourceMetrics().MoveAndAppendTo(destReq.md.ResourceMetrics())
}
capacityLeft -= destReq.md.DataPointCount()
capacityLeft -= srcCount
continue
}

Expand Down
6 changes: 4 additions & 2 deletions exporter/exporterhelper/traces_batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ func (req *tracesRequest) MergeSplit(_ context.Context, cfg exporterbatcher.MaxS
if srcReq == nil {
continue
}
if srcReq.td.SpanCount() <= capacityLeft {

srcCount := srcReq.td.SpanCount()
if srcCount <= capacityLeft {
if destReq == nil {
destReq = srcReq
} else {
srcReq.td.ResourceSpans().MoveAndAppendTo(destReq.td.ResourceSpans())
}
capacityLeft -= destReq.td.SpanCount()
capacityLeft -= srcCount
continue
}

Expand Down
6 changes: 4 additions & 2 deletions exporter/exporterhelper/xexporterhelper/profiles_batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ func (req *profilesRequest) MergeSplit(_ context.Context, cfg exporterbatcher.Ma
if srcReq == nil {
continue
}
if srcReq.pd.SampleCount() <= capacityLeft {

srcCount := srcReq.pd.SampleCount()
if srcCount <= capacityLeft {
if destReq == nil {
destReq = srcReq
} else {
srcReq.pd.ResourceProfiles().MoveAndAppendTo(destReq.pd.ResourceProfiles())
}
capacityLeft -= destReq.pd.SampleCount()
capacityLeft -= srcCount
continue
}

Expand Down

0 comments on commit 57c6c15

Please sign in to comment.