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.

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

(otelarrowreceiver): add updown counters to track number of bytes and items pending #150

Merged

Conversation

moh-osman3
Copy link
Contributor

@moh-osman3 moh-osman3 commented Feb 8, 2024

Resolves #148

This PR adds two metrics to track the number of items and bytes that are pending in the receiver. Specifically when used with the concurrentbatchprocessor requests can be have a long lifetime in receiver due to the in flight byte limiter in the concurrentbatchprocessor.

@moh-osman3 moh-osman3 marked this pull request as ready for review February 8, 2024 20:11
if err := r.processAndConsume(thisCtx, method, ac, req, serverStream, authErr); err != nil {
return err
}
r.recvInFlightRequests.Add(thisCtx, -1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intended that the recvInFlightRequests up-down counter is never decremented when processAndConsume returns an error?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest adding the increment to the top of processAndConsume, with a defer func() { decrement() } to undo that count.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds a good idea :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that @lquerel! This should be fixed now

@lquerel lquerel requested a review from jmacd February 8, 2024 22:36
@moh-osman3 moh-osman3 force-pushed the mohosman/inflight-updown-counter branch from d126d71 to 4528377 Compare February 9, 2024 00:30
Copy link
Contributor

@lquerel lquerel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@jmacd jmacd merged commit 074ff65 into open-telemetry:main Feb 9, 2024
2 checks passed
jmacd added a commit that referenced this pull request Feb 14, 2024
Includes instrumentation changes, core collector upgrade and some
cleanup.

Included PRs:
- #146
- #149
- #150
- #151
- #153

---------

Co-authored-by: Joshua MacDonald <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add metrics to track in-flight bytes at OTel-Arrow receiver
3 participants