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

Jump to bottom

metadata: move FromOutgoingContextRaw() to internal #6765

Merged
merged 5 commits into from
Jan 18, 2024
Merged

metadata: move FromOutgoingContextRaw() to internal #6765

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c7d0145
Increment: Redefines metatdata.FromOutgoingContextRaw() as an interna…
Aditya-Sood Dec 14, 2023
208baa9
Merge branch 'master' into 4487-metadata-internal-method
Aditya-Sood Dec 14, 2023
2b34f91
Code Review Changes
Aditya-Sood Dec 22, 2023
26a8b06
Refactor: Fix comment for FromOutgoingContextRaw definition
Aditya-Sood Jan 15, 2024
4494626
Merge branch 'master' into 4487-metadata-internal-method
Aditya-Sood Jan 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions internal/internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ var (
ExitIdleModeForTesting any // func(*grpc.ClientConn) error

ChannelzTurnOffForTesting func()

// Gets the function definition from metadata package
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix formatting. We want the comment to start with the variable name. Also I feel like the comment doesnt explain what the method does, but instead talks about why this internal variable is being used.

how about something like this?

Suggested change
// Gets the function definition from metadata package
// FromOutgoingContextRaw returns the un-merged, intermediary contents of metadata.rawMD.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @arvindbr8, I've merged your commit suggestion

FromOutgoingContextRaw any // func(context.Context) (metadata.MD, [][]string, bool)
)

// HealthChecker defines the signature of the client-side LB channel health checking function.
Expand Down
4 changes: 3 additions & 1 deletion internal/transport/http2_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ import (
// atomically.
var clientConnectionCounter uint64

var metadataFromOutgoingContextRaw = internal.FromOutgoingContextRaw.(func(context.Context) (metadata.MD, [][]string, bool))

// http2Client implements the ClientTransport interface with HTTP2.
type http2Client struct {
lastRead int64 // Keep this field 64-bit aligned. Accessed atomically.
Expand Down Expand Up @@ -568,7 +570,7 @@ func (t *http2Client) createHeaderFields(ctx context.Context, callHdr *CallHdr)
headerFields = append(headerFields, hpack.HeaderField{Name: "grpc-trace-bin", Value: encodeBinHeader(b)})
}

if md, added, ok := metadata.FromOutgoingContextRaw(ctx); ok {
if md, added, ok := metadataFromOutgoingContextRaw(ctx); ok {
var k string
for k, vv := range md {
// HTTP doesn't allow you to set pseudoheaders after non pseudoheaders were set.
Expand Down
13 changes: 8 additions & 5 deletions metadata/metadata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,14 @@ import (
"context"
"fmt"
"strings"

"google.golang.org/grpc/internal"
)

func init() {
internal.FromOutgoingContextRaw = fromOutgoingContextRaw
}

// DecodeKeyValue returns k, v, nil.
//
// Deprecated: use k and v directly instead.
Expand Down Expand Up @@ -238,16 +244,13 @@ func copyOf(v []string) []string {
return vals
}

// FromOutgoingContextRaw returns the un-merged, intermediary contents of rawMD.
// fromOutgoingContextRaw returns the un-merged, intermediary contents of rawMD.
//
// Remember to perform strings.ToLower on the keys, for both the returned MD (MD
// is a map, there's no guarantee it's created using our helper functions) and
// the extra kv pairs (AppendToOutgoingContext doesn't turn them into
// lowercase).
//
// This is intended for gRPC-internal use ONLY. Users should use
// FromOutgoingContext instead.
func FromOutgoingContextRaw(ctx context.Context) (MD, [][]string, bool) {
func fromOutgoingContextRaw(ctx context.Context) (MD, [][]string, bool) {
raw, ok := ctx.Value(mdOutgoingKey{}).(rawMD)
if !ok {
return nil, nil, false
Expand Down
4 changes: 3 additions & 1 deletion stream.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ import (
"google.golang.org/grpc/status"
)

var metadataFromOutgoingContextRaw = internal.FromOutgoingContextRaw.(func(context.Context) (metadata.MD, [][]string, bool))

// StreamHandler defines the handler called by gRPC server to complete the
// execution of a streaming RPC.
//
Expand Down Expand Up @@ -184,7 +186,7 @@ func newClientStream(ctx context.Context, desc *StreamDesc, cc *ClientConn, meth
// when the RPC completes.
opts = append([]CallOption{OnFinish(func(error) { cc.idlenessMgr.OnCallEnd() })}, opts...)

if md, added, ok := metadata.FromOutgoingContextRaw(ctx); ok {
if md, added, ok := metadataFromOutgoingContextRaw(ctx); ok {
// validate md
if err := imetadata.Validate(md); err != nil {
return nil, status.Error(codes.Internal, err.Error())
Expand Down