Skip to content

Commit

Permalink
feat: improve tracing (#2992)
Browse files Browse the repository at this point in the history
  • Loading branch information
alnr committed Dec 29, 2022
1 parent 6fddfbf commit 04d0280
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 35 deletions.
9 changes: 7 additions & 2 deletions cmd/daemon/serve.go
Expand Up @@ -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)
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions courier/courier_dispatcher_test.go
Expand Up @@ -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)
})
}
Expand Down
4 changes: 2 additions & 2 deletions courier/handler_test.go
Expand Up @@ -7,7 +7,7 @@ import (
"context"
"errors"
"fmt"
"io/ioutil"
"io"
"net/http"
"net/http/httptest"
"testing"
Expand Down Expand Up @@ -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())

Expand Down
4 changes: 2 additions & 2 deletions courier/sms_test.go
Expand Up @@ -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)
Expand Down
13 changes: 8 additions & 5 deletions identity/validator.go
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/schema"
"github.com/ory/x/otelx"
)

type (
Expand Down Expand Up @@ -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),
)
})
}
2 changes: 1 addition & 1 deletion selfservice/hook/address_verifier.go
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/ory/kratos/session"
)

var _ login.PostHookExecutor = new(SessionDestroyer)
var _ login.PostHookExecutor = new(AddressVerifier)

type AddressVerifier struct{}

Expand Down
26 changes: 17 additions & 9 deletions selfservice/hook/session_destroyer.go
Expand Up @@ -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)
Expand All @@ -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)))
}
10 changes: 10 additions & 0 deletions selfservice/hook/session_issuer.go
Expand Up @@ -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 (
Expand All @@ -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
Expand Down
13 changes: 11 additions & 2 deletions selfservice/hook/verification.go
Expand Up @@ -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"
Expand All @@ -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)
Expand All @@ -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 {
Expand Down
24 changes: 12 additions & 12 deletions selfservice/hook/web_hook.go
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions selfservice/strategy/password/validator.go
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 04d0280

Please sign in to comment.