helifu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13376 )

Change subject: [util] Support for entity type/id/attrs/metrics filtering in 
metrics
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13376/2/src/kudu/util/metrics.h
File src/kudu/util/metrics.h:

http://gerrit.cloudera.org:8080/#/c/13376/2/src/kudu/util/metrics.h@409
PS2, Line 409: struct MetricJsonOptions {
> Can you move this functionality into MetricJsonOptions? I don't really see
Done


http://gerrit.cloudera.org:8080/#/c/13376/2/src/kudu/util/metrics.h@666
PS2, Line 666:   // For each registered entity, retires orphaned metrics. If an 
entity has no more
> Since 'opts' is going to gain more advanced filtering capabilities, would b
Done


http://gerrit.cloudera.org:8080/#/c/13376/1/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:

http://gerrit.cloudera.org:8080/#/c/13376/1/src/kudu/util/metrics.cc@220
PS1, Line 220: // Quick check the 'attributes'.
> This line of code is a bit strange, except that it skips matching the follo
Ah, i see. It is used to filter entity id. :)


http://gerrit.cloudera.org:8080/#/c/13376/2/src/kudu/util/metrics.cc
File src/kudu/util/metrics.cc:

http://gerrit.cloudera.org:8080/#/c/13376/2/src/kudu/util/metrics.cc@205
PS2, Line 205: } // anonymous namespace
> Can we make this patch more generic to matching entity types/entity ids? Th
Done



--
To view, visit http://gerrit.cloudera.org:8080/13376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45ca55bd1f0f3d4107cb208f6f855d24121823ee
Gerrit-Change-Number: 13376
Gerrit-PatchSet: 3
Gerrit-Owner: helifu <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: helifu <[email protected]>
Gerrit-Comment-Date: Tue, 21 May 2019 11:56:39 +0000
Gerrit-HasComments: Yes

Reply via email to