Commit 4e3163d
Classify undeclared exceptions for ServiceRouter
Summary:
`glean.glass`'s ServiceRouter server-fatal SLI is ~99.9% undeclared
`ApplicationException`, and ~94% of those are downstream `glean.query.prod`
failures leaking out of the Glass handler untyped. Two consequences:
1. The SLI measures Glean's throttling, not Glass's availability. The
`glean.glass fatals` detector fires on "a Glass client exceeded its Glean
per-client QPS share" — something Glass oncall cannot act on.
2. **ServiceRouter marks down healthy Glass hosts.** Undeclared exceptions get
`kUndeclaredAppErrorConfig`, which is `logFatal = true` *and*
`markdown = true`, so a Glean-side throttle shrinks the routable Glass pool
exactly when load is highest.
For a Thrift error, the following headers are appended to the response:
* 'uex': a short string for the exception type, e.g. ApplicationException
* 'ex': the error code for that exception type
* 'uexw': the complete error message.
hsthrift cannot distinguish undeclared exceptions on its own. The generated `respWriter'`
replaces the escaping exception with a fresh `Thrift.ApplicationException`
before `handlerWrapper` takes `show (typeOf ex)`, so `uex` is uniformly
`ApplicationException` and all triage has to string-match `uexw`.
This diff adds function `Glean.Glass.ErrorClassification.srErrorName`, which maps an escaping
exception to the `uex` name ServiceRouter should classify it under. D116930502 in configerator then defines the behaviour for each new `uex` value. The mapping of escapting exceptions is as follows:
| escaping exception | new `uex` name |
|---|---|
| downstream `THROTTLING_CLIENT_ID_REQUEST` | `GleanClientThrottled` |
| downstream `THROTTLING_ERROR` | `GleanServiceThrottled` |
| downstream `APP_QUEUE_TIMEOUT` | `GleanQueueTimeout` |
| downstream `HOST_OVERLOAD` / `APP_OVERLOAD` | `GleanOverload` |
| GHC allocation limit | `GlassAllocationLimit` |
| Glass's own 30s `withTimeout` | `GlassTimeout` |
`assignHeaders` emits the new `uex` header.
The downstream reason is recovered by matching the `(REASON)` token that
`TServiceRouterException::getExceptionMsg` puts at the front of its message.
String matching is unavoidable today — `HsChannel.h` stringifies the whole
`folly::exception_wrapper` into a `ChannelException Text` and drops the
response headers — but only the enum name is matched, never the prose after
it, and an unrecognised reason returns `Nothing`.
This diff is inert on its own: ServiceRouter falls back to
`kUndeclaredAppErrorConfig` for any name with no `appErrorsMap` entry, so
behaviour is unchanged until the companion `configerator` change adds the
per-name policy. `ex` and `uexw` are deliberately left alone, so the wire-level
record still says "undeclared application exception" and no client sees a
different response.
Reviewed By: bochko
Differential Revision: D116930531
fbshipit-source-id: 9f92d799e9d135d8f73379735244c11ca5e154211 parent 2359a1a commit 4e3163d
4 files changed
Lines changed: 232 additions & 5 deletions
File tree
- glean/glass
- Glean/Glass
- test/Glean/Glass/Test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
117 | 117 | | |
118 | 118 | | |
119 | 119 | | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
120 | 155 | | |
121 | 156 | | |
122 | 157 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
41 | 41 | | |
42 | 42 | | |
43 | 43 | | |
| 44 | + | |
44 | 45 | | |
45 | 46 | | |
46 | 47 | | |
| |||
56 | 57 | | |
57 | 58 | | |
58 | 59 | | |
| 60 | + | |
59 | 61 | | |
60 | 62 | | |
61 | 63 | | |
| |||
84 | 86 | | |
85 | 87 | | |
86 | 88 | | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
87 | 95 | | |
88 | 96 | | |
89 | 97 | | |
| |||
164 | 172 | | |
165 | 173 | | |
166 | 174 | | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
167 | 178 | | |
168 | | - | |
169 | | - | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
170 | 183 | | |
171 | | - | |
| 184 | + | |
172 | 185 | | |
173 | 186 | | |
174 | 187 | | |
175 | | - | |
| 188 | + | |
176 | 189 | | |
177 | 190 | | |
178 | 191 | | |
179 | | - | |
180 | 192 | | |
181 | 193 | | |
182 | 194 | | |
| |||
Lines changed: 110 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
0 commit comments