javamelody
javamelody copied to clipboard
support for headers when using collector server to pull data from monitored clients.
During registration with the collector server, monitored clients can now provid a map of headers. This will be used by collector server when calling the monitored clients. This scenario is useful for example in cloud foundry, when there are many instances of an application behind a load balancer and we want to monitor each application separately. For such scenarios, cloud foundry uses a header X-CF-APP-INSTANCE to identify the specific instance.
I understand the goal.
At the moment, it will only work for 1 minute, because headers are not stored for later data collects in CollectorServer.collectWithoutErrors() and headers are not stored on disk either in case of collector server restart.
And I would probably prefer to have headers in the URL, than java serialization, when registering the app instance.
Ok. I agree that having it in the URL is a better option. I will change it that way.
Can someone review this please?
ok, I will review
A few remarks:
- It does not compile.
- You don't need to keep the method CollectorServer.collectForApplicationWithoutErrors(String, List<URL>)
- Rename Parameters.getCollectorUrlsHeadersByApplication() to Parameters.getCollectorHeadersByApplications() with s at the end
- Add a method CollectorServer.getHeadersByApplication(String). The headers you have saved may be useful.
- You don't need the appHeader parameter in CollectorServer.removeCollectorApplicationNodes(...) and it is probably not given in the request anyway. Use getHeadersByApplication(appName) instead.
- new RemoteCollector(String, List<URL>) is used in collectForApplicationForAction and new LabradorRetriever(URL) is used in doJmxValue and doProxy. These use cases should have the headers too.
A few remarks:
- It does not compile.
- You don't need to keep the method CollectorServer.collectForApplicationWithoutErrors(String, List)
- Rename Parameters.getCollectorUrlsHeadersByApplication() to Parameters.getCollectorHeadersByApplications() with s at the end
- Add a method CollectorServer.getHeadersByApplication(String). The headers you have saved may be useful.
- You don't need the appHeader parameter in CollectorServer.removeCollectorApplicationNodes(...) and it is probably not given in the request anyway. Use getHeadersByApplication(appName) instead.
- new RemoteCollector(String, List) is used in collectForApplicationForAction and new LabradorRetriever(URL) is used in doJmxValue and doProxy. These use cases should have the headers too.
Thanks for the review. I already fixed all of them except the compilation error. I could not see any compilation issue. Can you tell me exactly where the compilation error is? Are you using java 8 ?
The Maven's pom.xml uses the Java 6 syntax for source and targets Java 6. But sorry for "not compile" remark; my IDE has rejected some diffs from 835.diff and I did not checked. Changing the Fuzz factor gives a better results. There is no more problem with that.
@evernat Can we do something on this?
@evernat I just rebased from master. I know you might be busy but can you have a quick look on this so we can finally merge this PR. Thanks again.
Hi, I'm very interested in this subject, because I have the same need to identify each instance of my web application through the use of headers. Is it possible to advance this Pull Request?
Thank You
Julien
Hi @evernat
If necessary I can try to resume the branch to apply the remarks indicated, but I do not have access to it. Thank You.