fix: jaeger-agent sampling endpoint returns backwards incompatible JSON #3892
Conversation
…`[]` - Returning an empty slice for `perOperationStrategies` instead of the default nil slice allows json.Marshal to marshal it to `[]` instead of `null` - Fixes jaegertracing#3891 Signed-off-by: Prithvi Raj <[email protected]>
vprithvi
changed the title
perOperationStrategies returns []Signed-off-by: Prithvi Raj <[email protected]>
Codecov ReportBase: 97.64% // Head: 97.63% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #3892 +/- ##
==========================================
- Coverage 97.64% 97.63% -0.02%
==========================================
Files 293 293
Lines 17073 17075 +2
==========================================
Hits 16671 16671
- Misses 317 319 +2
Partials 85 85
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| @@ -72,6 +72,9 @@ func convertPerOperationFromDomain(s *api_v2.PerOperationSamplingStrategies) *sa | |||
| for i, k := range s.PerOperationStrategies { | |||
| r.PerOperationStrategies[i] = convertOperationFromDomain(k) | |||
| } | |||
| } else { | |||
| // Initialize to an empty array so that using json.Marshal results in [] instead of null. | |||
| r.PerOperationStrategies = make([]*sampling.OperationSamplingStrategy, 0) | |||
| } | |||
There was a problem hiding this comment.
I think you can simply remove the if statement and only keep its content
perOp := s.GetPerOperationStrategies()
// Default to empty array so that json.Marshal returns [] instead of null (Issue #3891).
r.PerOperationStrategies = make([]*sampling.OperationSamplingStrategy, len(perOp))
for i, k := range perOp {
r.PerOperationStrategies[i] = convertOperationFromDomain(k)
}
Signed-off-by: Prithvi Raj <[email protected]>
|
@yurishkuro ty! The builds seem to be failing when publishing to docker with the following, how to proceed? |
|
ah, it's because you created PR from your main branch, it will continue failing. Needs to be on a named branch. |
I see, I created #3897 with the same contents; |
Pull request was closed
perOperationStrategiesfield contains[]as a default value instead of nil. This was the behavior as of jaeger-agent v1.17 when using tchannel/thrift to communicate with collector.perOperationStrategiesinstead of the default nil slice allowsjson.Marshal to marshal it to
[]instead ofnullSigned-off-by: Prithvi Raj [email protected]