Add server configurations back in

[estolfo]

I unintentionally removed the server configurations in a previous PR and I'm opening this PR so we can discuss whether we want these settings to be configurable.

Questions:

• ReadTimeout and WriteTimeout are set to their zero value (0) if not set otherwise, which means there will be no timeout. Do we want users to be able to set these timeouts?
• Do we want users to be able to set a different ReadTimeout and WriteTimeout? Currently, they are set to the same user-configured value.
• DefaultMaxHeaderBytes is what is used for MaxHeaderBytes if it's not set otherwise. Its value is 1 << 20, so I don't see the point of hardcoding it ourselves to that value.

I personally think we should provide as few configuration options as possible to the user and only options in later if we need them.

[ghost]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2021-11-19T09:06:14.079+0000

• Duration: 6 min 27 sec

• Commit: 68a64e4

Test stats 🧪

Test	Results
Failed	0
Passed	54
Skipped	0
Total	54

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[astorm]

In general I'm right there with you with keeping configuration to a minimum, but here's my general thinking on why we might want the timeouts configurable

1. I'm uneasy with the default timeout of 0 -- I'm concerned about the the server staying open indefinitely and keeping the execution environment unfrozen/live, running up bill time

2. If a timeout is necessary, allowing users to influence this value also seems necessary. My thinking is if a user needs to extend this timeout for some unseen reason it doesn't seem reasonable to expect them to wait for us to do a release cycle.

3. This isn't the first time we've discussed this variable, so per Rename or Reconsider ELASTIC_APM_EXTENSION_TIMEOUT_SECONDS env Variable #61 we might also consider using ELASTIC_APM_API_REQUEST_TIME "plus a little more" as a value. This reduces the number of variables a user needs to configure, but also gives them indirect control over the timeout.

@estolfo does that influence/change your thinking at all?

[estolfo]

Those are good points, I agree we should keep the config in for the timeouts. We can discuss/change the name via the separate issue, which you've referenced.
What about MaxHeaderBytes? Should we remove that explicit configuration and rely on the default value, which is what we are using now anyway?

[astorm]

What about MaxHeaderBytes? Should we remove that explicit configuration and rely on the default value, which is what we are using now anyway?

Hmm, thinking that one through, making MaxHeaderBytes user-configurable seems like too much configuration complication at this point.

Re: whether we need an explicit value or not — that's a "six of one, half dozen the other" decision for me, but talking though those trade-offs

• Having an explicit value of 1MB (all hail bitwise arithmetic: 1 << 20) means our code is slightly more complicated

• Not having an explicit value means we're relying on a default, which might be different in older versions of Go or change in a future version of go

So in my mind having a default means we're more in control of how our program will behave when/if folks compile it themselves, so I'd be in favor of keeping it.

[estolfo]

ok @astorm that works for me 👍🏼

[astorm]

👍