jenkinsci / datadog-plugin

A Jenkins plugin used to forward metrics, events, and service checks to an account at Datadog, automatically.
https://plugins.jenkins.io/datadog/
MIT License
30 stars 48 forks source link

Move all HTTP calls to a dedicated class and use Jetty HTTP client instead of raw HttpUrlConnection #346

Closed nikita-tkachenko-datadog closed 1 year ago

nikita-tkachenko-datadog commented 1 year ago

Requirements for Contributing to this repository

What does this PR do?

This PR does a few improvements to the way the plugin does HTTP calls, making them more efficient and limiting the number of concurrent requests.

The reason to do this is that there is a customer with a Jenkins instance that runs multiple large pipelines: when executed with the plugin enabled, the process runs out of available file descriptors because the requests are slow (since each one opens a new TCP socket) and because there is no limit on the number of simultaneous requests and no backpressure if the network layer cannot keep up.

Description of the Change

Alternate Designs

Possible Drawbacks

Verification Process

Verified manually in Agentless/Evp-proxy/Agentful-APM modes. Separate set of verifications is done with Jenkins using an HTTP proxy (to ensure the plugin respect Jenkins' proxy settings).

Additional Notes

Release Notes

Review checklist (to be filled by reviewers)

nikita-tkachenko-datadog commented 1 year ago

I think this PR needs to be reviewed by the jenkins-plugin team as well, as AFAIK we wanted to use vanilla Java HTTP classes to avoid affecting Jenkins core with external dependencies.

Jenkins plugins are loaded using isolated classloaders, so including a third-party dependency in a plugin has no effect on Jenkins core. Plugin classloaders are children of the classloader that is used to load the core, so the worst that can happen in theory is a problem in the plugin in case it tries to load a class that is already available in the core loader (because children CLs delegate to parent CL). Anyway, this is not the case here, since neither Jenkins core nor the plugins that we depend on use Jetty HTTP.