[DO NOT MERGE] Demonstrate various usages of pdata Range functions #29613
Conversation
github-actions
bot
added
cmd/configschema
github-actions
bot
added
exporter/datadog
|
|
||
| signature := timeseriesSignature(il.Name(), metric, ip.Attributes(), resourceAttrs) | ||
| if ip.Flags().NoRecordedValue() { | ||
| a.registeredMetrics.Delete(signature) | ||
| return 0 |
There was a problem hiding this comment.
This is probably the worst case I ran into. We want to be able to short circuit the loop in order to stop accumulating into n, but we can't use n itself to check if we should stop aggregating. Therefore, a completely unrelated variable is introduced to control whether the loop actually executes.
| if err != nil { | ||
| return err | ||
| return // Would use RangeWhile instead if available |
There was a problem hiding this comment.
This was another situation where not being able to short circuit the loop resulted in unintuitive code. Instead of immediately returning an error, we have to save it, and then use it to determine whether or not each subsequent iteration should actually do anything.
|
Here are the improvements we could make here if using a |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Demonstrates functions proposed in open-telemetry/opentelemetry-collector#8938
The main takeaway for me is that
Rangeis quite useful, but there are quite a lot of situations where short circuiting the loop would be useful. It's possible to handle these with theRangefunc as is, but it feels unintuitive because it has to rely on checking an external value in each iteration. I think it's worth exploring whether it feels too intrusive to add a bool to indicate continuation of the loop.