javamelody icon indicating copy to clipboard operation
javamelody copied to clipboard

support for headers when using collector server to pull data from monitored clients.

Open batigoal82 opened this issue 6 years ago • 11 comments

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.

batigoal82 avatar May 12 '19 21:05 batigoal82

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.

evernat avatar May 23 '19 00:05 evernat

Ok. I agree that having it in the URL is a better option. I will change it that way.

batigoal82 avatar Jun 06 '19 15:06 batigoal82

Can someone review this please?

batigoal82 avatar Jun 27 '19 07:06 batigoal82

ok, I will review

evernat avatar Jun 27 '19 19:06 evernat

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.

evernat avatar Jun 29 '19 11:06 evernat

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 ?

batigoal82 avatar Jul 01 '19 14:07 batigoal82

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 avatar Jul 01 '19 18:07 evernat

@evernat Can we do something on this?

batigoal82 avatar Jul 30 '19 07:07 batigoal82

@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.

batigoal82 avatar Sep 22 '19 20:09 batigoal82

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

ricetrac avatar Sep 08 '22 07:09 ricetrac

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.

ricetrac avatar Sep 15 '22 07:09 ricetrac