From 70dd8bef88dee295600f295dfcee60118d1b705b Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Thu, 16 Feb 2023 14:38:27 +0100 Subject: [PATCH] feat: emit events through tracing (#1244) --- cmd/server/serve.go | 2 +- internal/check/engine.go | 20 ++++++------- internal/check/engine_test.go | 14 +++++---- internal/check/handler.go | 6 ++++ internal/driver/registry_default.go | 15 ++++++++++ internal/driver/registry_factory.go | 1 + internal/expand/engine.go | 22 ++++++++++---- internal/expand/engine_test.go | 16 +++++++---- internal/relationtuple/handler.go | 5 ++-- internal/relationtuple/transact_server.go | 26 +++++++++++------ internal/x/events/events.go | 35 +++++++++++++++++++++++ internal/x/registry.go | 5 ++++ ketoctx/options.go | 14 ++++++--- 13 files changed, 138 insertions(+), 43 deletions(-) create mode 100644 internal/x/events/events.go diff --git a/cmd/server/serve.go b/cmd/server/serve.go index 525442568..5ee39342f 100644 --- a/cmd/server/serve.go +++ b/cmd/server/serve.go @@ -23,7 +23,7 @@ ORY Keto can be configured using environment variables as well as a configuratio on configuration options, open the configuration documentation: >> https://www.ory.sh/keto/docs/reference/configuration <<`, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { reg, err := helpers.NewRegistry(cmd, opts) if err != nil { return err diff --git a/internal/check/engine.go b/internal/check/engine.go index 5fd24e6c4..20f23d257 100644 --- a/internal/check/engine.go +++ b/internal/check/engine.go @@ -7,10 +7,8 @@ import ( "context" "github.com/ory/herodot" + "github.com/ory/x/otelx" "github.com/pkg/errors" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/codes" - "go.opentelemetry.io/otel/trace" "github.com/ory/keto/internal/check/checkgroup" "github.com/ory/keto/internal/driver/config" @@ -19,6 +17,7 @@ import ( "github.com/ory/keto/internal/persistence" "github.com/ory/keto/internal/relationtuple" "github.com/ory/keto/internal/x" + "github.com/ory/keto/internal/x/events" "github.com/ory/keto/internal/x/graph" "github.com/ory/keto/ketoapi" ) @@ -35,6 +34,8 @@ type ( persistence.Provider config.Provider x.LoggerProvider + x.TracingProvider + x.NetworkIDProvider } EngineOpt func(*Engine) @@ -70,15 +71,10 @@ func (e *Engine) CheckIsMember(ctx context.Context, r *relationTuple, restDepth // the object in the namespace either directly or indirectly and returns a check // result. func (e *Engine) CheckRelationTuple(ctx context.Context, r *relationTuple, restDepth int) (res checkgroup.Result) { - ctx, span := trace.SpanFromContext(ctx).TracerProvider().Tracer("keto/internal/check").Start(ctx, "Engine.CheckRelationTuple") - defer func() { - if res.Err != nil { - span.SetStatus(codes.Error, res.Err.Error()) - } else { - span.SetAttributes(attribute.String("membership", res.Membership.String())) - } - span.End() - }() + ctx, span := e.d.Tracer(ctx).Tracer().Start(ctx, "Engine.CheckRelationTuple") + defer otelx.End(span, &res.Err) + + events.Add(ctx, e.d, events.PermissionsChecked) // global max-depth takes precedence when it is the lesser or if the request // max-depth is less than or equal to 0 diff --git a/internal/check/engine_test.go b/internal/check/engine_test.go index 9534a96b3..8169149fc 100644 --- a/internal/check/engine_test.go +++ b/internal/check/engine_test.go @@ -30,6 +30,7 @@ type deps struct { configProvider x.LoggerProvider x.TracingProvider + x.NetworkIDProvider } func newDepsProvider(t testing.TB, namespaces []*namespace.Namespace, pageOpts ...x.PaginationOptionSetter) *deps { @@ -38,11 +39,12 @@ func newDepsProvider(t testing.TB, namespaces []*namespace.Namespace, pageOpts . mr := relationtuple.NewManagerWrapper(t, reg, pageOpts...) return &deps{ - ManagerWrapper: mr, - Provider: reg, - configProvider: reg, - LoggerProvider: reg, - TracingProvider: reg, + ManagerWrapper: mr, + Provider: reg, + configProvider: reg, + LoggerProvider: reg, + TracingProvider: reg, + NetworkIDProvider: reg, } } @@ -292,7 +294,7 @@ func TestEngine(t *testing.T) { {Name: "obj"}, {Name: "org"}, }) - //require.NoError(t, reg.RelationTupleManager().WriteRelationTuples(ctx, &writeRel, &orgOwnerRel, &userMembershipRel)) + // require.NoError(t, reg.RelationTupleManager().WriteRelationTuples(ctx, &writeRel, &orgOwnerRel, &userMembershipRel)) insertFixtures(t, reg.RelationTupleManager(), []string{ "obj:object#write@obj:object#owner", "obj:object#owner@org:organization#member", diff --git a/internal/check/handler.go b/internal/check/handler.go index 735c725e0..a2b077756 100644 --- a/internal/check/handler.go +++ b/internal/check/handler.go @@ -112,6 +112,7 @@ func (h *Handler) getCheckNoStatus(w http.ResponseWriter, r *http.Request, _ htt // Check Permission Or Error Request Parameters // // swagger:parameters checkPermissionOrError +// nolint:deadcode,unused type checkPermissionOrError struct { // in: query MaxDepth int `json:"max-depth"` @@ -176,6 +177,7 @@ func (h *Handler) getCheck(ctx context.Context, q url.Values) (bool, error) { // Check Permission using Post Request Parameters // // swagger:parameters postCheckPermission +// nolint:deadcode,unused type postCheckPermission struct { // in: query MaxDepth int `json:"max-depth"` @@ -187,6 +189,7 @@ type postCheckPermission struct { // Check Permission using Post Request Body // // swagger:model postCheckPermissionBody +// nolint:deadcode,unused type postCheckPermissionBody struct { ketoapi.RelationQuery } @@ -221,7 +224,9 @@ func (h *Handler) postCheckNoStatus(w http.ResponseWriter, r *http.Request, _ ht // Post Check Permission Or Error Request Parameters // // swagger:parameters postCheckPermissionOrError +// nolint:deadcode,unused type postCheckPermissionOrError struct { + // nolint:deadcode,unused // in: query MaxDepth int `json:"max-depth"` @@ -232,6 +237,7 @@ type postCheckPermissionOrError struct { // Post Check Permission Or Error Body // // swagger:model postCheckPermissionOrErrorBody +// nolint:deadcode,unused type postCheckPermissionOrErrorBody struct { ketoapi.RelationQuery } diff --git a/internal/driver/registry_default.go b/internal/driver/registry_default.go index 405c5a0cd..c6ba0372e 100644 --- a/internal/driver/registry_default.go +++ b/internal/driver/registry_default.go @@ -9,6 +9,7 @@ import ( "sync" "github.com/gobuffalo/pop/v6" + "github.com/gofrs/uuid" "github.com/ory/herodot" "github.com/ory/x/dbal" "github.com/ory/x/fsx" @@ -68,6 +69,7 @@ type ( handlers []Handler sqaService *metricsx.Service tracer *otelx.Tracer + tracerWrapper ketoctx.TracerWrapper pmm *prometheus.MetricsManager metricsHandler *prometheus.Handler @@ -148,6 +150,12 @@ func (r *RegistryDefault) Tracer(ctx context.Context) *otelx.Tracer { if err != nil { r.Logger().WithError(err).Fatalf("Unable to initialize Tracer.") } + + // Wrap the tracer if required + if r.tracerWrapper != nil { + t = r.tracerWrapper(t) + } + r.tracer = t } @@ -203,6 +211,13 @@ func (r *RegistryDefault) Persister() persistence.Persister { return r.p } +func (r *RegistryDefault) NetworkID(ctx context.Context) uuid.UUID { + if r.p == nil { + panic("no persister, but expected to have one") + } + return r.p.NetworkID(ctx) +} + func (r *RegistryDefault) Traverser() relationtuple.Traverser { if r.traverser == nil { panic("no traverser, but expected to have one") diff --git a/internal/driver/registry_factory.go b/internal/driver/registry_factory.go index 435cb72cb..ebf947932 100644 --- a/internal/driver/registry_factory.go +++ b/internal/driver/registry_factory.go @@ -78,6 +78,7 @@ func NewDefaultRegistry(ctx context.Context, flags *pflag.FlagSet, withoutNetwor r := &RegistryDefault{ c: c, l: l, + tracerWrapper: options.TracerWrapper, ctxer: options.Contextualizer(), defaultUnaryInterceptors: options.GRPCUnaryInterceptors(), defaultStreamInterceptors: options.GRPCStreamInterceptors(), diff --git a/internal/expand/engine.go b/internal/expand/engine.go index 16eb95606..3fe3d323c 100644 --- a/internal/expand/engine.go +++ b/internal/expand/engine.go @@ -6,13 +6,14 @@ package expand import ( "context" - "github.com/ory/keto/ketoapi" + "github.com/ory/x/otelx" "github.com/ory/keto/internal/driver/config" + "github.com/ory/keto/internal/relationtuple" "github.com/ory/keto/internal/x" + "github.com/ory/keto/internal/x/events" "github.com/ory/keto/internal/x/graph" - - "github.com/ory/keto/internal/relationtuple" + "github.com/ory/keto/ketoapi" ) type ( @@ -20,6 +21,8 @@ type ( relationtuple.ManagerProvider config.Provider x.LoggerProvider + x.TracingProvider + x.NetworkIDProvider } Engine struct { d EngineDependencies @@ -35,7 +38,16 @@ func NewEngine(d EngineDependencies) *Engine { } } -func (e *Engine) BuildTree(ctx context.Context, subject relationtuple.Subject, restDepth int) (*relationtuple.Tree, error) { +func (e *Engine) BuildTree(ctx context.Context, subject relationtuple.Subject, restDepth int) (t *relationtuple.Tree, err error) { + ctx, span := e.d.Tracer(ctx).Tracer().Start(ctx, "Engine.BuildTree") + defer otelx.End(span, &err) + events.Add(ctx, e.d, events.PermissionsExpanded) + + t, err = e.buildTreeRecursive(ctx, subject, restDepth) + return +} + +func (e *Engine) buildTreeRecursive(ctx context.Context, subject relationtuple.Subject, restDepth int) (*relationtuple.Tree, error) { // global max-depth takes precedence when it is the lesser or if the request max-depth is less than or equal to 0 if globalMaxDepth := e.d.Config(ctx).MaxReadDepth(); restDepth <= 0 || globalMaxDepth < restDepth { restDepth = globalMaxDepth @@ -89,7 +101,7 @@ func (e *Engine) BuildTree(ctx context.Context, subject relationtuple.Subject, r children := make([]*relationtuple.Tree, len(rels)) for ri, r := range rels { - child, err := e.BuildTree(ctx, r.Subject, restDepth-1) + child, err := e.buildTreeRecursive(ctx, r.Subject, restDepth-1) if err != nil { return nil, err } diff --git a/internal/expand/engine_test.go b/internal/expand/engine_test.go index 8679fd717..5d960fc40 100644 --- a/internal/expand/engine_test.go +++ b/internal/expand/engine_test.go @@ -27,14 +27,18 @@ import ( "github.com/ory/keto/internal/driver" ) -type configProvider = config.Provider -type loggerProvider = x.LoggerProvider +type ( + configProvider = config.Provider + loggerProvider = x.LoggerProvider +) // deps is defined to capture engine dependencies in a single struct type deps struct { *relationtuple.ManagerWrapper // managerProvider configProvider loggerProvider + x.TracingProvider + x.NetworkIDProvider } func newTestEngine(t *testing.T, namespaces []*namespace.Namespace, paginationOpts ...x.PaginationOptionSetter) (*relationtuple.ManagerWrapper, *expand.Engine) { @@ -42,9 +46,11 @@ func newTestEngine(t *testing.T, namespaces []*namespace.Namespace, paginationOp require.NoError(t, innerReg.Config(context.Background()).Set(config.KeyNamespaces, namespaces)) reg := relationtuple.NewManagerWrapper(t, innerReg, paginationOpts...) e := expand.NewEngine(&deps{ - ManagerWrapper: reg, - configProvider: innerReg, - loggerProvider: innerReg, + ManagerWrapper: reg, + configProvider: innerReg, + loggerProvider: innerReg, + TracingProvider: innerReg, + NetworkIDProvider: innerReg, }) return reg, e } diff --git a/internal/relationtuple/handler.go b/internal/relationtuple/handler.go index f3ea6a685..dc54199a4 100644 --- a/internal/relationtuple/handler.go +++ b/internal/relationtuple/handler.go @@ -6,9 +6,8 @@ package relationtuple import ( "google.golang.org/grpc" - rts "github.com/ory/keto/proto/ory/keto/relation_tuples/v1alpha2" - "github.com/ory/keto/internal/x" + rts "github.com/ory/keto/proto/ory/keto/relation_tuples/v1alpha2" ) type ( @@ -17,6 +16,8 @@ type ( MapperProvider x.LoggerProvider x.WriterProvider + x.TracingProvider + x.NetworkIDProvider } handler struct { d handlerDeps diff --git a/internal/relationtuple/transact_server.go b/internal/relationtuple/transact_server.go index b84dd92e3..9369b81bd 100644 --- a/internal/relationtuple/transact_server.go +++ b/internal/relationtuple/transact_server.go @@ -8,20 +8,18 @@ import ( "encoding/json" "net/http" - "github.com/ory/keto/internal/x/validate" - "github.com/ory/keto/ketoapi" - - rts "github.com/ory/keto/proto/ory/keto/relation_tuples/v1alpha2" - "github.com/julienschmidt/httprouter" "github.com/ory/herodot" "github.com/pkg/errors" -) -var ( - _ rts.WriteServiceServer = (*handler)(nil) + "github.com/ory/keto/internal/x/events" + "github.com/ory/keto/internal/x/validate" + "github.com/ory/keto/ketoapi" + rts "github.com/ory/keto/proto/ory/keto/relation_tuples/v1alpha2" ) +var _ rts.WriteServiceServer = (*handler)(nil) + func protoTuplesWithAction(deltas []*rts.RelationTupleDelta, action rts.RelationTupleDelta_Action) (filtered []*ketoapi.RelationTuple, err error) { for _, d := range deltas { if d.Action == action { @@ -36,6 +34,8 @@ func protoTuplesWithAction(deltas []*rts.RelationTupleDelta, action rts.Relation } func (h *handler) TransactRelationTuples(ctx context.Context, req *rts.TransactRelationTuplesRequest) (*rts.TransactRelationTuplesResponse, error) { + events.Add(ctx, h.d, events.RelationtuplesChanged) + insertTuples, err := protoTuplesWithAction(req.RelationTupleDeltas, rts.RelationTupleDelta_ACTION_INSERT) if err != nil { return nil, err @@ -66,6 +66,8 @@ func (h *handler) TransactRelationTuples(ctx context.Context, req *rts.TransactR } func (h *handler) DeleteRelationTuples(ctx context.Context, req *rts.DeleteRelationTuplesRequest) (*rts.DeleteRelationTuplesResponse, error) { + events.Add(ctx, h.d, events.RelationtuplesDeleted) + var q ketoapi.RelationQuery switch { @@ -91,6 +93,7 @@ func (h *handler) DeleteRelationTuples(ctx context.Context, req *rts.DeleteRelat // Create Relationship Request Parameters // // swagger:parameters createRelationship +// nolint:deadcode,unused type createRelationship struct { // in: body Body createRelationshipBody @@ -99,6 +102,7 @@ type createRelationship struct { // Create Relationship Request Body // // swagger:model createRelationshipBody +// nolint:deadcode,unused type createRelationshipBody struct { ketoapi.RelationQuery } @@ -124,6 +128,8 @@ type createRelationshipBody struct { func (h *handler) createRelation(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { ctx := r.Context() + events.Add(ctx, h.d, events.RelationtuplesCreated) + var rt ketoapi.RelationTuple if err := json.NewDecoder(r.Body).Decode(&rt); err != nil { h.d.Writer().WriteError(w, r, errors.WithStack(herodot.ErrBadRequest.WithError(err.Error()))) @@ -176,6 +182,8 @@ func (h *handler) createRelation(w http.ResponseWriter, r *http.Request, _ httpr func (h *handler) deleteRelations(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { ctx := r.Context() + events.Add(ctx, h.d, events.RelationtuplesDeleted) + if err := validate.All(r, validate.NoExtraQueryParams(ketoapi.RelationQueryKeys...), validate.QueryParamsContainsOneOf(ketoapi.NamespaceKey), @@ -244,6 +252,8 @@ func internalTuplesWithAction(deltas []*ketoapi.PatchDelta, action ketoapi.Patch func (h *handler) patchRelationTuples(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { ctx := r.Context() + events.Add(ctx, h.d, events.RelationtuplesChanged) + var deltas []*ketoapi.PatchDelta if err := json.NewDecoder(r.Body).Decode(&deltas); err != nil { h.d.Writer().WriteError(w, r, herodot.ErrBadRequest.WithError(err.Error())) diff --git a/internal/x/events/events.go b/internal/x/events/events.go new file mode 100644 index 000000000..a063d78c4 --- /dev/null +++ b/internal/x/events/events.go @@ -0,0 +1,35 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package events + +import ( + "context" + + "github.com/ory/x/otelx/semconv" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" + + "github.com/ory/keto/internal/x" +) + +type Event string + +const ( + RelationtuplesCreated Event = "RelationtuplesCreated" + RelationtuplesDeleted Event = "RelationtuplesDeleted" + RelationtuplesChanged Event = "RelationtuplesChanged" + + PermissionsExpanded Event = "PermissionsExpanded" + PermissionsChecked Event = "PermissionsChecked" +) + +// Add adds an event to the current span in the context. +func Add(ctx context.Context, p x.NetworkIDProvider, event Event) { + trace.SpanFromContext(ctx).AddEvent( + string(event), + trace.WithAttributes( + attribute.String(semconv.AttrNID, p.NetworkID(ctx).String()), + ), + ) +} diff --git a/internal/x/registry.go b/internal/x/registry.go index 00bddd941..4a0215c84 100644 --- a/internal/x/registry.go +++ b/internal/x/registry.go @@ -6,6 +6,7 @@ package x import ( "context" + "github.com/gofrs/uuid" "github.com/ory/herodot" "github.com/ory/x/logrusx" "github.com/ory/x/otelx" @@ -22,3 +23,7 @@ type WriterProvider interface { type TracingProvider interface { Tracer(ctx context.Context) *otelx.Tracer } + +type NetworkIDProvider interface { + NetworkID(context.Context) uuid.UUID +} diff --git a/ketoctx/options.go b/ketoctx/options.go index 6e3f1242e..1e0d7c4e9 100644 --- a/ketoctx/options.go +++ b/ketoctx/options.go @@ -8,6 +8,7 @@ import ( "github.com/ory/x/healthx" "github.com/ory/x/logrusx" + "github.com/ory/x/otelx" "github.com/ory/x/popx" "google.golang.org/grpc" ) @@ -15,6 +16,7 @@ import ( type ( opts struct { logger *logrusx.Logger + TracerWrapper TracerWrapper contextualizer Contextualizer httpMiddlewares []func(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) grpcUnaryInterceptors []grpc.UnaryServerInterceptor @@ -22,14 +24,18 @@ type ( migrationOpts []popx.MigrationBoxOption readyCheckers healthx.ReadyCheckers } - Option func(o *opts) + Option func(o *opts) + TracerWrapper func(*otelx.Tracer) *otelx.Tracer ) // WithLogger sets the logger. func WithLogger(l *logrusx.Logger) Option { - return func(o *opts) { - o.logger = l - } + return func(o *opts) { o.logger = l } +} + +// WithTracerWrapper sets a function that wraps the tracer. +func WithTracerWrapper(wrapper TracerWrapper) Option { + return func(o *opts) { o.TracerWrapper = wrapper } } // WithContextualizer sets the contextualizer.