-
Notifications
You must be signed in to change notification settings - Fork 271
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
test: make integration tests shut up #771
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PR #766 changed the proxy tests to set a much more verbose default log filter, relying on libtest's output capturing to prevent the tests from spamming stdout when they don't fail. However, the integration tests run the proxy in a separate thread, and libtest can't currently capture output from spawned threads. This results in a bunch of unnecessary noise when running the integration tests. This branch changes the integration tests so that the spawned proxy thread uses the old filter settings instead. We still get more detailed output from stack tests, and from the main test threads in the integration tests. I removed some code that would set the value of the log level env vars, since that made this impossible — I'm not really sure why that code was ever necessary in the first place. Also, I fixed a compiler error in `linkerd2-app-test` due to a missing Cargo feature. Signed-off-by: Eliza Weisman <eliza@buoyant.io>
olix0r
approved these changes
Dec 15, 2020
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Dec 15, 2020
This release features a change to the proxy's cache eviction strategy to ensure that clients (and their load balancers) are reused by new outbound connections. This can dramatically reduce memory consumption, especially for busy HTTP/1.1 clients. Also, the proxy's HTTP detection scheme has been made more robust. Previously, the proxy would perform a only single read to determine whether a TCP stream was HTTP, which could lead to false positives. Now, the proxy reads until at least the first newline, which is what the HTTP parser actually needs to make a proper determination. With this, the default dispatch timeouts have been increased to 5s to accomodate connection pools that may not issue an immediate request. Furthermore, this release includes an upgrade to Tokio v0.3 and its associated ecosystem. --- * update buffers to use Tokio 0.3 MPSC channels (linkerd/linkerd2-proxy#759) * Update the proxy to use Tokio 0.3 (linkerd/linkerd2-proxy#732) * Rename DetectHttp to NewServeHttp (linkerd/linkerd2-proxy#760) * http: more consistent names for body types (linkerd/linkerd2-proxy#761) * io: simplify the `Io` trait (linkerd/linkerd2-proxy#762) * trace: nicer traces in tests, clean up trace configuration (linkerd/linkerd2-proxy#766) * Ensure that services are held as long they are being used (linkerd/linkerd2-proxy#767) * outbound: add stack tests for http (linkerd/linkerd2-proxy#765) * cache: Ensure that actively held services are not evicted (linkerd/linkerd2-proxy#768) * cache: Only spawn a single task per cache entry (linkerd/linkerd2-proxy#770) * test: make integration tests shut up (linkerd/linkerd2-proxy#771) * metrics: Add support for microsecond counters (linkerd/linkerd2-proxy#772) * Add a protocol label to stack metrics (linkerd/linkerd2-proxy#773) * detect: Make protocol detection more robust (linkerd/linkerd2-proxy#744)
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Dec 15, 2020
This release features a change to the proxy's cache eviction strategy to ensure that clients (and their load balancers) are reused by new outbound connections. This can dramatically reduce memory consumption, especially for busy HTTP/1.1 clients. Also, the proxy's HTTP detection scheme has been made more robust. Previously, the proxy would perform a only single read to determine whether a TCP stream was HTTP, which could lead to false positives. Now, the proxy reads until at least the first newline, which is what the HTTP parser actually needs to make a proper determination. With this, the default dispatch timeouts have been increased to 5s to accomodate connection pools that may not issue an immediate request. Furthermore, this release includes an upgrade to Tokio v0.3 and its associated ecosystem. --- * update buffers to use Tokio 0.3 MPSC channels (linkerd/linkerd2-proxy#759) * Update the proxy to use Tokio 0.3 (linkerd/linkerd2-proxy#732) * Rename DetectHttp to NewServeHttp (linkerd/linkerd2-proxy#760) * http: more consistent names for body types (linkerd/linkerd2-proxy#761) * io: simplify the `Io` trait (linkerd/linkerd2-proxy#762) * trace: nicer traces in tests, clean up trace configuration (linkerd/linkerd2-proxy#766) * Ensure that services are held as long they are being used (linkerd/linkerd2-proxy#767) * outbound: add stack tests for http (linkerd/linkerd2-proxy#765) * cache: Ensure that actively held services are not evicted (linkerd/linkerd2-proxy#768) * cache: Only spawn a single task per cache entry (linkerd/linkerd2-proxy#770) * test: make integration tests shut up (linkerd/linkerd2-proxy#771) * metrics: Add support for microsecond counters (linkerd/linkerd2-proxy#772) * Add a protocol label to stack metrics (linkerd/linkerd2-proxy#773) * detect: Make protocol detection more robust (linkerd/linkerd2-proxy#744)
olix0r
pushed a commit
that referenced
this pull request
Jan 14, 2021
In stack tests, we set a fairly verbose `tracing` filter, since the logs are captured by libtest and only printed if a test fails or when the `--show-output` flag is passed. However, in integration tests, we disable most tracing, because earlier versions of Rust did not support output capturing for spawned threads, and the integration tests run the proxy in a separate thread (see #771). Now that we're on Rust 1.49, however, libtest now supports output capturing for spawned threads, so this PR changes the integration tests to enable more verbose logging. This should help with debugging test failures. I made a few other minor improvements to tracing in tests. In particular: * Share the *same* tracing subscriber between the test thread and the test proxy. This should resolve the issue where logs from the proxy in an integration test and logs from the test code have timestamps that are relative to different start times, making the logs appear to time travel. * Replace some `println!`s with tracing events. Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR #766 changed the proxy tests to set a much more verbose default log
filter, relying on libtest's output capturing to prevent the tests from
spamming stdout when they don't fail. However, the integration tests run
the proxy in a separate thread, and libtest can't currently capture
output from spawned threads. This results in a bunch of unnecessary
noise when running the integration tests.
When Rust 1.49.0 is released, we should revisit this, as output capturing
will work for spawned threads.
This branch changes the integration tests so that the spawned proxy
thread uses the old filter settings instead. We still get more detailed
output from stack tests, and from the main test threads in the
integration tests. I removed some code that would set the value of the
log level env vars, since that made this impossible — I'm not really
sure why that code was ever necessary in the first place.
Also, I fixed a compiler error in
linkerd2-app-test
due to a missingCargo feature.
Signed-off-by: Eliza Weisman eliza@buoyant.io