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

[7.6][Heartbeat] Improved redirect support (#14125) #15939

Closed
wants to merge 1 commit into from

Conversation

andrewvc
Copy link
Contributor

Backport of #14125

This patch better handles redirects in heartbeat. It takes a different tack than what was described in #13146 . This PR is on the whole more pragmatic, accomplishing substantive change with undue complication of the schema and beats codebase.

Changes in this patch:

  • Added http.response.redirects field. This is an array of URLs redirected to inclusive of the final destination, exclusive of the original
  • Utilize the host codepath, rather than the IP codepath for execution of the check, previously only used for proxies in heartbeat, for redirects. This simplifies the code, hides timing data that can be screwy with redirects (things like fine grained DNS resolution). This means that the following fields no longer appear with redirects enabled in the event: http.rtt.content.us, http.rtt.response_header.us, http.rtt.total.us, http.rtt.validate.us, http.rtt.write_request.us.
  • Default max_redirects: 0 instead of 10

The new default does make this a breaking change, however, we either need to break that setting and preserve the timing information for the common use case of someone checking a non-redirecting URL, or keeping it at 10 but losing the timing info. The third option would be to invest additional eng. effort here to track those numbers across reqs, but the benefit seems, minimal given that they are not displayed in the UI, only in dashboards.

(cherry picked from commit 694dac8)

This patch better handles redirects in heartbeat. It takes a different tack than what was described in elastic#13146 . This PR is on the whole more pragmatic, accomplishing substantive change with undue complication of the schema and beats codebase.

Changes in this patch:

* Added http.response.redirects field. This is an array of URLs redirected to inclusive of the final destination, exclusive of the original
* Utilize the host codepath, rather than the IP codepath for execution of the check, previously only used for proxies in heartbeat, for redirects. This simplifies the code, hides timing data that can be screwy with redirects (things like fine grained DNS resolution). This means that the following fields no longer appear with redirects enabled in the event: http.rtt.content.us, http.rtt.response_header.us, http.rtt.total.us, http.rtt.validate.us, http.rtt.write_request.us.
* Default max_redirects: 0 instead of 10

The new default does make this a breaking change, however, we either need to break that setting and preserve the timing information for the common use case of someone checking a non-redirecting URL, or keeping it at 10 but losing the timing info. The third option would be to invest additional eng. effort here to track those numbers across reqs, but the benefit seems, minimal given that they are not displayed in the UI, only in dashboards.

(cherry picked from commit 694dac8)
@andrewvc andrewvc requested a review from a team as a code owner January 29, 2020 18:36
@andrewvc andrewvc self-assigned this Jan 29, 2020
@andrewvc andrewvc added Team:obs-ds-hosted-services Label for the Observability Hosted Services team bug enhancement Heartbeat v7.6.0 labels Jan 29, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime (:uptime)

@andrewvc
Copy link
Contributor Author

andrewvc commented Jan 29, 2020

Dupe of #14125

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug enhancement Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team v7.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants