Skip to content

More flexible contract for PublicMetrics #1094

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.

Sign up for GitHub

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

Jump to bottom
Closed
snicoll opened this issue Jun 13, 2014 · 4 comments
Closed

More flexible contract for PublicMetrics #1094

snicoll opened this issue Jun 13, 2014 · 4 comments
Assignees
@snicoll
Labels
type: enhancement A general enhancement
Milestone
1.2.0.M1

Comments

@snicoll
Copy link
Member

snicoll commented Jun 13, 2014

The current approach for PublicMetrics is to locate a bean of that type and request its metrics every time the health endpoint is invoked.

This does not allow for others to contribute metrics. A typical example is #1013 where I would have to add these to the only PublicMetrics implementation we have.

Instead we should create a composite and locale all instances by default. Of course, a user may redefine that special bean by name to override this behaviour.
@snicoll snicoll added this to the 1.2.0 milestone Jun 13, 2014
@snicoll snicoll self-assigned this Jun 13, 2014
snicoll added a commit to snicoll/spring-boot that referenced this issue Jun 13, 2014
@snicoll
Flexible registration of additional PublicMetrics
97a7c52 
This commit permits the use of several PublicMetrics instances by
default. Previously, only one PublicMetrics service could be specified
and a user configuration would remove all the defaulting.

VanillaPublicMetrics now takes a collection of PublicMetrics and
invokes them in sequence to build the final collection of metrics.
The system-related metrics have been moved to SystemPublicMetrics and
are registered by default.

Also updated the documentation to mention this feature and how it
could be fully overridden.

Fixes spring-projectsgh-1094
@gesellix
Copy link
Contributor

👍

Another example would be to contribute metrics from a MetricsRegistry, in cases where one uses Codahale Metrics or even Ryan Tenney's metrics-spring.

We currently extend the MetricsEndpoint to merge PublicMetrics with a MetricRegistry which doesn't feel like the best way.

@snicoll
Copy link
Member Author

snicoll commented Jun 17, 2014

I didn't thought of that one. Nice! So you confirm this works fine for you? (in theory I mean, unless you pulled my branch to try)

@gesellix
Copy link
Contributor

I didn't have a chance to test your implementation, but I will and leave a note here.

@snicoll
Copy link
Member Author

snicoll commented Jun 17, 2014

Awesome, thanks!

@markfisher markfisher modified the milestones: 1.1.4, 1.2.0 Jul 3, 2014
@markfisher markfisher reopened this Jul 3, 2014
@philwebb philwebb removed this from the 1.1.4 milestone Jul 3, 2014
@snicoll snicoll added this to the 1.2.0 milestone Jul 17, 2014
philwebb pushed a commit that referenced this issue Jul 29, 2014
Refactor PublicMetrics registration
14c6243 
Rework flexible PublicMetrics registration introduced in 2be6b3e to
restore compatibility with v1.1 VanillaPublicMetrics. The new
MetricReaderPublicMetrics class now exposes metrics from a MetricReader
and VanillaPublicMetrics is deprecated. The MetricsEndpoint can now
exposes a collection of PublicMetric interface directly.

See gh-1094
@snicoll snicoll mentioned this issue Aug 25, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@snicoll snicoll

Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
1.2.0.M1
Development

No branches or pull requests
4 participants
@markfisher @gesellix @snicoll @philwebb