From 04d0280ca1338b93ac6e3026a8a2d852fbb46ef2 Mon Sep 17 00:00:00 2001 From: Arne Date: Thu, 29 Dec 2022 15:06:19 +0100 Subject: [PATCH] feat: improve tracing (#2992) --- cmd/daemon/serve.go | 9 ++++++-- courier/courier_dispatcher_test.go | 1 + courier/handler_test.go | 4 ++-- courier/sms_test.go | 4 ++-- identity/validator.go | 13 ++++++----- selfservice/hook/address_verifier.go | 2 +- selfservice/hook/session_destroyer.go | 26 ++++++++++++++-------- selfservice/hook/session_issuer.go | 10 +++++++++ selfservice/hook/verification.go | 13 +++++++++-- selfservice/hook/web_hook.go | 24 ++++++++++---------- selfservice/strategy/password/validator.go | 7 ++++++ 11 files changed, 78 insertions(+), 35 deletions(-) diff --git a/cmd/daemon/serve.go b/cmd/daemon/serve.go index ae92288e701..62ab630ca51 100644 --- a/cmd/daemon/serve.go +++ b/cmd/daemon/serve.go @@ -166,7 +166,7 @@ func ServeAdmin(r driver.Registry, cmd *cobra.Command, args []string, slOpts *se ) if r.Config().DisableAdminHealthRequestLog(ctx) { - adminLogger.ExcludePaths(x.AdminPrefix+healthx.AliveCheckPath, x.AdminPrefix+healthx.ReadyCheckPath) + adminLogger.ExcludePaths(x.AdminPrefix+healthx.AliveCheckPath, x.AdminPrefix+healthx.ReadyCheckPath, x.AdminPrefix+prometheus.MetricsPrometheusPath) } n.Use(adminLogger) n.UseFunc(x.RedirectAdminMiddleware) @@ -183,7 +183,12 @@ func ServeAdmin(r driver.Registry, cmd *cobra.Command, args []string, slOpts *se var handler http.Handler = n if tracer := r.Tracer(ctx); tracer.IsLoaded() { - handler = otelx.TraceHandler(handler, otelhttp.WithTracerProvider(tracer.Provider())) + handler = otelx.TraceHandler(handler, + otelhttp.WithTracerProvider(tracer.Provider()), + otelhttp.WithFilter(func(req *http.Request) bool { + return req.URL.Path != x.AdminPrefix+prometheus.MetricsPrometheusPath + }), + ) } // #nosec G112 - the correct settings are set by graceful.WithDefaults diff --git a/courier/courier_dispatcher_test.go b/courier/courier_dispatcher_test.go index 8b807de581e..544584836d4 100644 --- a/courier/courier_dispatcher_test.go +++ b/courier/courier_dispatcher_test.go @@ -53,6 +53,7 @@ func TestDispatchMessageWithInvalidSMTP(t *testing.T) { require.Error(t, err) messages, err := reg.CourierPersister().NextMessages(ctx, 10) + require.NoError(t, err) require.Len(t, messages, 1) }) } diff --git a/courier/handler_test.go b/courier/handler_test.go index 1b017f28341..135f7209a9a 100644 --- a/courier/handler_test.go +++ b/courier/handler_test.go @@ -7,7 +7,7 @@ import ( "context" "errors" "fmt" - "io/ioutil" + "io" "net/http" "net/http/httptest" "testing" @@ -61,7 +61,7 @@ func TestHandler(t *testing.T) { t.Helper() res, err := base.Client().Get(base.URL + href) require.NoError(t, err) - body, err := ioutil.ReadAll(res.Body) + body, err := io.ReadAll(res.Body) require.NoError(t, err) require.NoError(t, res.Body.Close()) diff --git a/courier/sms_test.go b/courier/sms_test.go index a433b440785..c77358f5aba 100644 --- a/courier/sms_test.go +++ b/courier/sms_test.go @@ -123,11 +123,11 @@ func TestDisallowedInternalNetwork(t *testing.T) { ctx := context.Background() conf, reg := internal.NewFastRegistryWithMocks(t) - conf.MustSet(ctx, config.ViperKeyCourierSMSRequestConfig, fmt.Sprintf(`{ + conf.MustSet(ctx, config.ViperKeyCourierSMSRequestConfig, `{ "url": "http://127.0.0.1/", "method": "GET", "body": "file://./stub/request.config.twilio.jsonnet" - }`)) + }`) conf.MustSet(ctx, config.ViperKeyCourierSMSEnabled, true) conf.MustSet(ctx, config.ViperKeyCourierSMTPURL, "http://foo.url") conf.MustSet(ctx, config.ViperKeyClientHTTPNoPrivateIPRanges, true) diff --git a/identity/validator.go b/identity/validator.go index 010bbefb739..6e5c197eed6 100644 --- a/identity/validator.go +++ b/identity/validator.go @@ -10,6 +10,7 @@ import ( "github.com/ory/kratos/driver/config" "github.com/ory/kratos/schema" + "github.com/ory/x/otelx" ) type ( @@ -55,9 +56,11 @@ func (v *Validator) ValidateWithRunner(ctx context.Context, i *Identity, runners } func (v *Validator) Validate(ctx context.Context, i *Identity) error { - return v.ValidateWithRunner(ctx, i, - NewSchemaExtensionCredentials(i), - NewSchemaExtensionVerification(i, v.d.Config().SelfServiceFlowVerificationRequestLifespan(ctx)), - NewSchemaExtensionRecovery(i), - ) + return otelx.WithSpan(ctx, "identity.Validator.Validate", func(ctx context.Context) error { + return v.ValidateWithRunner(ctx, i, + NewSchemaExtensionCredentials(i), + NewSchemaExtensionVerification(i, v.d.Config().SelfServiceFlowVerificationRequestLifespan(ctx)), + NewSchemaExtensionRecovery(i), + ) + }) } diff --git a/selfservice/hook/address_verifier.go b/selfservice/hook/address_verifier.go index ce6ae3b645d..b4c3ee44558 100644 --- a/selfservice/hook/address_verifier.go +++ b/selfservice/hook/address_verifier.go @@ -17,7 +17,7 @@ import ( "github.com/ory/kratos/session" ) -var _ login.PostHookExecutor = new(SessionDestroyer) +var _ login.PostHookExecutor = new(AddressVerifier) type AddressVerifier struct{} diff --git a/selfservice/hook/session_destroyer.go b/selfservice/hook/session_destroyer.go index b03a5348318..d0080343229 100644 --- a/selfservice/hook/session_destroyer.go +++ b/selfservice/hook/session_destroyer.go @@ -4,13 +4,17 @@ package hook import ( + "context" "net/http" - "github.com/ory/kratos/selfservice/flow/recovery" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" "github.com/ory/kratos/selfservice/flow/login" + "github.com/ory/kratos/selfservice/flow/recovery" "github.com/ory/kratos/session" "github.com/ory/kratos/ui/node" + "github.com/ory/x/otelx" ) var _ login.PostHookExecutor = new(SessionDestroyer) @@ -31,15 +35,19 @@ func NewSessionDestroyer(r sessionDestroyerDependencies) *SessionDestroyer { } func (e *SessionDestroyer) ExecuteLoginPostHook(_ http.ResponseWriter, r *http.Request, _ node.UiNodeGroup, _ *login.Flow, s *session.Session) error { - if _, err := e.r.SessionPersister().RevokeSessionsIdentityExcept(r.Context(), s.Identity.ID, s.ID); err != nil { - return err - } - return nil + return otelx.WithSpan(r.Context(), "selfservice.hook.ExecuteLoginPostHook", func(ctx context.Context) error { + if _, err := e.r.SessionPersister().RevokeSessionsIdentityExcept(ctx, s.Identity.ID, s.ID); err != nil { + return err + } + return nil + }, trace.WithAttributes(attribute.String("hook", KeySessionDestroyer))) } func (e *SessionDestroyer) ExecutePostRecoveryHook(_ http.ResponseWriter, r *http.Request, _ *recovery.Flow, s *session.Session) error { - if _, err := e.r.SessionPersister().RevokeSessionsIdentityExcept(r.Context(), s.Identity.ID, s.ID); err != nil { - return err - } - return nil + return otelx.WithSpan(r.Context(), "selfservice.hook.ExecutePostRecoveryHook", func(ctx context.Context) error { + if _, err := e.r.SessionPersister().RevokeSessionsIdentityExcept(ctx, s.Identity.ID, s.ID); err != nil { + return err + } + return nil + }, trace.WithAttributes(attribute.String("hook", KeySessionDestroyer))) } diff --git a/selfservice/hook/session_issuer.go b/selfservice/hook/session_issuer.go index 5a1a3738eb8..38064c99ede 100644 --- a/selfservice/hook/session_issuer.go +++ b/selfservice/hook/session_issuer.go @@ -4,15 +4,19 @@ package hook import ( + "context" "net/http" "time" "github.com/pkg/errors" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" "github.com/ory/kratos/selfservice/flow" "github.com/ory/kratos/selfservice/flow/registration" "github.com/ory/kratos/session" "github.com/ory/kratos/x" + "github.com/ory/x/otelx" ) var ( @@ -38,6 +42,12 @@ func NewSessionIssuer(r sessionIssuerDependencies) *SessionIssuer { } func (e *SessionIssuer) ExecutePostRegistrationPostPersistHook(w http.ResponseWriter, r *http.Request, a *registration.Flow, s *session.Session) error { + return otelx.WithSpan(r.Context(), "selfservice.hook.ExecutePostRegistrationPostPersistHook", func(ctx context.Context) error { + return e.executePostRegistrationPostPersistHook(w, r.WithContext(ctx), a, s) + }, trace.WithAttributes(attribute.String("hook", KeySessionIssuer))) +} + +func (e *SessionIssuer) executePostRegistrationPostPersistHook(w http.ResponseWriter, r *http.Request, a *registration.Flow, s *session.Session) error { s.AuthenticatedAt = time.Now().UTC() if err := e.r.SessionPersister().UpsertSession(r.Context(), s); err != nil { return err diff --git a/selfservice/hook/verification.go b/selfservice/hook/verification.go index be6f0c2721a..e16ac2204ca 100644 --- a/selfservice/hook/verification.go +++ b/selfservice/hook/verification.go @@ -4,8 +4,12 @@ package hook import ( + "context" "net/http" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" + "github.com/ory/kratos/driver/config" "github.com/ory/kratos/identity" "github.com/ory/kratos/selfservice/flow" @@ -14,6 +18,7 @@ import ( "github.com/ory/kratos/selfservice/flow/verification" "github.com/ory/kratos/session" "github.com/ory/kratos/x" + "github.com/ory/x/otelx" ) var _ registration.PostHookPostPersistExecutor = new(Verifier) @@ -36,11 +41,15 @@ func NewVerifier(r verifierDependencies) *Verifier { } func (e *Verifier) ExecutePostRegistrationPostPersistHook(_ http.ResponseWriter, r *http.Request, f *registration.Flow, s *session.Session) error { - return e.do(r, s.Identity, f) + return otelx.WithSpan(r.Context(), "selfservice.hook.ExecutePostRegistrationPostPersistHook", func(ctx context.Context) error { + return e.do(r.WithContext(ctx), s.Identity, f) + }, trace.WithAttributes(attribute.String("hook", "verifier"))) } func (e *Verifier) ExecuteSettingsPostPersistHook(w http.ResponseWriter, r *http.Request, a *settings.Flow, i *identity.Identity) error { - return e.do(r, i, a) + return otelx.WithSpan(r.Context(), "selfservice.hook.ExecuteSettingsPostPersistHook", func(ctx context.Context) error { + return e.do(r.WithContext(ctx), i, a) + }, trace.WithAttributes(attribute.String("hook", "verifier"))) } func (e *Verifier) do(r *http.Request, i *identity.Identity, f flow.Flow) error { diff --git a/selfservice/hook/web_hook.go b/selfservice/hook/web_hook.go index f23701abbdf..ac9528660ac 100644 --- a/selfservice/hook/web_hook.go +++ b/selfservice/hook/web_hook.go @@ -109,7 +109,7 @@ func (e *WebHook) ExecuteLoginPreHook(_ http.ResponseWriter, req *http.Request, RequestURL: x.RequestURL(req).String(), RequestCookies: cookies(req), }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) ExecuteLoginPostHook(_ http.ResponseWriter, req *http.Request, _ node.UiNodeGroup, flow *login.Flow, session *session.Session) error { @@ -122,7 +122,7 @@ func (e *WebHook) ExecuteLoginPostHook(_ http.ResponseWriter, req *http.Request, RequestCookies: cookies(req), Identity: session.Identity, }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) ExecuteVerificationPreHook(_ http.ResponseWriter, req *http.Request, flow *verification.Flow) error { @@ -134,7 +134,7 @@ func (e *WebHook) ExecuteVerificationPreHook(_ http.ResponseWriter, req *http.Re RequestURL: x.RequestURL(req).String(), RequestCookies: cookies(req), }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) ExecutePostVerificationHook(_ http.ResponseWriter, req *http.Request, flow *verification.Flow, id *identity.Identity) error { @@ -147,7 +147,7 @@ func (e *WebHook) ExecutePostVerificationHook(_ http.ResponseWriter, req *http.R RequestCookies: cookies(req), Identity: id, }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) ExecuteRecoveryPreHook(_ http.ResponseWriter, req *http.Request, flow *recovery.Flow) error { @@ -159,7 +159,7 @@ func (e *WebHook) ExecuteRecoveryPreHook(_ http.ResponseWriter, req *http.Reques RequestCookies: cookies(req), RequestURL: x.RequestURL(req).String(), }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) ExecutePostRecoveryHook(_ http.ResponseWriter, req *http.Request, flow *recovery.Flow, session *session.Session) error { @@ -172,7 +172,7 @@ func (e *WebHook) ExecutePostRecoveryHook(_ http.ResponseWriter, req *http.Reque RequestCookies: cookies(req), Identity: session.Identity, }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) ExecuteRegistrationPreHook(_ http.ResponseWriter, req *http.Request, flow *registration.Flow) error { @@ -184,7 +184,7 @@ func (e *WebHook) ExecuteRegistrationPreHook(_ http.ResponseWriter, req *http.Re RequestURL: x.RequestURL(req).String(), RequestCookies: cookies(req), }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) ExecutePostRegistrationPrePersistHook(_ http.ResponseWriter, req *http.Request, flow *registration.Flow, id *identity.Identity) error { @@ -200,7 +200,7 @@ func (e *WebHook) ExecutePostRegistrationPrePersistHook(_ http.ResponseWriter, r RequestCookies: cookies(req), Identity: id, }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) ExecutePostRegistrationPostPersistHook(_ http.ResponseWriter, req *http.Request, flow *registration.Flow, session *session.Session) error { @@ -216,7 +216,7 @@ func (e *WebHook) ExecutePostRegistrationPostPersistHook(_ http.ResponseWriter, RequestCookies: cookies(req), Identity: session.Identity, }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) ExecuteSettingsPreHook(_ http.ResponseWriter, req *http.Request, flow *settings.Flow) error { @@ -228,7 +228,7 @@ func (e *WebHook) ExecuteSettingsPreHook(_ http.ResponseWriter, req *http.Reques RequestURL: x.RequestURL(req).String(), RequestCookies: cookies(req), }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) ExecuteSettingsPostPersistHook(_ http.ResponseWriter, req *http.Request, flow *settings.Flow, id *identity.Identity) error { @@ -244,7 +244,7 @@ func (e *WebHook) ExecuteSettingsPostPersistHook(_ http.ResponseWriter, req *htt RequestCookies: cookies(req), Identity: id, }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) ExecuteSettingsPrePersistHook(_ http.ResponseWriter, req *http.Request, flow *settings.Flow, id *identity.Identity) error { @@ -260,7 +260,7 @@ func (e *WebHook) ExecuteSettingsPrePersistHook(_ http.ResponseWriter, req *http RequestCookies: cookies(req), Identity: id, }) - }) + }, trace.WithAttributes(attribute.String("hook", KeyWebHook))) } func (e *WebHook) execute(ctx context.Context, data *templateContext) error { diff --git a/selfservice/strategy/password/validator.go b/selfservice/strategy/password/validator.go index 775046b856e..8688ad96c28 100644 --- a/selfservice/strategy/password/validator.go +++ b/selfservice/strategy/password/validator.go @@ -25,6 +25,7 @@ import ( "github.com/ory/herodot" "github.com/ory/kratos/driver/config" "github.com/ory/x/httpx" + "github.com/ory/x/otelx" ) const hashCacheItemTTL = time.Hour @@ -162,6 +163,12 @@ func (s *DefaultPasswordValidator) fetch(hpw []byte, apiDNSName string) (int64, } func (s *DefaultPasswordValidator) Validate(ctx context.Context, identifier, password string) error { + return otelx.WithSpan(ctx, "password.DefaultPasswordValidator.Validate", func(ctx context.Context) error { + return s.validate(ctx, identifier, password) + }) +} + +func (s *DefaultPasswordValidator) validate(ctx context.Context, identifier, password string) error { passwordPolicyConfig := s.reg.Config().PasswordPolicyConfig(ctx) if len(password) < int(passwordPolicyConfig.MinPasswordLength) {