grpc metrics and minor refactoring #62

laxmanchekka · 2022-12-07T19:23:04Z

Description

Added grpc server/client metrics monitoring.

Testing

Tested manually

Checklist:

My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
Any dependent changes have been merged and published in downstream modules

Documentation

NA

* refactor: example * upgrade grpc-utils lib to the latest Co-authored-by: Laxman Ch <laxman@traceable.ai>

codecov · 2022-12-07T19:25:56Z

Codecov Report

Merging #62 (46f57de) into main (11e9213) will not change coverage.
The diff coverage is 94.28%.

@@            Coverage Diff            @@
##               main      #62   +/-   ##
=========================================
  Coverage     69.50%   69.50%           
+ Complexity      106      105    -1     
=========================================
  Files            15       15           
  Lines           564      564           
  Branches         32       33    +1     
=========================================
  Hits            392      392           
  Misses          153      153           
  Partials         19       19

Flag	Coverage Δ
unit	`69.50% <94.28%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...viceframework/metrics/PlatformMetricsRegistry.java	`82.78% <94.28%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

laxmanchekka · 2022-12-07T19:29:31Z

Example metrics

Server metrics

# HELP grpc_server_responses_sent_messages_total The total number of responses sent
# TYPE grpc_server_responses_sent_messages_total counter
grpc_server_responses_sent_messages_total{app="query-service",method="execute",methodType="SERVER_STREAMING",service="org.hypertrace.core.query.service.QueryService",} 135.0
grpc_server_responses_sent_messages_total{app="query-service",method="Check",methodType="UNARY",service="grpc.health.v1.Health",} 98.0

Client metrics

# HELP grpc_client_processing_duration_seconds The total time taken for the client to complete the call, including network delay
# TYPE grpc_client_processing_duration_seconds summary
grpc_client_processing_duration_seconds_count{app="query-service",method="findAttributes",methodType="SERVER_STREAMING",service="org.hypertrace.core.attribute.service.v1.AttributeService",statusCode="OK",} 1.0
grpc_client_processing_duration_seconds_sum{app="query-service",method="findAttributes",methodType="SERVER_STREAMING",service="org.hypertrace.core.attribute.service.v1.AttributeService",statusCode="OK",} 0.393107214
grpc_client_processing_duration_seconds_count{app="query-service",method="Check",methodType="UNARY",service="grpc.health.v1.Health",statusCode="OK",} 98.0
grpc_client_processing_duration_seconds_sum{app="query-service",method="Check",methodType="UNARY",service="grpc.health.v1.Health",statusCode="OK",} 0.26274974
# HELP grpc_client_processing_duration_seconds_max The total time taken for the client to complete the call, including network delay
# TYPE grpc_client_processing_duration_seconds_max gauge
grpc_client_processing_duration_seconds_max{app="query-service",method="findAttributes",methodType="SERVER_STREAMING",service="org.hypertrace.core.attribute.service.v1.AttributeService",statusCode="OK",} 0.393107214
grpc_client_processing_duration_seconds_max{app="query-service",method="Check",methodType="UNARY",service="grpc.health.v1.Health",statusCode="OK",} 0.003172604

aaron-steinfeld · 2022-12-07T19:52:40Z

...ervice-framework/src/test/java/org/hypertrace/core/serviceframework/PlatformServiceTest.java

  @Test
  public void testMetricInitialization() {
-    PlatformService service = getService(Map.of("service.name", "test-service",
+    PlatformService service = getService(Map.of("service.name", "sample-app",


any reason for the changes in here? The default tag change should be transparent (And is, if I'm reading the test changes correctly)

With new changes, these tests are failing when they are tested with different default tags.
These tests are simulating a reload (lifecycle stop and start) kind of scenario. However, for commonTags there is no provision for cleanup/destroy. We need to destroy the registry itself and recreate it.

By using the same app name we are bypassing the test order dependencies.

Isn't that a caught regression though? In the prior implementation, it reset its state on start/stop and it no longer does. Seems like it would be easy enough to reset the meter registry, right?

aaron-steinfeld · 2022-12-07T20:05:56Z

...rics/src/main/java/org/hypertrace/core/serviceframework/metrics/PlatformMetricsRegistry.java

-    if (tags == null || tags.isEmpty()) {
-      return DEFAULT_TAGS;
+  private static Iterable<Tag> toIterable(Map<String, String> tags) {
+    Set<Tag> newTags = new HashSet<>();


any concern about replicable ordering? Could just as easily use a list

aaron-steinfeld · 2022-12-07T20:52:54Z

...rics/src/main/java/org/hypertrace/core/serviceframework/metrics/PlatformMetricsRegistry.java

-    Set<MeterRegistry> registries = new HashSet<>(METER_REGISTRY.getRegistries());
-    registries.forEach(METER_REGISTRY::remove);
+    meterRegistry.getRegistries().forEach(MeterRegistry::close);
+    meterRegistry.forEachMeter(meterRegistry::remove);


nit - a lot of this code - 430-434 I believe - is redundant now that we're just replacing the whole registry. We don't need to go through and clear stuff out any more.

github-actions · 2022-12-08T02:50:46Z

Unit Test Results

  9 files ±0   9 suites ±0 11s ⏱️ -2s
31 tests ±0 31 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 177d547. ± Comparison against base commit 11e9213.

laxmanchekka and others added 3 commits December 6, 2022 18:36

grpc metrics

5320c92

Grpc metrics suggestions (#61)

42596f2

* refactor: example * upgrade grpc-utils lib to the latest Co-authored-by: Laxman Ch <laxman@traceable.ai>

refactor default tags mechanism

fa97442

laxmanchekka requested a review from aaron-steinfeld December 7, 2022 19:23

laxmanchekka requested review from avinashkolluru, surajpuvvada and tim-mwangi as code owners December 7, 2022 19:23

This comment has been minimized.

Sign in to view

aaron-steinfeld reviewed Dec 7, 2022

View reviewed changes

review comments fix

46f57de

This comment has been minimized.

Sign in to view

aaron-steinfeld reviewed Dec 7, 2022

View reviewed changes

aaron-steinfeld approved these changes Dec 7, 2022

View reviewed changes

surajpuvvada approved these changes Dec 8, 2022

View reviewed changes

laxmanchekka merged commit 177d547 into main Dec 8, 2022

laxmanchekka deleted the grpc-metrics branch December 8, 2022 02:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

grpc metrics and minor refactoring #62

grpc metrics and minor refactoring #62

Uh oh!

laxmanchekka commented Dec 7, 2022

Uh oh!

This comment has been minimized.

codecov bot commented Dec 7, 2022 •

edited

Loading

Uh oh!

laxmanchekka commented Dec 7, 2022

Uh oh!

aaron-steinfeld Dec 7, 2022

Uh oh!

laxmanchekka Dec 7, 2022 •

edited

Loading

Uh oh!

aaron-steinfeld Dec 7, 2022

Uh oh!

aaron-steinfeld Dec 7, 2022

Uh oh!

This comment has been minimized.

aaron-steinfeld Dec 7, 2022

Uh oh!

github-actions bot commented Dec 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

grpc metrics and minor refactoring #62

grpc metrics and minor refactoring #62

Uh oh!

Conversation

laxmanchekka commented Dec 7, 2022

Description

Testing

Checklist:

Documentation

Uh oh!

This comment has been minimized.

codecov bot commented Dec 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

laxmanchekka commented Dec 7, 2022

Uh oh!

aaron-steinfeld Dec 7, 2022

Choose a reason for hiding this comment

Uh oh!

laxmanchekka Dec 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaron-steinfeld Dec 7, 2022

Choose a reason for hiding this comment

Uh oh!

aaron-steinfeld Dec 7, 2022

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

aaron-steinfeld Dec 7, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 8, 2022

Unit Test Results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Dec 7, 2022 •

edited

Loading

laxmanchekka Dec 7, 2022 •

edited

Loading