Skip to content

Commit

Permalink
registry: remove dependency on logrus for client
Browse files Browse the repository at this point in the history
To simplify the vendoring story for the client, we have now removed the
requirement for `logrus` and the forked `context` package (usually
imported as `dcontext`). We inject the logger via the metrics tracker
for the blob cache and via options on the token handler. We preserve
logs on the proxy cache for that case. Clients expecting these log
messages may need to be updated accordingly.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
  • Loading branch information
stevvooe committed Aug 15, 2017
1 parent 06fa77a commit 860b28c
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 8 deletions.
21 changes: 18 additions & 3 deletions registry/client/auth/session.go
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/docker/distribution/registry/client"
"github.com/docker/distribution/registry/client/auth/challenge"
"github.com/docker/distribution/registry/client/transport"
"github.com/sirupsen/logrus"
)

var (
Expand Down Expand Up @@ -135,6 +134,8 @@ type tokenHandler struct {
tokenLock sync.Mutex
tokenCache string
tokenExpiration time.Time

logger Logger
}

// Scope is a type which is serializable to a string
Expand Down Expand Up @@ -176,6 +177,18 @@ func (rs RegistryScope) String() string {
return fmt.Sprintf("registry:%s:%s", rs.Name, strings.Join(rs.Actions, ","))
}

// Logger defines the injectable logging interface, used on TokenHandlers.
type Logger interface {
Debugf(format string, args ...interface{})
}

func logDebugf(logger Logger, format string, args ...interface{}) {
if logger == nil {
return
}
logger.Debugf(format, args...)
}

// TokenHandlerOptions is used to configure a new token handler
type TokenHandlerOptions struct {
Transport http.RoundTripper
Expand All @@ -185,6 +198,7 @@ type TokenHandlerOptions struct {
ForceOAuth bool
ClientID string
Scopes []Scope
Logger Logger
}

// An implementation of clock for providing real time data.
Expand Down Expand Up @@ -220,6 +234,7 @@ func NewTokenHandlerWithOptions(options TokenHandlerOptions) AuthenticationHandl
clientID: options.ClientID,
scopes: options.Scopes,
clock: realClock{},
logger: options.Logger,
}

return handler
Expand Down Expand Up @@ -348,7 +363,7 @@ func (th *tokenHandler) fetchTokenWithOAuth(realm *url.URL, refreshToken, servic
if tr.ExpiresIn < minimumTokenLifetimeSeconds {
// The default/minimum lifetime.
tr.ExpiresIn = minimumTokenLifetimeSeconds
logrus.Debugf("Increasing token expiration to: %d seconds", tr.ExpiresIn)
logDebugf(th.logger, "Increasing token expiration to: %d seconds", tr.ExpiresIn)
}

if tr.IssuedAt.IsZero() {
Expand Down Expand Up @@ -439,7 +454,7 @@ func (th *tokenHandler) fetchTokenWithBasicAuth(realm *url.URL, service string,
if tr.ExpiresIn < minimumTokenLifetimeSeconds {
// The default/minimum lifetime.
tr.ExpiresIn = minimumTokenLifetimeSeconds
logrus.Debugf("Increasing token expiration to: %d seconds", tr.ExpiresIn)
logDebugf(th.logger, "Increasing token expiration to: %d seconds", tr.ExpiresIn)
}

if tr.IssuedAt.IsZero() {
Expand Down
15 changes: 14 additions & 1 deletion registry/proxy/proxyregistry.go
Expand Up @@ -121,8 +121,21 @@ func (pr *proxyingRegistry) Repositories(ctx context.Context, repos []string, la
func (pr *proxyingRegistry) Repository(ctx context.Context, name reference.Named) (distribution.Repository, error) {
c := pr.authChallenger

tkopts := auth.TokenHandlerOptions{
Transport: http.DefaultTransport,
Credentials: c.credentialStore(),
Scopes: []auth.Scope{
auth.RepositoryScope{
Repository: name.Name(),
Actions: []string{"pull"},
},
},
Logger: dcontext.GetLogger(ctx),
}

tr := transport.NewTransport(http.DefaultTransport,
auth.NewAuthorizer(c.challengeManager(), auth.NewTokenHandler(http.DefaultTransport, c.credentialStore(), name.Name(), "pull")))
auth.NewAuthorizer(c.challengeManager(),
auth.NewTokenHandlerWithOptions(tkopts)))

localRepo, err := pr.embedded.Repository(ctx, name)
if err != nil {
Expand Down
6 changes: 6 additions & 0 deletions registry/storage/blobcachemetrics.go
@@ -1,9 +1,11 @@
package storage

import (
"context"
"expvar"
"sync/atomic"

dcontext "github.com/docker/distribution/context"
"github.com/docker/distribution/registry/storage/cache"
)

Expand All @@ -25,6 +27,10 @@ func (bsc *blobStatCollector) Metrics() cache.Metrics {
return bsc.metrics
}

func (bsc *blobStatCollector) Logger(ctx context.Context) cache.Logger {
return dcontext.GetLogger(ctx)
}

// blobStatterCacheMetrics keeps track of cache metrics for blob descriptor
// cache requests. Note this is kept globally and made available via expvar.
// For more detailed metrics, its recommend to instrument a particular cache
Expand Down
27 changes: 23 additions & 4 deletions registry/storage/cache/cachedblobdescriptorstore.go
Expand Up @@ -4,7 +4,6 @@ import (
"context"

"github.com/docker/distribution"
dcontext "github.com/docker/distribution/context"
"github.com/opencontainers/go-digest"
)

Expand All @@ -17,12 +16,20 @@ type Metrics struct {
Misses uint64
}

// Logger can be provided on the MetricsTracker to log errors.
//
// Usually, this is just a proxy to dcontext.GetLogger.
type Logger interface {
Errorf(format string, args ...interface{})
}

// MetricsTracker represents a metric tracker
// which simply counts the number of hits and misses.
type MetricsTracker interface {
Hit()
Miss()
Metrics() Metrics
Logger(context.Context) Logger
}

type cachedBlobStatter struct {
Expand Down Expand Up @@ -54,7 +61,7 @@ func (cbds *cachedBlobStatter) Stat(ctx context.Context, dgst digest.Digest) (di
desc, err := cbds.cache.Stat(ctx, dgst)
if err != nil {
if err != distribution.ErrBlobUnknown {
dcontext.GetLogger(ctx).Errorf("error retrieving descriptor from cache: %v", err)
logErrorf(ctx, cbds.tracker, "error retrieving descriptor from cache: %v", err)
}

goto fallback
Expand All @@ -74,7 +81,7 @@ fallback:
}

if err := cbds.cache.SetDescriptor(ctx, dgst, desc); err != nil {
dcontext.GetLogger(ctx).Errorf("error adding descriptor %v to cache: %v", desc.Digest, err)
logErrorf(ctx, cbds.tracker, "error adding descriptor %v to cache: %v", desc.Digest, err)
}

return desc, err
Expand All @@ -96,7 +103,19 @@ func (cbds *cachedBlobStatter) Clear(ctx context.Context, dgst digest.Digest) er

func (cbds *cachedBlobStatter) SetDescriptor(ctx context.Context, dgst digest.Digest, desc distribution.Descriptor) error {
if err := cbds.cache.SetDescriptor(ctx, dgst, desc); err != nil {
dcontext.GetLogger(ctx).Errorf("error adding descriptor %v to cache: %v", desc.Digest, err)
logErrorf(ctx, cbds.tracker, "error adding descriptor %v to cache: %v", desc.Digest, err)
}
return nil
}

func logErrorf(ctx context.Context, tracker MetricsTracker, format string, args ...interface{}) {
if tracker == nil {
return
}

logger := tracker.Logger(ctx)
if logger == nil {
return
}
logger.Errorf(format, args...)
}

0 comments on commit 860b28c

Please sign in to comment.