Skip to content
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.

Sign up for GitHub

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

Workaround a bug in envoy #500

Merged
merged 7 commits into from
Apr 4, 2022
Merged

Workaround a bug in envoy #500

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
eb1b4ea
bug fix for reading thrift headers
marcoferrer Mar 30, 2022
b043097
Update headers.go
marcoferrer Mar 30, 2022
70458d0
bug fix for reading thrift headers
marcoferrer Mar 30, 2022
4ae05cf
Update thriftbp/headers.go
marcoferrer Mar 30, 2022
7de2d46
Update thriftbp/headers.go
marcoferrer Mar 30, 2022
6fb4d1f
Update thriftbp/headers.go
marcoferrer Mar 31, 2022
641ba02
Update headers.go
marcoferrer Mar 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions thriftbp/headers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package thriftbp

import (
"context"
"strings"

"github.com/apache/thrift/lib/go/thrift"

Expand Down Expand Up @@ -44,3 +45,23 @@ func AddClientHeader(ctx context.Context, key, value string) context.Context {
headers = append(headers, key)
return thrift.SetWriteHeaderList(ctx, headers)
}

// header gets the value of a thrift header by key
//
// If the value is not present we fall back to a case-insensitive check
func header(ctx context.Context, key string) (v string, ok bool) {
v, ok = thrift.GetHeader(ctx, key)
if !ok {
// We fall back to checking the key that an envoy proxy will send, which is a lower-case
// version of the thrift key.
//
//
// For details on this workaround, see:
// * https://github.com/reddit/baseplate.py/blob/0d189bae9bc15459b8d9d62856f31e45cbfcdb1a/baseplate/frameworks/thrift/__init__.py#L111
// * https://github.com/envoyproxy/envoy/issues/20595
// * https://github.com/reddit/baseplate.go/pull/500
v, ok = thrift.GetHeader(ctx, strings.ToLower(key))
}

return
}
14 changes: 7 additions & 7 deletions thriftbp/server_middlewares.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,16 +107,16 @@ func StartSpanFromThriftContext(ctx context.Context, name string) (context.Conte
var headers tracing.Headers
var sampled bool

if str, ok := thrift.GetHeader(ctx, transport.HeaderTracingTrace); ok {
if str, ok := header(ctx, transport.HeaderTracingTrace); ok {
headers.TraceID = str
}
if str, ok := thrift.GetHeader(ctx, transport.HeaderTracingSpan); ok {
if str, ok := header(ctx, transport.HeaderTracingSpan); ok {
headers.SpanID = str
}
if str, ok := thrift.GetHeader(ctx, transport.HeaderTracingFlags); ok {
if str, ok := header(ctx, transport.HeaderTracingFlags); ok {
headers.Flags = str
}
if str, ok := thrift.GetHeader(ctx, transport.HeaderTracingSampled); ok {
if str, ok := header(ctx, transport.HeaderTracingSampled); ok {
sampled = str == transport.HeaderTracingSampledTrue
headers.Sampled = &sampled
}
Expand Down Expand Up @@ -151,7 +151,7 @@ func InjectServerSpan(suppressor errorsbp.Suppressor) thrift.ProcessorMiddleware
return thrift.WrappedTProcessorFunction{
Wrapped: func(ctx context.Context, seqID int32, in, out thrift.TProtocol) (success bool, err thrift.TException) {
ctx, span := StartSpanFromThriftContext(ctx, name)
if userAgent, ok := thrift.GetHeader(ctx, transport.HeaderUserAgent); ok {
if userAgent, ok := header(ctx, transport.HeaderUserAgent); ok {
span.SetTag(tracing.TagKeyPeerService, userAgent)
}
defer func() {
Expand All @@ -171,7 +171,7 @@ func InjectServerSpan(suppressor errorsbp.Suppressor) thrift.ProcessorMiddleware
// headers set on the context onto the context and configures Thrift to forward
// the edge requent context header on any Thrift calls made by the server.
func InitializeEdgeContext(ctx context.Context, impl ecinterface.Interface) context.Context {
header, ok := thrift.GetHeader(ctx, transport.HeaderEdgeRequest)
header, ok := header(ctx, transport.HeaderEdgeRequest)
if !ok {
return ctx
}
Expand Down Expand Up @@ -211,7 +211,7 @@ func InjectEdgeContext(impl ecinterface.Interface) thrift.ProcessorMiddleware {
func ExtractDeadlineBudget(name string, next thrift.TProcessorFunction) thrift.TProcessorFunction {
return thrift.WrappedTProcessorFunction{
Wrapped: func(ctx context.Context, seqID int32, in, out thrift.TProtocol) (bool, thrift.TException) {
if s, ok := thrift.GetHeader(ctx, transport.HeaderDeadlineBudget); ok {
if s, ok := header(ctx, transport.HeaderDeadlineBudget); ok {
v, err := strconv.ParseInt(s, 10, 64)
if err == nil && v >= 1 {
var cancel context.CancelFunc
Expand Down