Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a client object to libhoney for isolating traffic #40

Merged
merged 25 commits into from Feb 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
eeef19d
add a libhoney Client type to manage separate transmission queues
maplebed Feb 7, 2019
4240acb
fixing up tests
maplebed Feb 8, 2019
fdcdc93
fix tests
maplebed Feb 10, 2019
95f6ccb
adding tests for clients
maplebed Feb 10, 2019
7838da8
attempting and failing another implementation of Client
maplebed Feb 12, 2019
a500a26
change the Client from an interface to a struct, move transmission to…
maplebed Feb 13, 2019
b9ce6c1
updating and adding tests
maplebed Feb 15, 2019
dc684f5
more test fixing
maplebed Feb 15, 2019
647b92d
naming cleanup
maplebed Feb 15, 2019
1c5f1c2
Other things (such as the beeline) use the default WriterOutput, so w…
maplebed Feb 15, 2019
6ea6c07
little by little
maplebed Feb 15, 2019
b4aecc2
more of the published interface must remain the same
maplebed Feb 15, 2019
8b0dd45
globals are terrible. sync.Once broke unrelated tests
maplebed Feb 16, 2019
08c75ea
try out newer versions of go
maplebed Feb 16, 2019
00dbc3b
oh yaml
maplebed Feb 16, 2019
aa134f8
adding more race tests
maplebed Feb 17, 2019
7bb922b
this is actually done; marshaling protection is in the transmission now.
maplebed Feb 19, 2019
0fd79da
add in protection against modifying an event after sending, to give c…
maplebed Feb 19, 2019
e89d0aa
PR feedback
maplebed Feb 19, 2019
c2a186f
race race race
maplebed Feb 19, 2019
90121c0
re-add missing mock output to match old api surface
maplebed Feb 19, 2019
020d8ee
re-add missing mock output to match old api surface
maplebed Feb 19, 2019
f673dec
removing nil pointer receiver checks in Client because they don't exi…
maplebed Feb 21, 2019
c9399e9
PR feedback
maplebed Feb 21, 2019
f4991a9
PR feedback
maplebed Feb 21, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions .travis.yml
Expand Up @@ -4,6 +4,10 @@ dist: trusty

go:
- 1.6
- 1.8
- 1.9
- "1.10"
- 1.11

script:
- go test ./... -race -v
233 changes: 233 additions & 0 deletions client.go
@@ -0,0 +1,233 @@
package libhoney

import (
"errors"
"sync"

"github.com/honeycombio/libhoney-go/transmission"
)

// Client represents an object that can create new builders and events and send
// them somewhere. It maintains its own sending queue for events, distinct from
// both the package-level libhoney queue and any other client. Clients should be
// created with NewClient(config). A manually created Client{} will function as
// a nil output and drop everything handed to it (so can be safely used in dev
// and tests). For more complete testing you can create a Client with a
// MockOutput transmission then inspect the events it would have sent.
type Client struct {
transmission transmission.Sender
logger Logger
builder *Builder
maplebed marked this conversation as resolved.
Show resolved Hide resolved

oneTx sync.Once
oneLogger sync.Once
oneBuilder sync.Once
}

// ClientConfig is a subset of the global libhoney config that focuses on the
// configuration of the client itself. The other config options are specific to
// a given transmission Sender and should be specified there if the defaults
// need to be overridden.
type ClientConfig struct {
// APIKey is the Honeycomb authentication token. If it is specified during
// libhoney initialization, it will be used as the default API key for all
// events. If absent, API key must be explicitly set on a builder or
// event. Find your team's API keys at https://ui.honeycomb.io/account
APIKey string

// Dataset is the name of the Honeycomb dataset to which to send these events.
// If it is specified during libhoney initialization, it will be used as the
// default dataset for all events. If absent, dataset must be explicitly set
// on a builder or event.
Dataset string

// SampleRate is the rate at which to sample this event. Default is 1,
// meaning no sampling. If you want to send one event out of every 250 times
// Send() is called, you would specify 250 here.
SampleRate uint

// APIHost is the hostname for the Honeycomb API server to which to send this
// event. default: https://api.honeycomb.io/
APIHost string

// Transmission allows you to override what happens to events after you call
// Send() on them. By default, events are asynchronously sent to the
// Honeycomb API. You can use the MockOutput included in this package in
// unit tests, or use the transmission.WriterSender to write events to
// STDOUT or to a file when developing locally.
Transmission transmission.Sender

// Logger defaults to nil and the SDK is silent. If you supply a logger here
// (or set it to &DefaultLogger{}), some debugging output will be emitted.
// Intended for human consumption during development to understand what the
// SDK is doing and diagnose trouble emitting events.
Logger Logger
}

// NewClient creates a Client with defaults correctly set
func NewClient(conf ClientConfig) (*Client, error) {
if conf.SampleRate == 0 {
conf.SampleRate = defaultSampleRate
}
if conf.APIHost == "" {
conf.APIHost = defaultAPIHost
}
if conf.Dataset == "" {
conf.Dataset = defaultDataset
}

c := &Client{
logger: conf.Logger,
}
c.ensureLogger()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not have another block above the c initialization, and guarantee that NewClient always returns an initialized logger?

if conf.Logger == nil {
  conf.Logger = &nullLogger{} 
}

c := &Client{
  logger: conf.Logger
}

the Transmission stuff below already it this way.

come to think of it, the Transmission checks below should be rewritten into the same style as the things above:

if conf.Transmission == nil {
  conf.Transmission = ...
}

and then can be included in the &Client initialization as with logger. wish Builder's initialization could be as well. that circular .client ref is a nice future refactoring target.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not clear why that would be better. The way it is not the sync.Once gets triggered during initialization so all future calls quickly noop. Delaying that till the first place they’re hit makes for a more likely collision (and therefore block) doesn’t it?

Can you say more about the motivation for this suggested change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just makes initialization more consistent - right now there are 3 different patterns for conf fallback to initialize different fields in the Client struct.

The behavior you're trying to avoid (no ensure* call leading to sync.Once collision later) is also how the c.transmission initialization below acts :/

How about both?

  1. change the initialization pattern (for everything but .builder, since it requires the backref) to be consistent:
  • Check conf setting for nil, fallback by putting the one we want in conf
  • Client init solely out of conf
  1. post Client init, call c.transmission.Start() so we can early error, then call both ensureLogger() and ensureTransmission() to use up their sync.Once's before returning the client.


if conf.Transmission == nil {
c.transmission = &transmission.Honeycomb{
MaxBatchSize: DefaultMaxBatchSize,
BatchTimeout: DefaultBatchTimeout,
MaxConcurrentBatches: DefaultMaxConcurrentBatches,
PendingWorkCapacity: DefaultPendingWorkCapacity,
UserAgentAddition: UserAgentAddition,
Logger: c.logger,
Metrics: sd,
}
} else {
c.transmission = conf.Transmission
}
if err := c.transmission.Start(); err != nil {
c.logger.Printf("transmission client failed to start: %s", err.Error())
return nil, err
}

c.builder = &Builder{
maplebed marked this conversation as resolved.
Show resolved Hide resolved
WriteKey: conf.APIKey,
Dataset: conf.Dataset,
SampleRate: conf.SampleRate,
APIHost: conf.APIHost,
dynFields: make([]dynamicField, 0, 0),
fieldHolder: fieldHolder{
data: make(map[string]interface{}),
},
client: c,
}

return c, nil
}

func (c *Client) ensureTransmission() {
maplebed marked this conversation as resolved.
Show resolved Hide resolved
c.oneTx.Do(func() {
if c.transmission == nil {
c.transmission = &transmission.DiscardSender{}
c.transmission.Start()
}
})
}

func (c *Client) ensureLogger() {
c.oneLogger.Do(func() {
if c.logger == nil {
c.logger = &nullLogger{}
}
})
}

func (c *Client) ensureBuilder() {
c.oneBuilder.Do(func() {
if c.builder == nil {
c.builder = &Builder{
dynFields: make([]dynamicField, 0, 0),
fieldHolder: fieldHolder{
data: make(map[string]interface{}),
},
client: c,
}
}
})
}

// Close waits for all in-flight messages to be sent. You should
// call Close() before app termination.
func (c *Client) Close() {
c.ensureLogger()
c.logger.Printf("closing libhoney client")
if c.transmission != nil {
c.transmission.Stop()
}
}

// Flush closes and reopens the Output interface, ensuring events
// are sent without waiting on the batch to be sent asyncronously.
// Generally, it is more efficient to rely on asyncronous batches than to
// call Flush, but certain scenarios may require Flush if asynchronous sends
// are not guaranteed to run (i.e. running in AWS Lambda)
// Flush is not thread safe - use it only when you are sure that no other
// parts of your program are calling Send
func (c *Client) Flush() {
c.ensureLogger()
c.logger.Printf("flushing libhoney client")
if c.transmission != nil {
c.transmission.Stop()
c.transmission.Start()
}
}

// TxResponses returns the channel from which the caller can read the responses
// to sent events.
func (c *Client) TxResponses() chan transmission.Response {
c.ensureTransmission()
return c.transmission.TxResponses()
}

// AddDynamicField takes a field name and a function that will generate values
// for that metric. The function is called once every time a NewEvent() is
// created and added as a field (with name as the key) to the newly created
// event.
func (c *Client) AddDynamicField(name string, fn func() interface{}) error {
c.ensureTransmission()
c.ensureBuilder()
return c.builder.AddDynamicField(name, fn)
}

// AddField adds a Field to the Client's scope. This metric will be inherited by
// all builders and events.
func (c *Client) AddField(name string, val interface{}) {
c.ensureTransmission()
c.ensureBuilder()
c.builder.AddField(name, val)
}

// Add adds its data to the Client's scope. It adds all fields in a struct or
// all keys in a map as individual Fields. These metrics will be inherited by
// all builders and events.
func (c *Client) Add(data interface{}) error {
c.ensureTransmission()
c.ensureBuilder()
return c.builder.Add(data)
}

// NewEvent creates a new event prepopulated with any Fields present in the
// Client's scope.
func (c *Client) NewEvent() *Event {
c.ensureTransmission()
c.ensureBuilder()
return c.builder.NewEvent()
}

// NewBuilder creates a new event builder. The builder inherits any Dynamic or
// Static Fields present in the Client's scope.
func (c *Client) NewBuilder() *Builder {
c.ensureTransmission()
c.ensureBuilder()
return c.builder.Clone()
}

// sendResponse sends a dropped event response down the response channel
func (c *Client) sendDroppedResponse(e *Event, message string) {
c.ensureTransmission()
r := transmission.Response{
Err: errors.New(message),
Metadata: e.Metadata,
}
c.transmission.SendResponse(r)

}