Skip to content

Commit

Permalink
refactor: use client.options internally to avoid copying (#651)
Browse files Browse the repository at this point in the history
Co-authored-by: Michi Hoffmann <cleptric@users.noreply.github.com>
  • Loading branch information
vaind and cleptric authored Jun 12, 2023
1 parent 4a965bc commit 97a00a4
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 46 deletions.
68 changes: 33 additions & 35 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,28 @@ type Client struct {
// single goroutine) or hub methods (for concurrent programs, for example web
// servers).
func NewClient(options ClientOptions) (*Client, error) {
// The default error event sample rate for all SDKs is 1.0 (send all).
//
// In Go, the zero value (default) for float64 is 0.0, which means that
// constructing a client with NewClient(ClientOptions{}), or, equivalently,
// initializing the SDK with Init(ClientOptions{}) without an explicit
// SampleRate would drop all events.
//
// To retain the desired default behavior, we exceptionally flip SampleRate
// from 0.0 to 1.0 here. Setting the sample rate to 0.0 is not very useful
// anyway, and the same end result can be achieved in many other ways like
// not initializing the SDK, setting the DSN to the empty string or using an
// event processor that always returns nil.
//
// An alternative API could be such that default options don't need to be
// the same as Go's zero values, for example using the Functional Options
// pattern. That would either require a breaking change if we want to reuse
// the obvious NewClient name, or a new function as an alternative
// constructor.
if options.SampleRate == 0.0 {
options.SampleRate = 1.0
}

if options.Debug {
debugWriter := options.DebugWriter
if debugWriter == nil {
Expand Down Expand Up @@ -374,8 +396,8 @@ func (client *Client) AddEventProcessor(processor EventProcessor) {
}

// Options return ClientOptions for the current Client.
// TODO don't access this internally to avoid creating a copy each time.
func (client Client) Options() ClientOptions {
// Note: internally, consider using `client.options` instead of `client.Options()` to avoid copying the object each time.
return client.options
}

Expand Down Expand Up @@ -476,7 +498,7 @@ func (client *Client) EventFromMessage(message string, level Level) *Event {
event.Level = level
event.Message = message

if client.Options().AttachStacktrace {
if client.options.AttachStacktrace {
event.Threads = []Thread{{
Stacktrace: NewStacktrace(),
Crashed: false,
Expand Down Expand Up @@ -516,34 +538,10 @@ func (client *Client) processEvent(event *Event, hint *EventHint, scope EventMod
return client.CaptureException(err, hint, scope)
}

options := client.Options()

// The default error event sample rate for all SDKs is 1.0 (send all).
//
// In Go, the zero value (default) for float64 is 0.0, which means that
// constructing a client with NewClient(ClientOptions{}), or, equivalently,
// initializing the SDK with Init(ClientOptions{}) without an explicit
// SampleRate would drop all events.
//
// To retain the desired default behavior, we exceptionally flip SampleRate
// from 0.0 to 1.0 here. Setting the sample rate to 0.0 is not very useful
// anyway, and the same end result can be achieved in many other ways like
// not initializing the SDK, setting the DSN to the empty string or using an
// event processor that always returns nil.
//
// An alternative API could be such that default options don't need to be
// the same as Go's zero values, for example using the Functional Options
// pattern. That would either require a breaking change if we want to reuse
// the obvious NewClient name, or a new function as an alternative
// constructor.
if options.SampleRate == 0.0 {
options.SampleRate = 1.0
}

// Transactions are sampled by options.TracesSampleRate or
// options.TracesSampler when they are started. All other events
// (errors, messages) are sampled here.
if event.Type != transactionType && !sample(options.SampleRate) {
if event.Type != transactionType && !sample(client.options.SampleRate) {
Logger.Println("Event dropped due to SampleRate hit.")
return nil
}
Expand All @@ -556,15 +554,15 @@ func (client *Client) processEvent(event *Event, hint *EventHint, scope EventMod
if hint == nil {
hint = &EventHint{}
}
if event.Type == transactionType && options.BeforeSendTransaction != nil {
if event.Type == transactionType && client.options.BeforeSendTransaction != nil {
// Transaction events
if event = options.BeforeSendTransaction(event, hint); event == nil {
if event = client.options.BeforeSendTransaction(event, hint); event == nil {
Logger.Println("Transaction dropped due to BeforeSendTransaction callback.")
return nil
}
} else if event.Type != transactionType && options.BeforeSend != nil {
} else if event.Type != transactionType && client.options.BeforeSend != nil {
// All other events
if event = options.BeforeSend(event, hint); event == nil {
if event = client.options.BeforeSend(event, hint); event == nil {
Logger.Println("Event dropped due to BeforeSend callback.")
return nil
}
Expand All @@ -590,23 +588,23 @@ func (client *Client) prepareEvent(event *Event, hint *EventHint, scope EventMod
}

if event.ServerName == "" {
event.ServerName = client.Options().ServerName
event.ServerName = client.options.ServerName

if event.ServerName == "" {
event.ServerName = hostname
}
}

if event.Release == "" {
event.Release = client.Options().Release
event.Release = client.options.Release
}

if event.Dist == "" {
event.Dist = client.Options().Dist
event.Dist = client.options.Dist
}

if event.Environment == "" {
event.Environment = client.Options().Environment
event.Environment = client.options.Environment
}

event.Platform = "go"
Expand Down
6 changes: 2 additions & 4 deletions dynamic_sampling_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ func DynamicSamplingContextFromTransaction(span *Span) DynamicSamplingContext {
}
}

options := client.Options()

if traceID := span.TraceID.String(); traceID != "" {
entries["trace_id"] = traceID
}
Expand All @@ -66,10 +64,10 @@ func DynamicSamplingContextFromTransaction(span *Span) DynamicSamplingContext {
entries["public_key"] = publicKey
}
}
if release := options.Release; release != "" {
if release := client.options.Release; release != "" {
entries["release"] = release
}
if environment := options.Environment; environment != "" {
if environment := client.options.Environment; environment != "" {
entries["environment"] = environment
}

Expand Down
7 changes: 3 additions & 4 deletions hub.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,17 +280,16 @@ func (hub *Hub) AddBreadcrumb(breadcrumb *Breadcrumb, hint *BreadcrumbHint) {
return
}

options := client.Options()
max := options.MaxBreadcrumbs
max := client.options.MaxBreadcrumbs
if max < 0 {
return
}

if options.BeforeBreadcrumb != nil {
if client.options.BeforeBreadcrumb != nil {
if hint == nil {
hint = &BreadcrumbHint{}
}
if breadcrumb = options.BeforeBreadcrumb(breadcrumb, hint); breadcrumb == nil {
if breadcrumb = client.options.BeforeBreadcrumb(breadcrumb, hint); breadcrumb == nil {
Logger.Println("breadcrumb dropped due to BeforeBreadcrumb callback.")
return
}
Expand Down
2 changes: 1 addition & 1 deletion integrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (iei *ignoreErrorsIntegration) Name() string {
}

func (iei *ignoreErrorsIntegration) SetupOnce(client *Client) {
iei.ignoreErrors = transformStringsIntoRegexps(client.Options().IgnoreErrors)
iei.ignoreErrors = transformStringsIntoRegexps(client.options.IgnoreErrors)
client.AddEventProcessor(iei.processor)
}

Expand Down
2 changes: 1 addition & 1 deletion interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func NewRequest(r *http.Request) *Request {
var env map[string]string
headers := map[string]string{}

if client := CurrentHub().Client(); client != nil && client.Options().SendDefaultPII {
if client := CurrentHub().Client(); client != nil && client.options.SendDefaultPII {
// We read only the first Cookie header because of the specification:
// https://tools.ietf.org/html/rfc6265#section-5.4
// When the user agent generates an HTTP request, the user agent MUST NOT
Expand Down
2 changes: 1 addition & 1 deletion span_recorder.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type spanRecorder struct {
func (r *spanRecorder) record(s *Span) {
maxSpans := defaultMaxSpans
if client := CurrentHub().Client(); client != nil {
maxSpans = client.Options().MaxSpans
maxSpans = client.options.MaxSpans
}
r.mu.Lock()
defer r.mu.Unlock()
Expand Down

0 comments on commit 97a00a4

Please sign in to comment.