stack: add AnnotateError middleware
#2158
Merged
Conversation
This file contains hidden or 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
This branch introduces a new `AnnotateError` middleware in `linkerd-stack`, for adding context to errors returned by a wrapped `Service`. The `annotate_error` module contains a new trait, `ErrorContext`, which is implemented by types that provide context to `Error`s by wrapping them with a new `Error` type, and the `AnnotateError` middleware itself. The middleware consists of a `NewService` which extracts a `Param` implementing `ErrorContext` and produces a `Service`, which wraps any errors returned by the inner `Service` using that `ErrorContext`. Additionally, `NewAnnotateError` may be constructed with an `ExtractParam` implementation that configures how an `ErrorContext` implementation is produced from a target type, when the `Param`s themselves do not implement `ErrorContext`. Finally, a pre-made implementation of `ErrorContext`, called `FromTarget`, that produces new errors when the error type implements `From<(&T, Error)>` is provided. In this case, the error type can implement `From` with the appropriate target type, and the `annotate_error::layer_from_target` constructor can be used, without requiring an `ExtractParam` implementation to be written for each stack.
olix0r
reviewed
Jan 13, 2023
olix0r
reviewed
Jan 13, 2023
This was referenced Jan 13, 2023
hawkw
added a commit
that referenced
this pull request
Jan 26, 2023
In order to improve the proxy's debuggability, it's desirable that the errors logged by the proxy and/or returned to HTTP clients include relevant information about where in the proxy that error occurred. Currently, buffer layers are constructed with a string "name" parameter that describes which part of the stack is behind that buffer, so that the fail-fast errors returned by those buffers can contain the stack layer's name. While this provides some useful context, it isn't a complete solution, as this context is only added to fail-fast errors, and not other errors such as I/O errors, Additionally, these names only indicate where in the stack the error occurred, and don't provide metadata describing *which instance* of that stack layer produced the error --- e.g., there is no context describing the client address, logical address, concrete address, or endpoint (as appropriate) that produced the error. Instead, we should annotate *all* errors with the following information: - The name of the stack layer where the error occurred - Target metadata including the logical, concrete, and endpoint addresses (as appropriate) This branch uses the `AnnotateError` middleware added in PR #2158 to add layers to the proxy which annotate errors with metadata extracted from targets. Furthermore, this branch removes the existing name parameter from buffer layers, and replaces them with the `AnnotateError` middleware, which is used to add target metadata *and* a string name describing the stack layer to all errors. Additionally, the various `HttpRescue` implementations have been changed slightly, so that when a specific cause is found for an error, the entire error chain is still formatted in the HTTP response `l5d-error` header, rather than _just_ the cause. This ensures that all context added to an error is included in the formatted message, rather than just the root cause (which lacks target metadata).
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Feb 16, 2023
This release includes many internal changes to prepare for the new client policy API. Stack metric label values have changed to reflect the new shape of the outbound proxy. --- * build(deps): bump try-lock from 0.2.3 to 0.2.4 (linkerd/linkerd2-proxy#2139) * build(deps): bump regex from 1.7.0 to 1.7.1 (linkerd/linkerd2-proxy#2145) * build(deps): bump tokio from 1.24.0 to 1.24.1 (linkerd/linkerd2-proxy#2144) * Parameterize the load balancer stack (linkerd/linkerd2-proxy#2142) * build(deps): bump prost-types from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2147) * build(deps): bump prost from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2148) * build(deps): bump prost-build from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2149) * stack: add `AnnotateError` middleware (linkerd/linkerd2-proxy#2158) * Fix proxy-core dependencies (linkerd/linkerd2-proxy#2163) * build(deps): bump tj-actions/changed-files from 35.3.1 to 35.4.1 (linkerd/linkerd2-proxy#2153) * orig_proto: don't set `connection: close` on errors (linkerd/linkerd2-proxy#2171) * build(deps): bump bumpalo from 3.11.1 to 3.12.0 (linkerd/linkerd2-proxy#2166) * build(deps): bump proc-macro2 from 1.0.49 to 1.0.50 (linkerd/linkerd2-proxy#2165) * build(deps): bump tj-actions/changed-files from 35.4.1 to 35.4.4 (linkerd/linkerd2-proxy#2172) * build(deps): bump windows_x86_64_msvc from 0.42.0 to 0.42.1 (linkerd/linkerd2-proxy#2164) * configure buffers from target `Param`s (linkerd/linkerd2-proxy#2173) * Simplify profile discovery (linkerd/linkerd2-proxy#2170) * stack: Unify AnnotateError and MapErr (linkerd/linkerd2-proxy#2180) * build(deps): bump windows_aarch64_msvc from 0.42.0 to 0.42.1 (linkerd/linkerd2-proxy#2176) * build(deps): bump async-trait from 0.1.61 to 0.1.63 (linkerd/linkerd2-proxy#2177) * build(deps): bump tokio from 1.24.1 to 1.24.2 (linkerd/linkerd2-proxy#2178) * Add the `meshtls-boring-fips` feature flag (linkerd/linkerd2-proxy#2168) * Update HTTP error responder to log version info (linkerd/linkerd2-proxy#2182) * build(deps): bump which from 4.3.0 to 4.4.0 (linkerd/linkerd2-proxy#2185) * build(deps): bump derive_arbitrary from 1.2.2 to 1.2.3 (linkerd/linkerd2-proxy#2184) * build(deps): bump unicode-bidi from 0.3.8 to 0.3.10 (linkerd/linkerd2-proxy#2183) * build(deps): bump linkerd/dev from 38 to 39 (linkerd/linkerd2-proxy#2175) * add target metadata to error contexts (linkerd/linkerd2-proxy#2162) * build(deps): bump rustls from 0.20.7 to 0.20.8 (linkerd/linkerd2-proxy#2187) * build(deps): bump ahash from 0.8.2 to 0.8.3 (linkerd/linkerd2-proxy#2188) * build(deps): bump matches from 0.1.9 to 0.1.10 (linkerd/linkerd2-proxy#2189) * Rename MakeThunk to NewThunk (linkerd/linkerd2-proxy#2197) * Add `NewQueueWithoutTimeout` (linkerd/linkerd2-proxy#2196) * Cache discovery results independently of proxy stacks (linkerd/linkerd2-proxy#2195) * core: Rename Stack utilities for clarity (linkerd/linkerd2-proxy#2199) * gateway: Unify discovery for HTTP & opaque stacks (linkerd/linkerd2-proxy#2198) * build(deps): bump libfuzzer-sys from 0.4.5 to 0.4.6 (linkerd/linkerd2-proxy#2193) * build(deps): bump arbitrary from 1.2.2 to 1.2.3 (linkerd/linkerd2-proxy#2191) * http: Remove `Clone` requirement in servers (linkerd/linkerd2-proxy#2200) * build(deps): bump either from 1.8.0 to 1.8.1 (linkerd/linkerd2-proxy#2192) * build(deps): bump bytes from 1.3.0 to 1.4.0 (linkerd/linkerd2-proxy#2202) * build(deps): bump cc from 1.0.78 to 1.0.79 (linkerd/linkerd2-proxy#2203) * build(deps): bump tj-actions/changed-files from 35.4.4 to 35.5.1 (linkerd/linkerd2-proxy#2211) * build(deps): bump tokio from 1.24.2 to 1.25.0 (linkerd/linkerd2-proxy#2206) * build(deps): bump miniz_oxide from 0.6.2 to 0.6.4 (linkerd/linkerd2-proxy#2207) * build(deps): bump async-trait from 0.1.63 to 0.1.64 (linkerd/linkerd2-proxy#2205) * Split outbound test modules into files (linkerd/linkerd2-proxy#2213) * Disable broken tests (linkerd/linkerd2-proxy#2214) * Add traceparent header parsing for w3c tracecontext (linkerd/linkerd2-proxy#2179) * Simplify the `Resolve` trait alias (linkerd/linkerd2-proxy#2218) * downgrade `miniz_oxide` from yanked 0.6.4 to 0.6.2 (linkerd/linkerd2-proxy#2219) * build(deps): bump heck from 0.4.0 to 0.4.1 (linkerd/linkerd2-proxy#2215) * ci: Fix check-each workflow (linkerd/linkerd2-proxy#2222) * Use a cascading stack with protocol detection (linkerd/linkerd2-proxy#2221) * ci: Fix quotation in list-crates (linkerd/linkerd2-proxy#2225) * outbound: Split out separate 'opaq' modules (linkerd/linkerd2-proxy#2224) * Update `linkerd2-proxy-api` to v0.8.0 (linkerd/linkerd2-proxy#2223) * outbound: Lint stack target types (linkerd/linkerd2-proxy#2226) * outbound: Split sidecar and ingress stack modules (linkerd/linkerd2-proxy#2227) * gateway: Split 'http' and 'opaq' modules (linkerd/linkerd2-proxy#2230) * test: Disable tap::rejects_incorrect_identity_when_identity_is_expected (linkerd/linkerd2-proxy#2231) * outbound: Improve discovery cache test (linkerd/linkerd2-proxy#2233) * integration: add destination update builders (linkerd/linkerd2-proxy#2232) * Rename linkerd-server-policy to linkerd-proxy-server-policy (linkerd/linkerd2-proxy#2235) * integration: add test for direct HTTP connections (linkerd/linkerd2-proxy#2234) * outbound: Refactor stack target types (linkerd/linkerd2-proxy#2210) * Add client-policy types (linkerd/linkerd2-proxy#2236) Signed-off-by: Oliver Gould <[email protected]>
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Feb 17, 2023
This release includes many internal changes to prepare for the new client policy API. Stack metric label values have changed to reflect the new shape of the outbound proxy. This change also includes some test improvements that helped debug an issue while merging this. --- * build(deps): bump try-lock from 0.2.3 to 0.2.4 (linkerd/linkerd2-proxy#2139) * build(deps): bump regex from 1.7.0 to 1.7.1 (linkerd/linkerd2-proxy#2145) * build(deps): bump tokio from 1.24.0 to 1.24.1 (linkerd/linkerd2-proxy#2144) * Parameterize the load balancer stack (linkerd/linkerd2-proxy#2142) * build(deps): bump prost-types from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2147) * build(deps): bump prost from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2148) * build(deps): bump prost-build from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2149) * stack: add `AnnotateError` middleware (linkerd/linkerd2-proxy#2158) * Fix proxy-core dependencies (linkerd/linkerd2-proxy#2163) * build(deps): bump tj-actions/changed-files from 35.3.1 to 35.4.1 (linkerd/linkerd2-proxy#2153) * orig_proto: don't set `connection: close` on errors (linkerd/linkerd2-proxy#2171) * build(deps): bump bumpalo from 3.11.1 to 3.12.0 (linkerd/linkerd2-proxy#2166) * build(deps): bump proc-macro2 from 1.0.49 to 1.0.50 (linkerd/linkerd2-proxy#2165) * build(deps): bump tj-actions/changed-files from 35.4.1 to 35.4.4 (linkerd/linkerd2-proxy#2172) * build(deps): bump windows_x86_64_msvc from 0.42.0 to 0.42.1 (linkerd/linkerd2-proxy#2164) * configure buffers from target `Param`s (linkerd/linkerd2-proxy#2173) * Simplify profile discovery (linkerd/linkerd2-proxy#2170) * stack: Unify AnnotateError and MapErr (linkerd/linkerd2-proxy#2180) * build(deps): bump windows_aarch64_msvc from 0.42.0 to 0.42.1 (linkerd/linkerd2-proxy#2176) * build(deps): bump async-trait from 0.1.61 to 0.1.63 (linkerd/linkerd2-proxy#2177) * build(deps): bump tokio from 1.24.1 to 1.24.2 (linkerd/linkerd2-proxy#2178) * Add the `meshtls-boring-fips` feature flag (linkerd/linkerd2-proxy#2168) * Update HTTP error responder to log version info (linkerd/linkerd2-proxy#2182) * build(deps): bump which from 4.3.0 to 4.4.0 (linkerd/linkerd2-proxy#2185) * build(deps): bump derive_arbitrary from 1.2.2 to 1.2.3 (linkerd/linkerd2-proxy#2184) * build(deps): bump unicode-bidi from 0.3.8 to 0.3.10 (linkerd/linkerd2-proxy#2183) * build(deps): bump linkerd/dev from 38 to 39 (linkerd/linkerd2-proxy#2175) * add target metadata to error contexts (linkerd/linkerd2-proxy#2162) * build(deps): bump rustls from 0.20.7 to 0.20.8 (linkerd/linkerd2-proxy#2187) * build(deps): bump ahash from 0.8.2 to 0.8.3 (linkerd/linkerd2-proxy#2188) * build(deps): bump matches from 0.1.9 to 0.1.10 (linkerd/linkerd2-proxy#2189) * Rename MakeThunk to NewThunk (linkerd/linkerd2-proxy#2197) * Add `NewQueueWithoutTimeout` (linkerd/linkerd2-proxy#2196) * Cache discovery results independently of proxy stacks (linkerd/linkerd2-proxy#2195) * core: Rename Stack utilities for clarity (linkerd/linkerd2-proxy#2199) * gateway: Unify discovery for HTTP & opaque stacks (linkerd/linkerd2-proxy#2198) * build(deps): bump libfuzzer-sys from 0.4.5 to 0.4.6 (linkerd/linkerd2-proxy#2193) * build(deps): bump arbitrary from 1.2.2 to 1.2.3 (linkerd/linkerd2-proxy#2191) * http: Remove `Clone` requirement in servers (linkerd/linkerd2-proxy#2200) * build(deps): bump either from 1.8.0 to 1.8.1 (linkerd/linkerd2-proxy#2192) * build(deps): bump bytes from 1.3.0 to 1.4.0 (linkerd/linkerd2-proxy#2202) * build(deps): bump cc from 1.0.78 to 1.0.79 (linkerd/linkerd2-proxy#2203) * build(deps): bump tj-actions/changed-files from 35.4.4 to 35.5.1 (linkerd/linkerd2-proxy#2211) * build(deps): bump tokio from 1.24.2 to 1.25.0 (linkerd/linkerd2-proxy#2206) * build(deps): bump miniz_oxide from 0.6.2 to 0.6.4 (linkerd/linkerd2-proxy#2207) * build(deps): bump async-trait from 0.1.63 to 0.1.64 (linkerd/linkerd2-proxy#2205) * Split outbound test modules into files (linkerd/linkerd2-proxy#2213) * Disable broken tests (linkerd/linkerd2-proxy#2214) * Add traceparent header parsing for w3c tracecontext (linkerd/linkerd2-proxy#2179) * Simplify the `Resolve` trait alias (linkerd/linkerd2-proxy#2218) * downgrade `miniz_oxide` from yanked 0.6.4 to 0.6.2 (linkerd/linkerd2-proxy#2219) * build(deps): bump heck from 0.4.0 to 0.4.1 (linkerd/linkerd2-proxy#2215) * ci: Fix check-each workflow (linkerd/linkerd2-proxy#2222) * Use a cascading stack with protocol detection (linkerd/linkerd2-proxy#2221) * ci: Fix quotation in list-crates (linkerd/linkerd2-proxy#2225) * outbound: Split out separate 'opaq' modules (linkerd/linkerd2-proxy#2224) * Update `linkerd2-proxy-api` to v0.8.0 (linkerd/linkerd2-proxy#2223) * outbound: Lint stack target types (linkerd/linkerd2-proxy#2226) * outbound: Split sidecar and ingress stack modules (linkerd/linkerd2-proxy#2227) * gateway: Split 'http' and 'opaq' modules (linkerd/linkerd2-proxy#2230) * test: Disable tap::rejects_incorrect_identity_when_identity_is_expected (linkerd/linkerd2-proxy#2231) * outbound: Improve discovery cache test (linkerd/linkerd2-proxy#2233) * integration: add destination update builders (linkerd/linkerd2-proxy#2232) * Rename linkerd-server-policy to linkerd-proxy-server-policy (linkerd/linkerd2-proxy#2235) * integration: add test for direct HTTP connections (linkerd/linkerd2-proxy#2234) * outbound: Refactor stack target types (linkerd/linkerd2-proxy#2210) * Add client-policy types (linkerd/linkerd2-proxy#2236)
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.
This branch introduces a new
AnnotateErrormiddleware inlinkerd-stack, for adding context to errors returned by a wrappedService.The
annotate_errormodule contains a new trait,ErrorContext, which is implemented by types that provide context toErrors by wrapping them with a newErrortype, and theAnnotateErrormiddleware itself. The middleware consists of aNewServicewhich extracts aParamimplementingErrorContextand produces aService, which wraps any errors returned by the innerServiceusing thatErrorContext.Additionally,
NewAnnotateErrormay be constructed with anExtractParamimplementation that configures how anErrorContextimplementation is produced from a target type, when theParams themselves do not implementErrorContext. Finally, a pre-made implementation ofErrorContext, calledFromTarget, that produces new errors when the error type implementsFrom<(&T, Error)>is provided. In this case, the error type can implementFromwith the appropriate target type, and theannotate_error::layer_from_targetconstructor can be used, without requiring anExtractParamimplementation to be written for each stack.This change was extracted from PR #2154.