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

[grpc storage]: Propagate tenant to grpc backend #6030

Merged
merged 3 commits into from
Oct 3, 2024
Merged

[grpc storage]: Propagate tenant to grpc backend #6030

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
090962d
storage: propagate tenant to grpc backend
frzifus Sep 30, 2024
37fb0e1
Revert "storage: propagate tenant to grpc backend"
frzifus Oct 1, 2024
20a31ba
fix: translate tenancy config from v1 to v2
frzifus Oct 1, 2024
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
27 changes: 24 additions & 3 deletions plugin/storage/grpc/shared/grpc_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,26 @@ import (
"github.com/jaegertracing/jaeger/model"
"github.com/jaegertracing/jaeger/pkg/bearertoken"
_ "github.com/jaegertracing/jaeger/pkg/gogocodec" // force gogo codec registration
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/proto-gen/storage_v1"
"github.com/jaegertracing/jaeger/storage/dependencystore"
"github.com/jaegertracing/jaeger/storage/spanstore"
)

// BearerTokenKey is the key name for the bearer token context value.
const BearerTokenKey = "bearer.token"
const (
// BearerTokenKey is the key name for the bearer token context value.
BearerTokenKey = "bearer.token"
// TenantKey is the key name for the x-tenant context value.
TenantKey = "x.tenant"
Copy link
Member Author

@frzifus frzifus Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Does it make sense to reuse the configured tenant header key?
Coming from --multi-tenancy.header=x-scope-orgid?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well.. That should already be the case. Since tenancy.NewClientUnaryInterceptor and tenancy.NewClientStreamInterceptor are registered.

Copy link
Member Author

Choose a reason for hiding this comment

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

Running Jaeger query without this patch:

SPAN_STORAGE_TYPE=grpc ./query --query.base-path=/ --grpc-storage.server=127.0.0.1:7777  --query.bearer-token-propagation=true  --multi-tenancy.enabled=true --multi-tenancy.header=x-scope-orgid --query.grpc-server.host-port=localhost:16685 --query.http-server.host-port=localhost:16686 --admin.http.host-port=localhost:16687

Then going a request:

curl -X GET "http://localhost:16686/api/traces?service=dummy" \
  -H "Authorization: Bearer abc" \
  -H "x-scope-orgid: 124"

Will only propagate the following metadata:

Stream Request Metadata: map[:authority:[127.0.0.1:7777] bearer.token:[abc] content-type:[application/grpc] grpc-accept-encoding:[snappy,zstd,gzip] user-agent:[grpc-go/1.65.0]]

Using this patch x.tenant:[124] will be part of it.

Copy link
Member Author

@frzifus frzifus Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Well.. That should already be the case. Since tenancy.NewClientUnaryInterceptor and tenancy.NewClientStreamInterceptor are registered.

Seems the problem was that this branch was never executed. Since the tenancy options from config_v1 have not been translated into config_v2.

jaeger/plugin/storage/grpc/factory.go

Lines 134 to 136 in 19386cf

if tenancyMgr.Enabled {
opts = append(opts, grpc.WithUnaryInterceptor(tenancy.NewClientUnaryInterceptor(tenancyMgr)))
opts = append(opts, grpc.WithStreamInterceptor(tenancy.NewClientStreamInterceptor(tenancyMgr)))

After adding the tenancy options in TranslateToConfigV2 the tenant infos x-scope-orgid:[124] are propagated.

Stream Request Metadata: map[:authority:[127.0.0.1:7777] bearer.token:[abc] content-type:[application/grpc] grpc-accept-encoding:[snappy,zstd,gzip] user-agent:[grpc-go/1.67.0] x-scope-orgid:[124]]

cc @yurishkuro @pavolloffay

)

var (
_ StoragePlugin = (*GRPCClient)(nil)
_ ArchiveStoragePlugin = (*GRPCClient)(nil)
_ PluginCapabilities = (*GRPCClient)(nil)

// upgradeContext composites several steps of upgrading context
frzifus marked this conversation as resolved.
Show resolved Hide resolved
upgradeContext = composeContextUpgradeFuncs(upgradeContextWithBearerToken)
upgradeContext = composeContextUpgradeFuncs(upgradeContextWithBearerToken, upgradeContextWithXTenant)
)

// GRPCClient implements shared.StoragePlugin and reads/writes spans and dependencies
Expand Down Expand Up @@ -88,6 +93,22 @@ func upgradeContextWithBearerToken(ctx context.Context) context.Context {
return ctx
}

// upgradeContextWithXTenant turns the context into a gRPC outgoing context with x tenant
// in the request metadata, if the original context has x-tenant attached.
// Otherwise returns original context.
func upgradeContextWithXTenant(ctx context.Context) context.Context {
tenant := tenancy.GetTenant(ctx)
if tenant != "" {
md, ok := metadata.FromOutgoingContext(ctx)
if !ok {
md = metadata.New(nil)
}
md.Set(TenantKey, tenant)
return metadata.NewOutgoingContext(ctx, md)
}
return ctx
}

// DependencyReader implements shared.StoragePlugin.
func (c *GRPCClient) DependencyReader() dependencystore.Reader {
return c
Expand Down
17 changes: 17 additions & 0 deletions plugin/storage/grpc/shared/grpc_client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (

"github.com/jaegertracing/jaeger/model"
"github.com/jaegertracing/jaeger/pkg/bearertoken"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/proto-gen/storage_v1"
grpcMocks "github.com/jaegertracing/jaeger/proto-gen/storage_v1/mocks"
"github.com/jaegertracing/jaeger/storage/spanstore"
Expand Down Expand Up @@ -132,6 +133,22 @@ func TestContextUpgradeWithoutToken(t *testing.T) {
assert.Falsef(t, ok, "Expected no metadata in context")
}

func TestContextUpgradeWithTenant(t *testing.T) {
testTenantValue := "test-tenant-val-1"
ctx := tenancy.WithTenant(context.Background(), testTenantValue)
upgradedCtx := upgradeContextWithXTenant(ctx)
md, ok := metadata.FromOutgoingContext(upgradedCtx)
assert.Truef(t, ok, "Expected metadata in context")
frzifus marked this conversation as resolved.
Show resolved Hide resolved
xTenantFromMetadata := md.Get(TenantKey)
assert.Equal(t, []string{testTenantValue}, xTenantFromMetadata)
}

func TestContextUpgradeWithoutTenant(t *testing.T) {
upgradedTenant := upgradeContextWithXTenant(context.Background())
_, ok := metadata.FromOutgoingContext(upgradedTenant)
assert.Falsef(t, ok, "Expected no metadata in context")
frzifus marked this conversation as resolved.
Show resolved Hide resolved
}

func TestGRPCClientGetServices(t *testing.T) {
withGRPCClient(func(r *grpcClientTest) {
r.spanReader.On("GetServices", mock.Anything, &storage_v1.GetServicesRequest{}).
Expand Down
Loading