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

Report metrics for in-flight requests #486

Merged
merged 3 commits into from
Mar 4, 2020

Conversation

carterkozak
Copy link
Contributor

Before this PR

Difficult to tell how many requests were actively in flight.

After this PR

==COMMIT_MSG==
Report metrics for in-flight requests
==COMMIT_MSG==

Possible downsides?

metrics ain't free?

@changelog-app
Copy link

changelog-app bot commented Mar 4, 2020

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Report metrics for in-flight requests

Check the box to generate changelog(s)

  • Generate changelog entry

@carterkozak
Copy link
Contributor Author

Draft, still needs test coverage.

A couple open questions before I clean it up, will add them inline.

requests.active:
type: counter
tags: [stage]
docs: Number of requests that are actively running. The stage may refer to 'in-flight' requests actively executing over the wire, or 'processing' which may be awaiting a client or backing off for a retry.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking for feedback on naming dialogue.client.requests.active should probably use singular request.
stage in-flight: maybe rename to wire.
processing is fairly vague, but as such it's not going to be incorrect.

Copy link
Contributor

Choose a reason for hiding this comment

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

request.active seems alright. Agree in-flight is a bit confusing since from a users perspective the request is in-flight as soon as it hits our code. Perhaps running and queued for consistency with CJR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like it, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh hmm, queued isn't quite right because it includes everything, both queued and running.

@carterkozak carterkozak marked this pull request as ready for review March 4, 2020 21:45
@ferozco
Copy link
Contributor

ferozco commented Mar 4, 2020

👍

@bulldozer-bot bulldozer-bot bot merged commit f129c6f into develop Mar 4, 2020
@bulldozer-bot bulldozer-bot bot deleted the ckozak/inflight_request_metrics branch March 4, 2020 22:56
@svc-autorelease
Copy link
Collaborator

Released 0.14.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants