Skip to content

Conversation

@ewencp
Copy link
Contributor

@ewencp ewencp commented Jan 20, 2015

Add some basic metrics that work for any rest-utils application. The two commits add metrics at different levels. The first adds some basic TCP connection-level stats by hooking into Jetty, and the second adds HTTP level stats by hooking into Jersey.

It's probably easiest to see what there is by looking at the JMX stats for the example application. A few important questions for review:

  1. What else should we include by default, i.e. are any key metrics still missing? More metrics to add on existing sensors? Latency 95th/99th/99.9th percentiles are the obvious ones I can think of, but it's unclear to me how good the current implementation of percentiles is.
  2. Metrics names/organization. The metrics library has some support for hierarchical Sensors with aggregation, but the ordering of names felt backwards. It led to names like request-rate.TopicResource.produce so that all requests could be aggregated into request-rate, but that seemed wrong when I also wanted multiple metrics grouped under TopicResource.produce. The current patch just has extra sensors that are always used for the global stats.
  3. Names are always autogenerated via class names and method names. Is this what we want, or should we allow/encourage/force better naming via attributes?

@ewencp
Copy link
Contributor Author

ewencp commented Jan 20, 2015

@nehanarkhede Thoughts on this? Ideally we can get a good set to cover most of what both kafka-rest and schema-registry need.

@ewencp
Copy link
Contributor Author

ewencp commented Jan 22, 2015

Blocked on confluentinc/common#11

@ewencp
Copy link
Contributor Author

ewencp commented Jan 28, 2015

@nehanarkhede Ready for you to look at this again. confluentinc/kafka-rest#23 shows how it gets used. There are a couple of changes required because I had to rework some rest-utils classes (we'll also have to make them in schema-registry), but the major change to users of rest-utils is just annotating the resource methods with better names.

Conflicts:
	core/src/main/java/io/confluent/rest/RestConfig.java
@ewencp
Copy link
Contributor Author

ewencp commented Jan 29, 2015

Merging based on review of confluentinc/kafka-rest#23.

ewencp added a commit that referenced this pull request Jan 29, 2015
@ewencp ewencp merged commit 9d45c0f into master Jan 29, 2015
@ewencp ewencp deleted the request-metrics branch January 29, 2015 17:50
mapr-devops pushed a commit to mapr/rest-utils that referenced this pull request Aug 23, 2025
…confluentinc#6)

Co-authored-by: Prince Raheja <114437476+rahejaprince@users.noreply.github.com>
Co-authored-by: ConfluentSemaphore <40306929+ConfluentSemaphore@users.noreply.github.com>
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.

2 participants