add target metadata to error contexts by hawkw · Pull Request #2162 · linkerd/linkerd2-proxy

hawkw · 2023-01-17T19:17:32Z

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).

This changes the `HttpRescue` impls to use the entire error value as the message, rather than just the cause value. This way, we will include all context generated by `AnnotateError` layers in the stack.

* simplify endpoint errors Signed-off-by: Oliver Gould <ver@buoyant.io> * Consolidate AnnotateError and MapErr `AnnotateError` is really, really similar to `MapErr`. This change merges the implementations and cleans up some of the caller type boilerplate. * Update linkerd/stack/src/map_err.rs Co-authored-by: Eliza Weisman <eliza@buoyant.io> * comment * debug * Fix NewMapErr::layer_from_target constructor Signed-off-by: Oliver Gould <ver@buoyant.io> Co-authored-by: Eliza Weisman <eliza@buoyant.io>

* don't reuse error types across stacks * make http::Version impl Debug only * error messages, boilerplate * build(deps): bump windows_aarch64_msvc from 0.42.0 to 0.42.1 (#2176) Bumps windows_aarch64_msvc from 0.42.0 to 0.42.1. --- updated-dependencies: - dependency-name: windows_aarch64_msvc dependency-type: indirect update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump async-trait from 0.1.61 to 0.1.63 (#2177) Bumps [async-trait](https://github.com/dtolnay/async-trait) from 0.1.61 to 0.1.63. - [Release notes](https://github.com/dtolnay/async-trait/releases) - [Commits](dtolnay/async-trait@0.1.61...0.1.63) --- updated-dependencies: - dependency-name: async-trait dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump tokio from 1.24.1 to 1.24.2 (#2178) Bumps [tokio](https://github.com/tokio-rs/tokio) from 1.24.1 to 1.24.2. - [Release notes](https://github.com/tokio-rs/tokio/releases) - [Commits](https://github.com/tokio-rs/tokio/commits) --- updated-dependencies: - dependency-name: tokio dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add the `meshtls-boring-fips` feature flag (#2168) The `boring` crate exposes a `fips` feature flag, but this feature was not exposed by the `linkerd-meshtls-boring`. This change makes it theoretically possible to build the proxy in this mode. This feature is NOT tested in CI due to the fussiness of the required build environment and longer-term maintenance concerns. Signed-off-by: Zahari Dichev <zaharidichev@gmail.com> * Update HTTP error responder to log version info (#2182) The HTTP error responder already knows whether a request is considered HTTP/1, HTTP/2, or gRPC. This change updates these log messages to include the relevant protocol information for each failed request. Furthermore, this change updates the `http::Version` enum to implement `Debug` with full HTTP version strings like "HTTP/1" and "HTTP/2". We then remove the version for debug spans, as it's needlessly verbose for most uses. When debugging, we can identify the version based on specific events in a span. It need not exist on every event on the span. Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Zahari Dichev <zaharidichev@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Zahari Dichev <zaharidichev@gmail.com>

olix0r · 2023-01-25T00:34:16Z

@hawkw i'm ambivalent about how to resolve the test issue. It seems okay to keep the string matching. But it's also not clear if we actually care that the failfast error message is present, do we? We just care that the header isn't empty. IDK...

hawkw · 2023-01-25T00:37:59Z

yeah...if we care about the presence of the failfast message, we could change the tests to assert that the string contains it, rather than making an exact assertion, so they the tests don't care if the error contexts change a little? but i don't know if the presence of the failfast message is super important, either? i guess it's nice to be checking that the header value isn't a totally unrelated error, maybe?

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 <ver@buoyant.io>

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)

hawkw added 10 commits January 13, 2023 15:38

change error responders to use the whole error

a7f3f79

This changes the `HttpRescue` impls to use the entire error value as the message, rather than just the cause value. This way, we will include all context generated by `AnnotateError` layers in the stack.

outbound: annotate errors with concrete/logical addrs

92a3b5f

outbound: annotate errors with endpoint metadata

d05b24a

add route labels to errors

e110485

annotate inbound errors

63bac6e

remove buffer names

e6af8d8

add error annotation to forward stack

8c8c7a2

annotate controller client errors

ec8902d

nicer formatting for inbound logical errors

24bb1d8

update error messages in tests

90f80e6

hawkw requested a review from a team as a code owner January 17, 2023 19:17

hawkw force-pushed the eliza/error-context-2 branch from 421cc3e to bff17b7 Compare January 17, 2023 20:24

whoops

f43ed87

hawkw force-pushed the eliza/error-context-2 branch from bff17b7 to f43ed87 Compare January 17, 2023 20:28

hawkw mentioned this pull request Jan 20, 2023

configure buffers from target Params #2173

Merged

olix0r and others added 5 commits January 21, 2023 06:47

Merge branch 'main' into eliza/error-context-2

cd1a86e

Merge branch 'main' into eliza/error-context-2

af55f74

Merge branch 'main' into eliza/error-context-2

75849b4

make error matching less exact

a055299

olix0r approved these changes Jan 25, 2023

View reviewed changes

hawkw merged commit 55ba563 into main Jan 26, 2023

hawkw deleted the eliza/error-context-2 branch January 26, 2023 00:20

olix0r mentioned this pull request Feb 16, 2023

proxy: v2.190.1 linkerd/linkerd2#10342

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add target metadata to error contexts#2162

add target metadata to error contexts#2162
hawkw merged 17 commits intomainfrom
eliza/error-context-2

hawkw commented Jan 17, 2023

Uh oh!

olix0r commented Jan 25, 2023

Uh oh!

hawkw commented Jan 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

hawkw commented Jan 17, 2023

Uh oh!

olix0r commented Jan 25, 2023

Uh oh!

hawkw commented Jan 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants