Emit metrics for new headers handling behaviour. Hide actual changes under feature flag. #2267
Conversation
…der feature flag.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #2267 +/- ##
==========================================
- Coverage 85.20% 85.17% -0.03%
==========================================
Files 270 272 +2
Lines 15555 15687 +132
==========================================
+ Hits 13253 13361 +108
- Misses 1877 1890 +13
- Partials 425 436 +11 ☔ View full report in Codecov by Sentry. |
| "go.uber.org/net/metrics" | ||
| ) | ||
|
|
||
| var ( |
There was a problem hiding this comment.
I don't like the idea of using global variables here. Let's just create a simple struct type ReserveHeaderMetrics struct and plumb it into dispatcher so all 3 transports get the same copy. This also will get rid of sync.Once usage.
|
|
||
| func incHeaderMetric(vector *metrics.CounterVector, source, dest string) { | ||
| if vector != nil { | ||
| if counter, err := vector.Get("source", source, "dest", dest); counter != nil && err == nil { |
There was a problem hiding this comment.
under what case the error is not nil, and counter could be empty?
| @@ -153,10 +168,17 @@ func metadataToTransportRequest(md metadata.MD) (*transport.Request, error) { | |||
| request.Encoding = transport.Encoding(getContentSubtype(value)) | |||
| } | |||
| default: | |||
| if isReserved(header) || isReservedWithDollarSign(header) { | |||
There was a problem hiding this comment.
can you add a comment on why do we need check both condition?. Same for the if enforceHeaderRules condition
Currently underlying implementation of headers handling wary significantly from one transport to another (docs/docs/headers-handling.md in this PR). Eventually, in one of the following releases, we want to make behaviour consistent and protective: filter inbound 'rpc-' headers, return error for attempting of 'rpc-' header setting.
Proposed changes are backward incompatible, so to identify the edges that are affected by the future changes, at first stage let's emit metrics.
Metrics:
reserved_headers_strippedandreserved_headers_errormetrics with"component": "yarpc-header-migration"constant tag and with source and dest variable tagsPart of #2265