Choose a suitable default port about apm-server HOT 17 CLOSED

@crackerplace Good point about elasticsearch. I thought it can be define in one string but it seems you are right.

For our elasticsearch output we use hosts: https://github.com/elastic/apm-server/blob/master/apm-server.reference.yml#L143 But here the difference is that this can be an array and each host defined can have a different port.

I somehow like that it can be defined in one string but I also see that lots of other places use host and port.

@roncohen Any thoughts?

from apm-server.

OK, let's go with port 8200. I'm opening a separate issue to discuss configuration naming.

from apm-server.

My suggestion: 8200. Elasticsearch's default port is 9200, which makes 8200 easy to remember for people familiar with ES.

from apm-server.

@beniwohli I can work on this change.

Also,if we change host to addr ,I believe it would be better ? This might require change in apm-server.yml too.Not sure if this breaks many things.

from apm-server.

@crackerplace I can definitively see your point on why you would want to rename it to address as this is what is accepted in the end by the server in the Golang code. I would like to keep it host as I'm hoping people will past hostname:port which is AFAIK host. Also I'm probably biased because of ES also using host: https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-network.html#common-network-settings :-) So happy to be convinced otherwise.

from apm-server.

@beniwohli I like your suggestion of 8200. Because it's starting with an 8 it somehow triggers my brain to think http and the other part is nicely close to ES.

from apm-server.

I like 8200 as well. It looks like it's not a very occupied port, but it does look like we'd be sharing the port with GoToMyPC: http://support.citrixonline.com/en_us/meeting/all_files/G2M060010. I don't think that's a deal breaker.

We might want to consider a different default port for when the apm-server is expecting SSL/TLS connections, similar to 80/443 for HTTP/S. AFAK it can be a bit difficult to debug issues caused by the server expecting a TLS connection but the agents using cleartext HTTP and vice-versa. Elasticsearch itself doesn't have separate ports for TLS/cleartext, which is probably a good indication that we shouldn't either - unless they regret that choice ;)

from apm-server.

@ruflin Separating host and port might be better instead of my former view point of using addr as addr looks more apt when connecting to external servers such as done in client libraries for redis etc.

I understand your reasoning as well and that sounds ok too.
Just a point,even elasticsearch has network.host and http.port.I don't think port is part of the host.

from apm-server.

I am in a middle of server refactor right now, I will make 8200 default unless objections.

Separating host and port can be useful if you want to specify a different port for TLS, then you could also automatically redirect non secure connections over HTTPS.

from apm-server.

@jalvz let's keep it separate, unless there's a specific reason to include that in a refactor?

from apm-server.

i'm not touching that in the refactor either way (neither the port change, which i found it touches more things than initially thought, so scratch my last comment)

but with "keep separate" you mean "do separate"? right now they are not separated.

from apm-server.

"keep separate" as in let's not change the default port as part of the refactor PR

from apm-server.

Is the port for the nodejs client going to change too? I noticed it's currently setup to use 8081 which is the port Sickrage/Medusa/Sickgear, etc. use

from apm-server.

@OmgImAlexis I imagine you're talking about the following code:

This is only used for manual testing and is not run during normal operations. So I wouldn't personally worry too much about.

That being said, it's super easy in Node.js to simply find a random available port and listen on that. To modify the above code to do that, simply apply this patch:

-http.createServer(function (req, res) {
-  console.log(req.method, req.url)
-
-  res.writeHead(200, {'Content-Type': 'text/plain'})
-  res.end('Hello World\n')
-
-  apm.captureError(new Error('Ups, something broke'))
-
-}).listen(8081, function () {
-  console.log('server is listening on port 8081')
-})
+var server = http.createServer(function (req, res) {
+  console.log(req.method, req.url)
+
+  res.writeHead(200, {'Content-Type': 'text/plain'})
+  res.end('Hello World\n')
+
+  apm.captureError(new Error('Ups, something broke'))
+})
+
+server.listen(function () {
+  console.log('server is listening on port', server.address().port)
+})

This is the standard way in Node.js to make sure there's no port conflicts (though I'm not sure how easy it is to modify Docker to support this is).

from apm-server.

@watson I'd suggest using get-port with the host set to 0.0.0.0 for that.

from apm-server.

@OmgImAlexis I personally quite like when I do manual testing that it's always on the same port as do it some other frameworks. Agree that we should probably change the default port from 8081 to something else to prevent conflicts. But lets move this to an other issue as it it's related to manual testing and no the apm server intake port.

from apm-server.

Thanks for your input @OmgImAlexis and @crackerplace!
I'll close this issue now. Feel free to reopen if you feel there's more to discuss.

from apm-server.