Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce httpcommon package in libbeat (add support for Proxy) #25219

Merged
merged 46 commits into from
Jun 29, 2021

Conversation

urso
Copy link

@urso urso commented Apr 21, 2021

  • Enhancement

What does this PR do?

We introduces a libbeat/common/transport/httpcommon package. The package provides structures that can be used to extract common http settings from a configuration object and construct a http.RoundTripper or a http.Client. Currently the types configure:

  • Proxy settings
  • TLS
  • Timeout

The HTTPTransportSettings and HTTPClientProxySettings implement Unpack, which will be used automatically when a configuration is unpacked into a configuration structure. The Unpacker implemetnations directly initialize TLS and parse URLs and other settings that can fail. If some setting is missconfigured, Beats will fail early, reporting the full path of the setting (input, output, ...) that was missconfigured. Initializing the proxy URL and TLS, also reduces the responsiblities for the inputs/outputs to provide copy and paste code to do so.

Example usage:

type mySettings struct {
  ...

  Transport httpcommon.HTTPTransportSettings `config:",inline"`
}

...

settings := defaultSettings()
if err := config.Unpack(&settings); err != nil { return .... }

client := settings.Transport.Client(
  httpcommon.WithAPMHTTPInstrumentation(),
)

...

resp, err := client.Get(...)

Features already provided by httpcommon:

  • common dialer configuration
  • common TLS configuration (already pre initialized)
  • out of the box proxy support for everything HTTP
  • (optionally) instrument your HTTP client with APM
  • Timeout settign
  • Copy some Transport defaults from http.DefaultTransport. This initializes the idle connection limit and idle connection timeout (don't leak connections).

By default (can be disabled via functional options), proxy support defaults to the HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables. The environment variables are automatically ignored if the URLs host is localhost or the loopback device (by IP).

Many features that use an http.Client have been update to use the httpcommon package in order to extract "common" setting (this list only documents changes on existing features):

  • Heartbeat http monitor:
    • Added setting: proxy_headers
    • Added APM instrumentation
  • Elasticsearch output:
    • Added settings: proxy_headers, proxy_disable
  • Internal Elasticsearch monitoring reporter (legacy x-pack monitoring):
    • Added settings: proxy_disable, proxy_headers
  • Kibana client (setup.kibana.* namespace):
    • Added proxy support
    • automatically close idle connection
  • Filebeat httpjson input:
    • Add proxy settings
    • Automatically instrument the http client with APM HTTP support
  • Filebeat/Metricbeat cloudfoundry:
    • Add proxy support
    • APM HTTP instrumentalization
    • automatically close idle connections
  • Metricbeat http helper:
    • Added proxy support
    • automatically Close idle connections (default was no limits on number of idle connection and no timeout)
    • Added APM HTTP auto instrumentation
    • Affected modules (Note: I didn't check lightweight modules):
      • Consul
      • HTTP
      • Beat
      • Elasticsearch
      • Prometheus
      • NATS
      • Ceph
      • Traefik
      • Logstash
      • Rabbit MQ
      • Jolokia
      • Envoyproxy
      • PHP FPM
      • nginx
      • dropwizard
      • apache
      • couchbase
      • kubernetes
      • Kibana
      • etcd
      • couchdb
      • golang
      • haproxy
  • Agent:
    • Updated: Downloader supports
      • Adds proxy support
    • Updated: remote.Config, which is also used fleet server config namespace
      • Adds proxy support

Missing (left over for follow up PRs):

  • More settings:
    • make connection pool settings configurable?
    • Headers
    • Authentication:
      • Basic Auth
      • OAuth
      • Bearer token (must be configured or read from store... not from disk)
    • More fine grained timeout settings
    • read/write buffer sizes
  • Testing: the package is more or less indirectly tested. E.g. proxy support via the Elasticsearch output. New tests for Unpack behavior and proxy tests from the output should be moved to the httpcommon package
  • Add proxy settings to enroll command (enroll reads some settings from the config file and applies those, but CLI flags are missing)
  • Documentation (many inputs/modules have been touched)

Why is it important?

The change improves consistency between different features that use HTTP. Plus the common copy and past boilerplate is removed and concentrated in a single place.

All Features that use HTTP will gain proxy support.

More APM HTTP instrumentation for HTTP based features.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
    - [ ] I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
    - [ ] I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

Setup an HTTP proxy and try to connect via said proxy:

  • Elasticsearch output
  • Kibana dashboard setup
  • Elastic Agent artifact download
  • Metricbeat modules (chose one... e.g. Elasticsearch module)

Related issues

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 21, 2021
@urso urso changed the title Introducde httpcommon package in libbeat Introduce httpcommon package in libbeat Apr 22, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Apr 22, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: urso commented: /test

  • Start Time: 2021-06-29T18:48:22.377+0000

  • Duration: 86 min 25 sec

  • Commit: 7e7bb06

Test stats 🧪

Test Results
Failed 0
Passed 29616
Skipped 3876
Total 33492

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 29616
Skipped 3876
Total 33492

@urso urso added the Team:Elastic-Agent Label for the Agent team label Apr 22, 2021
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Apr 22, 2021
@ph
Copy link
Contributor

ph commented Apr 22, 2021

This is a super great start! cc @mostlyjason @alvarolobato

@urso
Copy link
Author

urso commented Apr 22, 2021

Argh... I forgot to adapt legacy Central Managent support and broke the build :(

@urso urso added the backport-v7.14.0 Automated backport with mergify label Apr 22, 2021
@michalpristas
Copy link
Contributor

http2 change with fallback looks good

x-pack/elastic-agent/pkg/agent/cmd/container.go Outdated Show resolved Hide resolved
x-pack/elastic-agent/pkg/remote/client.go Outdated Show resolved Hide resolved
x-pack/elastic-agent/pkg/remote/client.go Outdated Show resolved Hide resolved
@urso urso requested a review from blakerouse June 29, 2021 14:12
@urso
Copy link
Author

urso commented Jun 29, 2021

/test

@@ -142,7 +139,7 @@ func NewWithConfig(log *logger.Logger, cfg Config, wrapper wrapperFunc) (*Client

httpClient := http.Client{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be http2.Client? Or will the httpcommon.WithForceAttepmtHTTP2(true) switch it to http2 transparently?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the net/http package can upgrade to http2. HTTP 1 or 2 is handled by the transport.

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good.

@urso urso merged commit 4accfa8 into elastic:master Jun 29, 2021
@urso urso deleted the http_common branch June 29, 2021 20:15
mergify bot pushed a commit that referenced this pull request Jun 29, 2021
(cherry picked from commit 4accfa8)

# Conflicts:
#	libbeat/monitoring/report/elasticsearch/config.go
#	metricbeat/module/elasticsearch/index/data_test.go
mdelapenya added a commit to mdelapenya/beats that referenced this pull request Jun 30, 2021
* master: (25 commits)
  fix: Force PLATFORMS environment variable when we build Elastic Agent dependencies on arm64 (elastic#26415)
  macos for metricbeat to run in the extended meta-stage (elastic#26573)
  Packaging: add arm7 platform in the main pipeline (elastic#26575)
  [Heartbeat] Skip flakey timer queue test (elastic#26592)
  Update to "read_pipeline" permission (elastic#26465) (elastic#26580)
  API keys do not reflect the need for read_pipeline (elastic#26466) (elastic#26582)
  Add Fleet agent.id to Agent monitoring data (elastic#26548)
  Add kinesis metricset (elastic#25989)
  Refactor of system/memory metricset (elastic#26334)
  Introduce httpcommon package in libbeat (add support for Proxy) (elastic#25219)
  [Filebeat] change multiline configuration in awss3 input to parsers (elastic#25873)
  docs: Hint for the error "Error extracting container id" (elastic#25824)
  [Docs] Fixed metricbeat redis exported field CPU descriptions (elastic#25846) (elastic#26496)
  Update urllib to 1.26.5. (elastic#26380)
  Update golang.org/x/crypto (elastic#26448)
  [Filebeat] Update Fortinet Ingest Pipeline (elastic#24816)
  Move parsers outside of filestream input so others can use them as well (elastic#26541)
  [Filebeat] Fix `threatintel.indicator.url.full` field not populating (elastic#26508)
  [Filebeat] Add network direction processor to Zeek and Suricata modules (elastic#24620)
  Logging code cleanup related to Nomad auto-discovery (elastic#26498)
  ...
blakerouse added a commit that referenced this pull request Jun 30, 2021
…upport for Proxy) (#26587)

* Introduce httpcommon package in libbeat (add support for Proxy) (#25219)

(cherry picked from commit 4accfa8)

# Conflicts:
#	libbeat/monitoring/report/elasticsearch/config.go
#	metricbeat/module/elasticsearch/index/data_test.go

* Fix issues with backport.

* Fix changelog.

* FIx integration tests for ml-importer.

Co-authored-by: Steffen Siering <steffen.siering@elastic.co>
Co-authored-by: Blake Rouse <blake.rouse@elastic.co>
mergify bot added a commit that referenced this pull request Jun 30, 2021
…upport for Proxy) (#26587)

* Introduce httpcommon package in libbeat (add support for Proxy) (#25219)

(cherry picked from commit 4accfa8)

# Conflicts:
#	libbeat/monitoring/report/elasticsearch/config.go
#	metricbeat/module/elasticsearch/index/data_test.go

* Fix issues with backport.

* Fix changelog.

* FIx integration tests for ml-importer.

Co-authored-by: Steffen Siering <steffen.siering@elastic.co>
Co-authored-by: Blake Rouse <blake.rouse@elastic.co>
(cherry picked from commit 37e37f1)
blakerouse added a commit to blakerouse/beats that referenced this pull request Jun 30, 2021
… (add support for Proxy) (elastic#26587)

* Introduce httpcommon package in libbeat (add support for Proxy) (elastic#25219)

(cherry picked from commit 4accfa8)

# Conflicts:
#	libbeat/monitoring/report/elasticsearch/config.go
#	metricbeat/module/elasticsearch/index/data_test.go

* Fix issues with backport.

* Fix changelog.

* FIx integration tests for ml-importer.

Co-authored-by: Steffen Siering <steffen.siering@elastic.co>
Co-authored-by: Blake Rouse <blake.rouse@elastic.co>
blakerouse added a commit that referenced this pull request Jun 30, 2021
…upport for Proxy) (#26587) (#26626)

* Introduce httpcommon package in libbeat (add support for Proxy) (#25219)

(cherry picked from commit 4accfa8)

# Conflicts:
#	libbeat/monitoring/report/elasticsearch/config.go
#	metricbeat/module/elasticsearch/index/data_test.go

* Fix issues with backport.

* Fix changelog.

* FIx integration tests for ml-importer.

Co-authored-by: Steffen Siering <steffen.siering@elastic.co>
Co-authored-by: Blake Rouse <blake.rouse@elastic.co>

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Steffen Siering <steffen.siering@elastic.co>
simitt added a commit to simitt/apm-server that referenced this pull request Jun 30, 2021
simitt added a commit to simitt/apm-server that referenced this pull request Jun 30, 2021
simitt added a commit to simitt/apm-server that referenced this pull request Jun 30, 2021
axw added a commit to elastic/apm-server that referenced this pull request Jul 1, 2021
* Revert "[7.x] Update to elastic/beats@2871d29be93a (backport #5454) (#5589)"

This reverts commit 4269aee.

* Update go version to 1.16.5

* Update to elastic/beats@9361be46ebf2

* update tests to new ecs version

* fix broken test due to elastic/beats#25219

* update python tests to new ecs version

* make update

Co-authored-by: Andrew Wilkins <axw@elastic.co>
mergify bot pushed a commit that referenced this pull request Jul 19, 2021
(cherry picked from commit 4accfa8)

# Conflicts:
#	libbeat/monitoring/report/elasticsearch/config.go
#	metricbeat/module/elasticsearch/index/data_test.go
#	x-pack/elastic-agent/pkg/agent/cmd/enroll_cmd.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.14.0 Automated backport with mergify needs_docs Team:Elastic-Agent Label for the Agent team v7.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for proxy_url to the dashboard setup task
8 participants