Skip to content

Commit

Permalink
Merge pull request #225 from Shopify/remove-tagloggable
Browse files Browse the repository at this point in the history
Remove the TagLoggable tooling
  • Loading branch information
pior committed Apr 12, 2024
2 parents b86453a + 5800bb6 commit 4be1d82
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 88 deletions.
8 changes: 6 additions & 2 deletions srvutil/metrics_observer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http"
"time"

"github.com/Shopify/goose/logger"
"github.com/Shopify/goose/metrics"
"github.com/Shopify/goose/redact"
"github.com/Shopify/goose/statsd"
Expand All @@ -29,10 +30,13 @@ func (o *DefaultRequestObserver) BeforeRequest(r *http.Request) {
func (o *DefaultRequestObserver) AfterRequest(r *http.Request, recorder HTTPRecorder, requestDuration time.Duration) {
ctx := r.Context()

ctx = statsd.WithTagLogFields(ctx, statsd.Tags{
fields := map[string]interface{}{
"statusCode": recorder.StatusCode(),
"statusClass": fmt.Sprintf("%dxx", recorder.StatusCode()/100), // 2xx, 5xx, etc.
})
}

ctx = statsd.WithTags(ctx, fields)
ctx = logger.WithFields(ctx, fields)

metrics.HTTPRequest.Duration(ctx, requestDuration)

Expand Down
45 changes: 0 additions & 45 deletions statsd/taggable.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"context"
"fmt"
"sort"

"github.com/Shopify/goose/logger"
)

// This create a private key-space in the Context, meaning that only this package can get or set "contextKey" types
Expand Down Expand Up @@ -83,49 +81,6 @@ func WatchingTaggable(ctx context.Context, t Taggable) context.Context {
return &taggableContext{Context: ctx, taggable: t}
}

// WithTagLogFields combines logger.WithFields(fields) and WithTags(tags)
// This simplifies the common operation of adding fields to the logger and the metrics
// This argument purposefully not typed as Tags, such that logrus.Fields and Tags can both be passed without additional casting.
func WithTagLogFields(ctx context.Context, tags map[string]interface{}) context.Context {
ctx = logger.WithFields(ctx, tags)
ctx = WithTags(ctx, tags)
return ctx
}

// WithTagLoggable combines WithTaggable and logger.WithLoggable
// If the Loggable is a Taggable already (implements StatsTags), it will be used directly.
// If StatsTags doesn't exist, LogFields() will be used instead.
func WithTagLoggable(ctx context.Context, l logger.Loggable) context.Context {
ctx = logger.WithLoggable(ctx, l)
if taggable, ok := l.(Taggable); ok {
ctx = WithTaggable(ctx, taggable)
} else {
ctx = WithTaggable(ctx, tagLoggable{l})
}
return ctx
}

type tagLoggable struct {
logger.Loggable
}

func (l tagLoggable) StatsTags() Tags {
return Tags(l.LogFields())
}

// WatchingTagLoggable combines WatchingTaggable and logger.WatchingLoggable
// If the Loggable is a Taggable already (implements StatsTags), it will be used directly.
// If StatsTags doesn't exist, LogFields() will be used instead.
func WatchingTagLoggable(ctx context.Context, l logger.Loggable) context.Context {
ctx = logger.WatchingLoggable(ctx, l)
if taggable, ok := l.(Taggable); ok {
ctx = WatchingTaggable(ctx, taggable)
} else {
ctx = WatchingTaggable(ctx, tagLoggable{l})
}
return ctx
}

// getStatsTags returns the merged tags as a list
// Meant to be used by the metrics when inlining the tags
func getStatsTags(ctx context.Context, extraTagList ...Tags) []string {
Expand Down
41 changes: 0 additions & 41 deletions statsd/taggable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,9 @@ package statsd
import (
"context"
"fmt"
"os"
"testing"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"

"github.com/Shopify/goose/logger"
)

var exampleBackend = NewForwardingBackend(func(_ context.Context, mType string, name string, value interface{}, tags []string, _ float64) error {
Expand All @@ -23,16 +19,6 @@ func (t *testTaggable) StatsTags() Tags {
return Tags(*t)
}

type testLoggable logrus.Fields

func (l *testLoggable) LogFields() logrus.Fields {
return logrus.Fields(*l)
}

func (l *testLoggable) StatsTags() Tags {
return SelectKeys(l.LogFields(), "testField")
}

func ExampleWithTags() {
SetBackend(exampleBackend)

Expand Down Expand Up @@ -61,33 +47,6 @@ func ExampleWatchingTaggable() {
// count: page.view: 10 [email:unknown user:anonymous]
}

func ExampleSelectKeys() {
// <setup for example>
logrusLogger := logrus.New()
logrusLogger.Out = os.Stdout
logrusLogger.Formatter = &logrus.TextFormatter{
DisableColors: true,
DisableTimestamp: true,
}
entry := logrus.NewEntry(logrusLogger)
SetBackend(exampleBackend)
// </setup for example>

session := &testLoggable{"foo": "bar", "testField": "test"}

ctx := context.Background()
ctx = WithTagLoggable(ctx, session)

metric := &Counter{Name: "page.view"}
metric.Count(ctx, 10)

logger.ContextLog(ctx, nil, entry).Info("example")

// Output:
// count: page.view: 10 [testField:test]
// level=info msg=example foo=bar testField=test
}

func TestEmptyContext(t *testing.T) {
ctx := context.Background()
// Using a basic type on purpose, disable linter
Expand Down

0 comments on commit 4be1d82

Please sign in to comment.