-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
ReactiveSocket metrics client/server #1236
ReactiveSocket metrics client/server #1236
Conversation
mattrjacobs
commented
Jun 10, 2016
- Client/Server in hystrix-examples-reactivesocket
- Improvements to HystrixCommandAsyncDemo
- Bugfixes/polishing to hystrix-reactivesocket-metric-stream. Sources should support arbitrary sharing between 0-n subscribers that may come and go. When subscriber count drops to 0, source does no work
* Client/Server in hystrix-examples-reactivesocket * Improvements to HystrixCommandAsyncDemo * Bugfixes/polishing to hystrix-reactivesocket-metric-stream. Sources should support arbitrary sharing between 0-n subscribers that may come and go. When subscriber count drops to 0, source does no work
/cc @robertroeser @neerajrj @nickmahilani - Could you take a look? |
collapserConfigMap.put(collapserKey, collapserConfig); | ||
} | ||
} catch (IOException ioe) { | ||
System.out.println("IO Exception : " + ioe); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can log with logger here
Good catch @nickmahilani - just found 2 others as well |
.map(SerialHystrixMetric::toPayload); | ||
break; | ||
case GENERAL_DASHBOARD_STREAM: | ||
source = HystrixDashboardStream.getInstance() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my understanding, why is the dashboard stream delay not configurable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Singletons for util/config stream to allow subscribers to share w/ archaius sounds like a good approach.
Right now, the utilization and configuration streams always accept a stream delay and are non-singletons. Looking back, this is probably a mistake as it makes it harder for multiple subscribers to share. I think there are 2 more tasks (coming next week):
|
just couple minor comments, PR looks good to me 👍 |