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

Add stats for DiscoveryClient #1282

Merged

Conversation

yobennett
Copy link
Contributor

@yobennett yobennett commented Mar 31, 2020

We introduce a nested class for DiscoveryClient to make it easier for users to read helpful attributes that can assist in log analysis and debugging. Most of these attributes are captured by existing metrics:

  • initTimestampMs: timestamp when the client was initialized
  • localRegistrySize: number of instances for all applications
  • lastSuccessfulRegistryFetchTimestampMs: timestamp for last successful fetch
  • lastSuccessfulHeartbeatTimestampMs: timestamp for last successful heartbeat

We add a new attribute called initLocalRegistrySize, which is the number of instances for all applications read when the client was initialized. We include a helper method that uses this attribute so that users can determine whether the client's initial fetch of registry information succeeded or failed. Note that other accessors are suffixed with Ms (for milliseconds) to denote the time unit.

Additionally, we update the existing registrySize with the value from initLocalRegistrySize during initialization instead of waiting until the first local registry refresh.

Copy link
Contributor

@troshko111 troshko111 left a comment

Choose a reason for hiding this comment

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

LGTM.

We introduce a nested class for `DiscoveryClient` to make it easier for
users to read helpful attributes that can assist in log analysis and
debugging. Most of these attributes are captured by existing metrics:

* `initTimestampMs`: timestamp when the client was initialized
* `localRegistrySize`: number of instances for all applications
* `lastSuccessfulRegistryFetchTimestampMs`: timestamp for last successful fetch
* `lastSuccessfulHeartbeatTimestampMs`: timestamp for last successful heartbeat

We add a new attribute called `initLocalRegistrySize`, which is the
number of instances for all applications read when the client was
initialized. We include a helper method that uses this attribute so
that users can determine whether the client's initial fetch of registry
information succeeded or failed. Note that other accessors are suffixed
with `Ms` (for milliseconds) to denote the time unit.

Additionally, we update the existing `registrySize` with the value from
`initLocalRegistrySize` during initialization instead of waiting until
the first local registry refresh.
@yobennett yobennett force-pushed the feature/add-discovery-client-stats branch from b64430a to 706c21c Compare April 7, 2020 17:15
@troshko111 troshko111 merged commit cac06f4 into Netflix:master Apr 7, 2020
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.

None yet

2 participants