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

batigoal82 · 2019-05-12T21:13:48Z

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.

evernat · 2019-05-23T00:40:46Z

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.

batigoal82 · 2019-06-06T15:10:33Z

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

batigoal82 · 2019-06-27T07:26:13Z

Can someone review this please?

evernat · 2019-06-27T19:48:29Z

ok, I will review

evernat · 2019-06-29T11:51:00Z

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.

batigoal82 · 2019-07-01T14:38:49Z

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 ?

evernat · 2019-07-01T18:21:26Z

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.

batigoal82 · 2019-07-30T07:15:11Z

@evernat Can we do something on this?

When using collector server to pull data from monitored clients, HTTP headers need to be supported. During registration with the collector server, monitored clients can now provide 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 · 2019-09-22T20:21:10Z

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

evernat · 2021-07-16T23:57:34Z

...elody-core/src/main/java/net/bull/javamelody/internal/common/MonitoredApplicationHeader.java

+			String[] headerKeyValue = header.split(":");
+			if (headerKeyValue.length == 2) {
+				headersMap.put(headerKeyValue[0], headerKeyValue[1]);
+			}


I think that this code will break X-Cf-App-Instance for Cloud Foundry, which is like YOUR-APP-GUID:YOUR-INSTANCE-INDEX. where YOUR-APP-GUID is application_id in System.getenv("VCAP_APPLICATION") and YOUR-INSTANCE-INDEX is System.getenv("CF_INSTANCE_INDEX").

See https://docs.cloudfoundry.org/concepts/http-routing.html#app-instance-routing
and https://docs.cloudfoundry.org/devguide/deploy-apps/environment-variable.html

evernat · 2021-07-17T00:00:08Z

...elody-core/src/main/java/net/bull/javamelody/internal/common/MonitoredApplicationHeader.java

+import java.util.Map;
+import java.util.Properties;
+
+import org.apache.logging.log4j.util.Strings;


log4j may not be available in the webapp. And there is no need for this import.

evernat · 2021-07-17T00:00:54Z

...elody-core/src/main/java/net/bull/javamelody/internal/common/MonitoredApplicationHeader.java

+	private void parseHeader(String headers) {
+		String[] headerParts = headers.split(";");
+		for (String header : headerParts) {
+			String[] headerKeyValue = header.split(":");


extract constants : and ;

evernat · 2021-07-17T00:06:43Z

javamelody-core/src/main/java/net/bull/javamelody/internal/model/CollectorServer.java

 		try {
 			clone = new LinkedHashMap<String, List<URL>>(
 					Parameters.getCollectorUrlsByApplications());
+			urlHeaders = Parameters.getCollectorHeadersByApplications();


Clone is needed for headers like for URLs : new LinkedHashMap<>(Parameters.getCollectorHeadersByApplications());
And by definition, URL has no headers. Http request has headers.

evernat · 2021-07-17T00:08:16Z

javamelody-core/src/main/java/net/bull/javamelody/internal/model/CollectorServer.java

+								new MonitoredApplicationHeader(urlHeaderForApplication));
+					}
+				});
+			}


if and else could be merged

ricetrac · 2022-09-08T07:35:32Z

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 · 2022-09-15T07:42:01Z

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.

batigoal82 force-pushed the support-header-for-monitored-clients-fix branch from 4821f9b to 2df4524 Compare May 16, 2019 10:42

batigoal82 force-pushed the support-header-for-monitored-clients-fix branch 3 times, most recently from bb42ed7 to afe32f5 Compare June 26, 2019 09:28

batigoal82 force-pushed the support-header-for-monitored-clients-fix branch from afe32f5 to 20425a4 Compare July 1, 2019 19:05

batigoal82 added a commit to batigoal82/javamelody that referenced this pull request Jul 1, 2019

Issue javamelody#835: fix PR comments

20425a4

batigoal82 force-pushed the support-header-for-monitored-clients-fix branch from 20425a4 to 5a13a97 Compare July 29, 2019 12:11

batigoal82 added a commit to batigoal82/javamelody that referenced this pull request Jul 29, 2019

Issue javamelody#835: fix PR comments

5a13a97

batigoal82 force-pushed the support-header-for-monitored-clients-fix branch from 5a13a97 to dce09ae Compare July 29, 2019 12:27

batigoal82 force-pushed the support-header-for-monitored-clients-fix branch from dce09ae to ebeb7f5 Compare September 22, 2019 19:56

evernat reviewed Jul 16, 2021

View reviewed changes

evernat reviewed Jul 17, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

batigoal82 commented May 12, 2019

evernat commented May 23, 2019

batigoal82 commented Jun 6, 2019 •

edited

batigoal82 commented Jun 27, 2019

evernat commented Jun 27, 2019

evernat commented Jun 29, 2019

batigoal82 commented Jul 1, 2019

evernat commented Jul 1, 2019

batigoal82 commented Jul 30, 2019

batigoal82 commented Sep 22, 2019

evernat Jul 16, 2021

evernat Jul 17, 2021

evernat Jul 17, 2021

evernat Jul 17, 2021 •

edited

evernat Jul 17, 2021

ricetrac commented Sep 8, 2022

ricetrac commented Sep 15, 2022

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

Are you sure you want to change the base?

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

Conversation

batigoal82 commented May 12, 2019

evernat commented May 23, 2019

batigoal82 commented Jun 6, 2019 • edited

batigoal82 commented Jun 27, 2019

evernat commented Jun 27, 2019

evernat commented Jun 29, 2019

batigoal82 commented Jul 1, 2019

evernat commented Jul 1, 2019

batigoal82 commented Jul 30, 2019

batigoal82 commented Sep 22, 2019

evernat Jul 16, 2021

Choose a reason for hiding this comment

evernat Jul 17, 2021

Choose a reason for hiding this comment

evernat Jul 17, 2021

Choose a reason for hiding this comment

evernat Jul 17, 2021 • edited

Choose a reason for hiding this comment

evernat Jul 17, 2021

Choose a reason for hiding this comment

ricetrac commented Sep 8, 2022

ricetrac commented Sep 15, 2022

batigoal82 commented Jun 6, 2019 •

edited

evernat Jul 17, 2021 •

edited